[PATCH 2/5] d3dcompiler: Introduce a separate structure for lvalue derefs.
Zebediah Figura
z.figura12 at gmail.com
Tue Apr 14 19:39:55 CDT 2020
From: Zebediah Figura <zfigura at codeweavers.com>
Signed-off-by: Zebediah Figura <z.figura12 at 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(" ");
--
2.26.0
More information about the wine-devel
mailing list