2 Jul
2020
2 Jul
'20
6:08 p.m.
On Thu, Jul 2, 2020 at 6:58 PM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Right. I'm convinced that never using an assignment / store as a
source is just a good idea, regardless.
On 7/2/20 11:15 AM, Matteo Bruni wrote:
Yeah, my feeling is that these new loads won't be different from other
redundant loads that come up in shaders during compilation (i.e. copy
propagation of some kind can scoop up all of them).
On Tue, Jun 30, 2020 at 2:03 AM Zebediah Figura
<z.figura12(a)gmail.com> wrote:
It didn't seem obviously better to me, given the extra allocation, but I
think there's no reason it can't work. Certainly it avoids 5/5...
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
dlls/d3dcompiler_43/d3dcompiler_private.h | 2 ++
dlls/d3dcompiler_43/utils.c | 14 ++++++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/dlls/d3dcompiler_43/d3dcompiler_private.h
b/dlls/d3dcompiler_43/d3dcompiler_private.h
index 23eff210940..2ff019d45a6 100644
--- a/dlls/d3dcompiler_43/d3dcompiler_private.h
+++ b/dlls/d3dcompiler_43/d3dcompiler_private.h
@@ -786,6 +786,8 @@ enum hlsl_ir_expr_op {
HLSL_IR_UNOP_POSTINC,
HLSL_IR_UNOP_POSTDEC,
+ HLSL_IR_UNOP_IDENT,
+
HLSL_IR_BINOP_ADD,
HLSL_IR_BINOP_SUB,
HLSL_IR_BINOP_MUL,
diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c
index 4bc3b9b0a64..b5422abd1e8 100644
--- a/dlls/d3dcompiler_43/utils.c
+++ b/dlls/d3dcompiler_43/utils.c
@@ -1448,6 +1448,7 @@ struct hlsl_ir_node *add_assignment(struct list *instrs, struct
hlsl_ir_node *lh
{
struct hlsl_ir_assignment *assign = d3dcompiler_alloc(sizeof(*assign));
struct hlsl_type *lhs_type;
+ struct hlsl_ir_node *copy;
DWORD writemask = 0;
lhs_type = lhs->data_type;
@@ -1511,7 +1512,7 @@ struct hlsl_ir_node *add_assignment(struct list *instrs, struct
hlsl_ir_node *lh
lhs = lhs_inner;
}
- init_node(&assign->node, HLSL_IR_ASSIGNMENT, lhs_type, lhs->loc);
+ init_node(&assign->node, HLSL_IR_ASSIGNMENT, NULL, lhs->loc);
assign->writemask = writemask;
assign->lhs.var = load_from_node(lhs)->src.var;
hlsl_src_from_node(&assign->lhs.offset,
load_from_node(lhs)->src.offset.node);
@@ -1528,7 +1529,14 @@ struct hlsl_ir_node *add_assignment(struct list *instrs, struct
hlsl_ir_node *lh
hlsl_src_from_node(&assign->rhs, rhs);
list_add_tail(instrs, &assign->node.entry);
- return &assign->node;
+ /* Don't use the instruction itself as a source, as this makes structure
+ * splitting easier. Instead copy it here. Since we retrieve sources from
+ * the last instruction in the list, we do need to copy. */
Not just that, what we now define as an assignment (really a STORE)
doesn't really make sense anymore as a source.
The need to copy the instruction is a bit annoying though. What about
storing the result into a temporary variable and adding an extra load
instead? Does it make
struct splitting hard / impossible (e.g. along
the lines of: replace this instruction with this other instruction and
the copy -> now we need to replace the copy -> back to square one)?
The algorithm I have basically generates store + load instructions for
each struct field, then removes the original. This breaks if the
original is used as a source, so I figured it was best if we just didn't
do that.
The other easy solution I see is to iterate through the instruction list
in reverse.