From: Zebediah Figura zfigura@codeweavers.com
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- aaa625217, and the approach it implies, works, but on further reflection it's still not very pretty. For example, the following line of HLSL:
var.a.b = 2.0;
produces the following IR:
2: 2.0 3: deref(var) 4: @3.b 5: @4.c = @2
This essentially works, provided that the codegen layer knows how to unwind a deref chain, but it's pretty janky. Node 4 implies we're reading from node 3, and node 3 implies we're reading from var, neither of which is actually happening. The RA pass can't easily be smart enough to recognize that 4 doesn't need to read from 3, which is a problem if we introduce a "maybe uninitialized" warning.
Moreover, if we introduce DCE, we can't DCE out node 3 because of node 4, and while we could DCE out node 4 in terms of removing it from the list, we can't actually destroy the node, since node 5 is using it. Having nodes not in the list is also janky, I feel.
With this patch we instead get the following IR:
2: 2.0 3: deref(var) 4: @3.b 5: @4.c 6: deref(var).b.c = @2
We still get redundant reads, but at least this time we can easily DCE them out. Needing less sanity checks in RA is also nice.
dlls/d3dcompiler_43/d3dcompiler_private.h | 21 +++++- dlls/d3dcompiler_43/hlsl.y | 28 +++++--- dlls/d3dcompiler_43/utils.c | 83 ++++++++++++++++------- 3 files changed, 97 insertions(+), 35 deletions(-)
diff --git a/dlls/d3dcompiler_43/d3dcompiler_private.h b/dlls/d3dcompiler_43/d3dcompiler_private.h index 1f1d8e26637..8f81271383b 100644 --- a/dlls/d3dcompiler_43/d3dcompiler_private.h +++ b/dlls/d3dcompiler_43/d3dcompiler_private.h @@ -873,10 +873,29 @@ struct hlsl_ir_deref struct hlsl_deref src; };
+struct hlsl_lvalue +{ + enum hlsl_ir_deref_type type; + union + { + struct hlsl_ir_var *var; + struct + { + struct hlsl_lvalue *array; + struct hlsl_ir_node *index; + } array; + struct + { + struct hlsl_lvalue *record; + struct hlsl_struct_field *field; + } record; + } v; +}; + struct hlsl_ir_assignment { struct hlsl_ir_node node; - struct hlsl_deref lhs; + struct hlsl_lvalue lhs; struct hlsl_ir_node *rhs; unsigned char writemask; }; diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index d3ef18bb6e9..5366181f356 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -2642,16 +2642,16 @@ static unsigned int index_instructions(struct list *instrs, unsigned int index) }
/* Walk the chain of derefs and retrieve the actual variable we care about. */ -static struct hlsl_ir_var *hlsl_var_from_deref(const struct hlsl_deref *deref) +static struct hlsl_ir_var *hlsl_var_from_lvalue(const struct hlsl_lvalue *deref) { switch (deref->type) { case HLSL_IR_DEREF_VAR: return deref->v.var; case HLSL_IR_DEREF_ARRAY: - return hlsl_var_from_deref(&deref_from_node(deref->v.array.array)->src); + return hlsl_var_from_lvalue(deref->v.array.array); case HLSL_IR_DEREF_RECORD: - return hlsl_var_from_deref(&deref_from_node(deref->v.record.record)->src); + return hlsl_var_from_lvalue(deref->v.record.record); } assert(0); return NULL; @@ -2674,7 +2674,7 @@ static void compute_liveness_recurse(struct list *instrs, unsigned int loop_firs case HLSL_IR_ASSIGNMENT: { struct hlsl_ir_assignment *assignment = assignment_from_node(instr); - var = hlsl_var_from_deref(&assignment->lhs); + var = hlsl_var_from_lvalue(&assignment->lhs); if (!var->first_write) var->first_write = loop_first ? min(instr->index, loop_first) : instr->index; assignment->rhs->last_read = instr->index; @@ -2693,10 +2693,22 @@ static void compute_liveness_recurse(struct list *instrs, unsigned int loop_firs case HLSL_IR_DEREF: { struct hlsl_ir_deref *deref = deref_from_node(instr); - var = hlsl_var_from_deref(&deref->src); - var->last_read = loop_last ? max(instr->index, loop_last) : instr->index; - if (deref->src.type == HLSL_IR_DEREF_ARRAY) - deref->src.v.array.index->last_read = instr->index; + + switch (deref->src.type) + { + case HLSL_IR_DEREF_VAR: + deref->src.v.var->last_read = loop_last ? max(instr->index, loop_last) : instr->index; + break; + + case HLSL_IR_DEREF_ARRAY: + deref->src.v.array.array->last_read = instr->index; + deref->src.v.array.index->last_read = instr->index; + break; + + case HLSL_IR_DEREF_RECORD: + deref->src.v.record.record->last_read = instr->index; + break; + } break; } case HLSL_IR_EXPR: diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c index d24341329d3..519f50612e8 100644 --- a/dlls/d3dcompiler_43/utils.c +++ b/dlls/d3dcompiler_43/utils.c @@ -1478,27 +1478,41 @@ static unsigned int invert_swizzle(unsigned int *swizzle, unsigned int writemask return new_writemask; }
-static BOOL validate_lhs_deref(const struct hlsl_ir_node *lhs) +static BOOL get_assignment_lhs(struct hlsl_lvalue *dst, struct hlsl_ir_node *node) { struct hlsl_ir_deref *deref;
- if (lhs->type != HLSL_IR_DEREF) + if (node->type != HLSL_IR_DEREF) { - hlsl_report_message(lhs->loc, HLSL_LEVEL_ERROR, "invalid lvalue"); + hlsl_report_message(node->loc, HLSL_LEVEL_ERROR, "invalid lvalue"); return FALSE; }
- deref = deref_from_node(lhs); + deref = deref_from_node(node); + dst->type = deref->src.type;
- if (deref->src.type == HLSL_IR_DEREF_VAR) - return TRUE; - if (deref->src.type == HLSL_IR_DEREF_ARRAY) - return validate_lhs_deref(deref->src.v.array.array); - if (deref->src.type == HLSL_IR_DEREF_RECORD) - return validate_lhs_deref(deref->src.v.record.record); + switch (deref->src.type) + { + case HLSL_IR_DEREF_VAR: + dst->v.var = deref->src.v.var; + return TRUE;
- assert(0); - return FALSE; + case HLSL_IR_DEREF_ARRAY: + dst->v.array.index = deref->src.v.array.index; + if (!(dst->v.array.array = d3dcompiler_alloc(sizeof(*dst->v.array.array)))) + return FALSE; + return get_assignment_lhs(dst->v.array.array, deref->src.v.array.array); + + case HLSL_IR_DEREF_RECORD: + dst->v.record.field = deref->src.v.record.field; + if (!(dst->v.record.record = d3dcompiler_alloc(sizeof(*dst->v.record.field)))) + return FALSE; + return get_assignment_lhs(dst->v.record.record, deref->src.v.record.record); + + default: + assert(0); + return FALSE; + } }
struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign_op assign_op, @@ -1506,6 +1520,7 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign { struct hlsl_ir_assignment *assign = d3dcompiler_alloc(sizeof(*assign)); DWORD writemask = (1 << lhs->data_type->dimx) - 1; + struct hlsl_ir_node *lhs_inner; struct hlsl_type *type;
if (!assign) @@ -1516,8 +1531,6 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign
while (lhs->type != HLSL_IR_DEREF) { - struct hlsl_ir_node *lhs_inner; - if (lhs->type == HLSL_IR_EXPR && expr_from_node(lhs)->op == HLSL_IR_UNOP_CAST) { FIXME("Cast on the lhs.\n"); @@ -1554,12 +1567,6 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign lhs = lhs_inner; }
- if (!validate_lhs_deref(lhs)) - { - d3dcompiler_free(assign); - return NULL; - } - TRACE("Creating proper assignment expression.\n"); if (writemask == BWRITERSP_WRITEMASK_ALL) type = lhs->data_type; @@ -1591,14 +1598,19 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign type_class = lhs->data_type->type; type = new_hlsl_type(NULL, type_class, lhs->data_type->base_type, dimx, 1); } + + if (!get_assignment_lhs(&assign->lhs, lhs)) + { + d3dcompiler_free(assign); + return NULL; + } + assign->node.type = HLSL_IR_ASSIGNMENT; assign->node.loc = lhs->loc; assign->node.data_type = type; assign->writemask = writemask; - rhs = implicit_conversion(rhs, type, &rhs->loc);
- assign->lhs = deref_from_node(lhs)->src; if (assign_op != ASSIGN_OP_ASSIGN) { enum hlsl_ir_expr_op op = op_from_assignment(assign_op); @@ -1619,9 +1631,8 @@ struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign } else { - list_remove(&lhs->entry); - /* Don't recursively free the deref; we just copied its members. */ - d3dcompiler_free(lhs); + /* We could free the LHS derefs here, since we aren't using them + * anymore, but it's easier just to leave that to a DCE pass. */ assign->rhs = rhs; }
@@ -1919,6 +1930,26 @@ static void debug_dump_ir_var(const struct hlsl_ir_var *var) wine_dbg_printf(" : %s", debugstr_a(var->semantic)); }
+static void debug_dump_lvalue(const struct hlsl_lvalue *deref) +{ + switch (deref->type) + { + case HLSL_IR_DEREF_VAR: + debug_dump_ir_var(deref->v.var); + break; + case HLSL_IR_DEREF_ARRAY: + debug_dump_lvalue(deref->v.array.array); + wine_dbg_printf("["); + debug_dump_src(deref->v.array.index); + wine_dbg_printf("]"); + break; + case HLSL_IR_DEREF_RECORD: + debug_dump_lvalue(deref->v.record.record); + wine_dbg_printf(".%s", debugstr_a(deref->v.record.field->name)); + break; + } +} + static void debug_dump_deref(const struct hlsl_deref *deref) { switch (deref->type) @@ -2107,7 +2138,7 @@ static const char *debug_writemask(DWORD writemask) static void debug_dump_ir_assignment(const struct hlsl_ir_assignment *assign) { wine_dbg_printf("= ("); - debug_dump_deref(&assign->lhs); + debug_dump_lvalue(&assign->lhs); if (assign->writemask != BWRITERSP_WRITEMASK_ALL) wine_dbg_printf("%s", debug_writemask(assign->writemask)); wine_dbg_printf(" ");