On Thu, Jul 2, 2020 at 6:58 PM Zebediah Figura zfigura@codeweavers.com wrote:
On 7/2/20 11:15 AM, Matteo Bruni wrote:
On Tue, Jun 30, 2020 at 2:03 AM Zebediah Figura z.figura12@gmail.com wrote:
Signed-off-by: Zebediah Figura zfigura@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?
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...
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).
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.
Right. I'm convinced that never using an assignment / store as a source is just a good idea, regardless.