From: Zebediah Figura Subject: Re: [PATCH 7/7] d3dcompiler: Store the "array" field in struct hlsl_deref as a struct hlsl_deref pointer. Message-Id: Date: Mon, 6 Apr 2020 20:52:18 -0500 In-Reply-To: <0732f556-c24b-6d01-3c15-051094855921@codeweavers.com> References: <20200306231736.8271-1-zfigura@codeweavers.com> <20200306231736.8271-7-zfigura@codeweavers.com> <1125771c-be33-aedf-d24d-251ac6154f38@codeweavers.com> <0732f556-c24b-6d01-3c15-051094855921@codeweavers.com> On 3/12/20 10:12 AM, Zebediah Figura wrote: > On 3/12/20 6:58 AM, Matteo Bruni wrote: >> On Wed, Mar 11, 2020 at 8:45 PM Zebediah Figura wrote: >>> >>> On 3/11/20 12:57 PM, Matteo Bruni wrote: >>>> On Sat, Mar 7, 2020 at 12:25 AM Zebediah Figura wrote: >>>>> >>>>> Signed-off-by: Zebediah Figura >>>>> --- >>>>> dlls/d3dcompiler_43/d3dcompiler_private.h | 2 +- >>>>> dlls/d3dcompiler_43/hlsl.y | 25 ++++++++++++++++++++--- >>>>> dlls/d3dcompiler_43/utils.c | 6 +++++- >>>>> 3 files changed, 28 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/dlls/d3dcompiler_43/d3dcompiler_private.h b/dlls/d3dcompiler_43/d3dcompiler_private.h >>>>> index d221d1056fa..f4f68b9e4cf 100644 >>>>> --- a/dlls/d3dcompiler_43/d3dcompiler_private.h >>>>> +++ b/dlls/d3dcompiler_43/d3dcompiler_private.h >>>>> @@ -907,7 +907,7 @@ struct hlsl_deref >>>>> struct hlsl_ir_var *var; >>>>> struct >>>>> { >>>>> - struct hlsl_ir_node *array; >>>>> + struct hlsl_deref *array; >>>>> struct hlsl_ir_node *index; >>>>> } array; >>>>> struct >>>>> diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y >>>>> index 40159c60dcc..11dd8026c16 100644 >>>>> --- a/dlls/d3dcompiler_43/hlsl.y >>>>> +++ b/dlls/d3dcompiler_43/hlsl.y >>>>> @@ -2114,8 +2114,17 @@ postfix_expr: primary_expr >>>>> /* This may be an array dereference or a vector/matrix >>>>> * subcomponent access. >>>>> * We store it as an array dereference in any case. */ >>>>> - struct hlsl_ir_deref *deref = d3dcompiler_alloc(sizeof(*deref)); >>>>> - struct hlsl_type *expr_type = node_from_list($1)->data_type; >>>>> + struct hlsl_ir_deref *deref = d3dcompiler_alloc(sizeof(*deref)), *src; >>>>> + struct hlsl_ir_node *node = node_from_list($1); >>>>> + struct hlsl_type *expr_type = node->data_type; >>>>> + >>>>> + if (node->type != HLSL_IR_DEREF) >>>>> + { >>>>> + FIXME("Unimplemented index of node type %s.\n", >>>>> + debug_node_type(node->type)); >>>>> + YYABORT; >>>>> + } >>>>> + src = deref_from_node(node); >>>> >>>> It looks like it's legal in HLSL to array dereference an arbitrary >>>> vector expression. E.g.: >>>> >>>> uniform int i; >>>> uniform float4 b; >>>> ... >>>> float4 a = {0.25, 0.5, 0.75, 1.0}; >>>> float r = (a * b)[i]; >>>> >>>> How do you plan to handle that? Mostly wondering if this patch would >>>> have to be effectively reverted in the future or you have something >>>> else in mind. >>>> >>> >>> Honestly, I was aware of this, and my plan was basically to ignore it >>> and deal with it when we come to it, if we ever do. >>> >>> I don't remember if there's a similar problem with struct fields, but >>> there probably is. If nothing else I would expect >>> "func_that_returns_struct().field" to work. >>> >>> The main reason I want this patch is because of the LHS. It only makes >>> sense to assign to a variable, valid HLSL expressions like "(++a)[0] = >>> b" notwithstanding. That gives us a clear separation between "variables" >>> and "anonymous expressions" which makes liveness tracking easier or at >>> least clearer (and also lets anonymous expressions be SSA values—not a >>> design choice I've taken advantage of thus far, but if in the future we >>> need to start doing optimizations, it's nice to have the compiler in a >>> state that makes that easy.) >>> >>> Currently my plan is to track liveness per variable (instead of e.g. per >>> element or field), so I have a function hlsl_var_from_deref() which >>> unwraps the hlsl_deref until it gets the initial variable. If it helps I >>> can post those patches here. >> >> It's not necessary, I understand your requirements[*]. I did basically >> the same thing in my hacky compiler. I'll mention that this is, in >> some way, one of the downsides of using the same IR for most / all of >> the compilation process. But it's not a blocker by any means. >> >> It's possible to solve the problem in a number of ways. Leaving aside >> full-blown SSA for the time being, we can still introduce shader >> transformation passes that can help in this regard. Probably the >> easiest would be a pass to split complex expressions into separate >> instructions, each in the form of a "conformant" assignment (i.e. to a >> temporary variable if necessary). So for my example above, you'd get >> something like: >> >>>> float r = (a * b)[i]; >> >> 1: deref(a) >> 2: deref(b) >> 3: * (a b) >> 4: deref(temp1) >> 5: = (4 3) >> 6: deref(i) >> 7: deref(temp1)[6] >> 8: deref(r) >> 9: = (8 7) > > Ah, so basically synthesize a variable every time we try to > index/subscript something that's not already a deref? Seems like an > attractive idea, and in fact would even work nicely with the example I > mentioned above ["(++a)[0] = b" apparently has no effect.] > > Though admittedly I am warming up to my last suggestion, which kind of > renders this unnecessary. > >> >> (I certainly didn't pick the nicest convention for those IR dumps, oh well...) >> >> Doing this kind of transformation before the liveness analysis should >> solve the issue at hand (and I'm pretty sure it would help with other >> optimization / codegen things). >> This calls for the question: how, and how much, should the IR before / >> after the pass be segregated? E.g. we could use the current >> hlsl_ir_deref and hlsl_ir_assignment only for the "before" and >> introduce separate versions for the "after" which enforce the "only >> derefs to variables" invariant. Other options would be to always use >> the current IR types, probably with some additional checks to ensure >> things are sane, or at the opposite end, create a whole separate, >> lower level IR (which could differ from the current IR in more ways >> than just the derefs, depending on what else might come in handy). >> Relatedly: I think it would be useful to have some form of validation >> (probably only enabled for warn+d3dcompiler or similar) to check that >> the shader IR invariants are respected at various points of the >> compilation process. > > Eh, maybe. I haven't found the language restrictive enough thus far to > warrant any new layers. Probably that'd change if I had to do serious > optimization, but thus far... > >> >>> As an alternative to 6/7 and 7/7 here, I could move the hlsl_node >>> unwrapping to hlsl_var_from_deref() [i.e. so that function checks the >>> node type of the "array" field and fails noisily for now if it's not >>> HLSL_IR_DEREF]. I don't particularly like that, both because it kind of >>> suggests that the internal representation of an LHS is allowed to not be >>> reducible to a variable, which is not great, and because codegen time is >>> not a particularly nice time to do those kinds of checks. >>> >>> A middle ground could be to do those checks only for the lhs in >>> make_assignment() or whatever it's called, leave them alone otherwise, >>> and then just throw a few assert()s in hlsl_var_from_deref(). >> >> I think this, combined with a transformation pass like the one I >> suggested (once you get to liveness), is a good option: you get to >> check for broken assignment LHS early and don't hamper support for >> those kind of fancy, but still valid, language constructs. >> >> [*]: Or at least I think I do; correct me if I'm off. >> > I gave it some examination and though and I decided I really don't like the current solution, for reasons which I mostly go into detail about in the attached patch 0001. I came up with two alternative approaches, and I'm not really sure which one I prefer: * patch 0001 just separates source and destination derefs, forcing the latter to be a chain of derefs ending in a var while the former can end in an arbitrary node, which has the advantage of generating less HLSL (not that it particularly matters); * patches 0100-0102 synthesizes a variable every time we encounter a subscript/index of anything other than a variable, thus implicitly forcing all derefs to be a chain of hlsl_deref ending in a var—which has the advantage of letting weird LHS constructions work "for free" (not that it particularly matters either). As an aside, I think that if a structure splitting pass does happen, we would need for arbitrary instructions to be able to hold either a hlsl_deref or an hlsl_ir_node for any source fields. I'm not actually sure that makes either approach better, but it seems at least worth thinking about. Any opinions? From 2b7cf1146d766824f10523e24499b8349d8ff118 Mon Sep 17 00:00:00 2001 From: Zebediah Figura Date: Sat, 4 Apr 2020 23:45:41 -0500 Subject: [PATCH] d3dcompiler: Introduce a separate structure for LHS derefs. Signed-off-by: Zebediah Figura --- 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 | 85 ++++++++++++++++------- 3 files changed, 99 insertions(+), 35 deletions(-) diff --git a/dlls/d3dcompiler_43/d3dcompiler_private.h b/dlls/d3dcompiler_43/d3dcompiler_private.h index 1f1d8e26637..46373102803 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_dst_deref +{ + enum hlsl_ir_deref_type type; + union + { + struct hlsl_ir_var *var; + struct + { + struct hlsl_dst_deref *array; + struct hlsl_ir_node *index; + } array; + struct + { + struct hlsl_dst_deref *record; + struct hlsl_struct_field *field; + } record; + } v; +}; + struct hlsl_ir_assignment { struct hlsl_ir_node node; - struct hlsl_deref lhs; + struct hlsl_dst_deref 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..b644a54ec8f 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_dst_deref(const struct hlsl_dst_deref *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_dst_deref(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_dst_deref(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_dst_deref(&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..a2b0ffc4572 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_dst_deref *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,28 @@ static void debug_dump_ir_var(const struct hlsl_ir_var *var) wine_dbg_printf(" : %s", debugstr_a(var->semantic)); } +static void debug_dump_dst_deref(const struct hlsl_dst_deref *deref) +{ + switch (deref->type) + { + case HLSL_IR_DEREF_VAR: + wine_dbg_printf("deref("); + debug_dump_ir_var(deref->v.var); + wine_dbg_printf(")"); + break; + case HLSL_IR_DEREF_ARRAY: + debug_dump_dst_deref(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_dst_deref(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 +2140,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_dst_deref(&assign->lhs); if (assign->writemask != BWRITERSP_WRITEMASK_ALL) wine_dbg_printf("%s", debug_writemask(assign->writemask)); wine_dbg_printf(" "); -- 2.26.0 From 1e8f91db792b76b2896e8d4e854168da73addf52 Mon Sep 17 00:00:00 2001 From: Zebediah Figura Date: Mon, 6 Apr 2020 15:51:48 -0500 Subject: [PATCH 100/102] d3dcompiler: Synthesize a variable when subscripting a non-deref node. Signed-off-by: Zebediah Figura --- dlls/d3dcompiler_43/hlsl.y | 59 +++++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index cbf3e136a70..3634989394a 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -507,6 +507,39 @@ static struct hlsl_ir_jump *new_return(struct hlsl_ir_node *value, struct source return jump; } +static struct hlsl_ir_var *new_synthetic_var(struct hlsl_type *type, const struct source_location loc) +{ + struct hlsl_ir_var *var; + + if (!(var = d3dcompiler_alloc(sizeof(*var)))) + { + hlsl_ctx.status = PARSE_ERR; + return NULL; + } + + var->data_type = type; + var->loc = loc; + list_add_tail(&hlsl_ctx.globals->vars, &var->scope_entry); + return var; +} + +static struct hlsl_ir_assignment *make_simple_assignment(struct hlsl_ir_var *lhs, + struct hlsl_ir_node *rhs, unsigned int writemask) +{ + struct hlsl_ir_assignment *assign; + + if (!(assign = d3dcompiler_alloc(sizeof(*assign)))) + return NULL; + + init_node(&assign->node, HLSL_IR_ASSIGNMENT, rhs->data_type, rhs->loc); + assign->lhs.type = HLSL_IR_DEREF_VAR; + assign->lhs.v.var = lhs; + assign->rhs = rhs; + assign->writemask = writemask; + + return assign; +} + static struct hlsl_ir_deref *new_var_deref(struct hlsl_ir_var *var, const struct source_location loc) { struct hlsl_ir_deref *deref = d3dcompiler_alloc(sizeof(*deref)); @@ -525,13 +558,31 @@ static struct hlsl_ir_deref *new_var_deref(struct hlsl_ir_var *var, const struct static struct hlsl_ir_deref *new_record_deref(struct hlsl_ir_node *record, struct hlsl_struct_field *field, const struct source_location loc) { - struct hlsl_ir_deref *deref = d3dcompiler_alloc(sizeof(*deref)); + struct hlsl_ir_deref *deref; - if (!deref) + if (record->type != HLSL_IR_DEREF) { - ERR("Out of memory.\n"); - return NULL; + struct hlsl_ir_assignment *assign; + struct hlsl_ir_var *var; + + if (!(var = new_synthetic_var(record->data_type, record->loc))) + return NULL; + + TRACE("Synthesized variable %p for %s node.\n", var, debug_node_type(record->type)); + + if (!(assign = make_simple_assignment(var, record, 0))) + return NULL; + list_add_after(&record->entry, &assign->node.entry); + + if (!(deref = new_var_deref(var, var->loc))) + return NULL; + list_add_after(&assign->node.entry, &deref->node.entry); + record = &deref->node; } + + if (!(deref = d3dcompiler_alloc(sizeof(*deref)))) + return NULL; + init_node(&deref->node, HLSL_IR_DEREF, field->type, loc); deref->src.type = HLSL_IR_DEREF_RECORD; deref->src.v.record.record = record; -- 2.26.0 From 4464d7133f5e717428c363f4e8a1bbb979e173db Mon Sep 17 00:00:00 2001 From: Zebediah Figura Date: Mon, 6 Apr 2020 16:30:55 -0500 Subject: [PATCH 101/102] d3dcompiler: Synthesize a variable when indexing a non-deref node. Signed-off-by: Zebediah Figura --- dlls/d3dcompiler_43/hlsl.y | 119 +++++++++++++++++++++---------------- 1 file changed, 69 insertions(+), 50 deletions(-) diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index 3634989394a..b1d88571282 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -555,30 +555,38 @@ static struct hlsl_ir_deref *new_var_deref(struct hlsl_ir_var *var, const struct return deref; } -static struct hlsl_ir_deref *new_record_deref(struct hlsl_ir_node *record, - struct hlsl_struct_field *field, const struct source_location loc) +static struct hlsl_ir_node *get_var_deref(struct hlsl_ir_node *node) { + struct hlsl_ir_assignment *assign; struct hlsl_ir_deref *deref; + struct hlsl_ir_var *var; - if (record->type != HLSL_IR_DEREF) - { - struct hlsl_ir_assignment *assign; - struct hlsl_ir_var *var; + if (node->type == HLSL_IR_DEREF) + return node; - if (!(var = new_synthetic_var(record->data_type, record->loc))) - return NULL; - TRACE("Synthesized variable %p for %s node.\n", var, debug_node_type(record->type)); + if (!(var = new_synthetic_var(node->data_type, node->loc))) + return NULL; - if (!(assign = make_simple_assignment(var, record, 0))) - return NULL; - list_add_after(&record->entry, &assign->node.entry); + TRACE("Synthesized variable %p for %s node.\n", var, debug_node_type(node->type)); - if (!(deref = new_var_deref(var, var->loc))) - return NULL; - list_add_after(&assign->node.entry, &deref->node.entry); - record = &deref->node; - } + if (!(assign = make_simple_assignment(var, node, 0))) + return NULL; + list_add_after(&node->entry, &assign->node.entry); + + if (!(deref = new_var_deref(var, var->loc))) + return NULL; + list_add_after(&assign->node.entry, &deref->node.entry); + return &deref->node; +} + +static struct hlsl_ir_deref *new_record_deref(struct hlsl_ir_node *record, + struct hlsl_struct_field *field, const struct source_location loc) +{ + struct hlsl_ir_deref *deref; + + if (!(record = get_var_deref(record))) + return NULL; if (!(deref = d3dcompiler_alloc(sizeof(*deref)))) return NULL; @@ -590,6 +598,49 @@ static struct hlsl_ir_deref *new_record_deref(struct hlsl_ir_node *record, return deref; } +static struct hlsl_ir_deref *new_array_deref(struct hlsl_ir_node *array, + struct hlsl_ir_node *index, const struct source_location loc) +{ + const struct hlsl_type *expr_type = array->data_type; + struct hlsl_ir_deref *deref; + struct hlsl_type *data_type; + + TRACE("Array dereference from type %s\n", debug_hlsl_type(expr_type)); + + if (expr_type->type == HLSL_CLASS_ARRAY) + { + data_type = expr_type->e.array.type; + } + else if (expr_type->type == HLSL_CLASS_MATRIX) + { + data_type = new_hlsl_type(NULL, HLSL_CLASS_VECTOR, expr_type->base_type, expr_type->dimx, 1); + } + else if (expr_type->type == HLSL_CLASS_VECTOR) + { + data_type = new_hlsl_type(NULL, HLSL_CLASS_SCALAR, expr_type->base_type, 1, 1); + } + else + { + if (expr_type->type == HLSL_CLASS_SCALAR) + hlsl_report_message(loc, HLSL_LEVEL_ERROR, "array-indexed expression is scalar"); + else + hlsl_report_message(loc, HLSL_LEVEL_ERROR, "expression is not array-indexable"); + return NULL; + } + + if (!(array = get_var_deref(array))) + return NULL; + + if (!(deref = d3dcompiler_alloc(sizeof(*deref)))) + return NULL; + + init_node(&deref->node, HLSL_IR_DEREF, data_type, loc); + deref->src.type = HLSL_IR_DEREF_ARRAY; + deref->src.v.array.array = array; + deref->src.v.array.index = index; + return deref; +} + static void struct_var_initializer(struct list *list, struct hlsl_ir_var *var, struct parse_initializer *initializer) { @@ -2216,34 +2267,7 @@ postfix_expr: primary_expr /* This may be an array dereference or a vector/matrix * subcomponent access. * We store it as an array dereference in any case. */ - const struct hlsl_type *expr_type = node_from_list($1)->data_type; struct hlsl_ir_deref *deref; - struct hlsl_type *data_type; - - TRACE("Array dereference from type %s\n", debug_hlsl_type(expr_type)); - - if (expr_type->type == HLSL_CLASS_ARRAY) - { - data_type = expr_type->e.array.type; - } - else if (expr_type->type == HLSL_CLASS_MATRIX) - { - data_type = new_hlsl_type(NULL, HLSL_CLASS_VECTOR, expr_type->base_type, expr_type->dimx, 1); - } - else if (expr_type->type == HLSL_CLASS_VECTOR) - { - data_type = new_hlsl_type(NULL, HLSL_CLASS_SCALAR, expr_type->base_type, 1, 1); - } - else - { - if (expr_type->type == HLSL_CLASS_SCALAR) - hlsl_report_message(get_location(&@2), HLSL_LEVEL_ERROR, "array-indexed expression is scalar"); - else - hlsl_report_message(get_location(&@2), HLSL_LEVEL_ERROR, "expression is not array-indexable"); - free_instr_list($1); - free_instr_list($3); - YYABORT; - } if (node_from_list($3)->data_type->type != HLSL_CLASS_SCALAR) { @@ -2253,17 +2277,12 @@ postfix_expr: primary_expr YYABORT; } - if (!(deref = d3dcompiler_alloc(sizeof(*deref)))) + if (!(deref = new_array_deref(node_from_list($1), node_from_list($3), get_location(&@2)))) { free_instr_list($1); free_instr_list($3); YYABORT; } - init_node(&deref->node, HLSL_IR_DEREF, data_type, get_location(&@2)); - deref->src.type = HLSL_IR_DEREF_ARRAY; - deref->src.v.array.array = node_from_list($1); - deref->src.v.array.index = node_from_list($3); - $$ = append_binop($1, $3, &deref->node); } /* "var_modifiers" doesn't make sense in this case, but it's needed -- 2.26.0 From ef617b2727fecba43a00b597e6aca3446d9a6f0d Mon Sep 17 00:00:00 2001 From: Zebediah Figura Date: Mon, 6 Apr 2020 20:06:57 -0500 Subject: [PATCH 102/102] d3dcompiler: Store the array and record fields of struct hlsl_deref as hlsl_deref pointers. Signed-off-by: Zebediah Figura --- dlls/d3dcompiler_43/d3dcompiler_private.h | 5 +- dlls/d3dcompiler_43/hlsl.y | 75 ++++++++++++++++++----- dlls/d3dcompiler_43/utils.c | 49 ++++++--------- 3 files changed, 81 insertions(+), 48 deletions(-) diff --git a/dlls/d3dcompiler_43/d3dcompiler_private.h b/dlls/d3dcompiler_43/d3dcompiler_private.h index d087aed5a80..a2b605be37f 100644 --- a/dlls/d3dcompiler_43/d3dcompiler_private.h +++ b/dlls/d3dcompiler_43/d3dcompiler_private.h @@ -854,12 +854,12 @@ struct hlsl_deref struct hlsl_ir_var *var; struct { - struct hlsl_ir_node *array; + struct hlsl_deref *array; struct hlsl_ir_node *index; } array; struct { - struct hlsl_ir_node *record; + struct hlsl_deref *record; struct hlsl_struct_field *field; } record; } v; @@ -1111,6 +1111,7 @@ void add_function_decl(struct wine_rb_tree *funcs, char *name, struct hlsl_ir_fu BOOL intrinsic) DECLSPEC_HIDDEN; struct bwriter_shader *parse_hlsl_shader(const char *text, enum shader_type type, DWORD major, DWORD minor, const char *entrypoint, char **messages) DECLSPEC_HIDDEN; +BOOL copy_deref(struct hlsl_deref *dst, const struct hlsl_deref *src) DECLSPEC_HIDDEN; const char *debug_base_type(const struct hlsl_type *type) DECLSPEC_HIDDEN; const char *debug_hlsl_type(const struct hlsl_type *type) DECLSPEC_HIDDEN; diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index b1d88571282..fac21f8b267 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -555,15 +555,50 @@ static struct hlsl_ir_deref *new_var_deref(struct hlsl_ir_var *var, const struct return deref; } -static struct hlsl_ir_node *get_var_deref(struct hlsl_ir_node *node) +static struct hlsl_deref *dup_deref(const struct hlsl_deref *src); + +BOOL copy_deref(struct hlsl_deref *dst, const struct hlsl_deref *src) +{ + dst->type = src->type; + if (src->type == HLSL_IR_DEREF_VAR) + { + dst->v.var = src->v.var; + } + else if (src->type == HLSL_IR_DEREF_ARRAY) + { + if (!(dst->v.array.array = dup_deref(src->v.array.array))) + return FALSE; + dst->v.array.index = src->v.array.index; + } + else if (src->type == HLSL_IR_DEREF_RECORD) + { + if (!(dst->v.record.record = dup_deref(src->v.record.record))) + return FALSE; + dst->v.record.field = src->v.record.field; + } + return TRUE; +} + +static struct hlsl_deref *dup_deref(const struct hlsl_deref *src) +{ + struct hlsl_deref *dst; + + if (!(dst = d3dcompiler_alloc(sizeof(*dst)))) + return NULL; + if (copy_deref(dst, src)) + return dst; + d3dcompiler_free(dst); + return NULL; +} + +static struct hlsl_deref *get_var_deref(struct hlsl_ir_node *node) { struct hlsl_ir_assignment *assign; - struct hlsl_ir_deref *deref; + struct hlsl_deref *deref; struct hlsl_ir_var *var; if (node->type == HLSL_IR_DEREF) - return node; - + return dup_deref(&deref_from_node(node)->src); if (!(var = new_synthetic_var(node->data_type, node->loc))) return NULL; @@ -574,26 +609,32 @@ static struct hlsl_ir_node *get_var_deref(struct hlsl_ir_node *node) return NULL; list_add_after(&node->entry, &assign->node.entry); - if (!(deref = new_var_deref(var, var->loc))) + if (!(deref = d3dcompiler_alloc(sizeof(*deref)))) return NULL; - list_add_after(&assign->node.entry, &deref->node.entry); - return &deref->node; + + deref->type = HLSL_IR_DEREF_VAR; + deref->v.var = var; + return deref; } static struct hlsl_ir_deref *new_record_deref(struct hlsl_ir_node *record, struct hlsl_struct_field *field, const struct source_location loc) { struct hlsl_ir_deref *deref; + struct hlsl_deref *src; - if (!(record = get_var_deref(record))) + if (!(deref = d3dcompiler_alloc(sizeof(*deref)))) return NULL; - if (!(deref = d3dcompiler_alloc(sizeof(*deref)))) + if (!(src = get_var_deref(record))) + { + d3dcompiler_free(deref); return NULL; + } init_node(&deref->node, HLSL_IR_DEREF, field->type, loc); deref->src.type = HLSL_IR_DEREF_RECORD; - deref->src.v.record.record = record; + deref->src.v.record.record = src; deref->src.v.record.field = field; return deref; } @@ -604,6 +645,7 @@ static struct hlsl_ir_deref *new_array_deref(struct hlsl_ir_node *array, const struct hlsl_type *expr_type = array->data_type; struct hlsl_ir_deref *deref; struct hlsl_type *data_type; + struct hlsl_deref *src; TRACE("Array dereference from type %s\n", debug_hlsl_type(expr_type)); @@ -628,15 +670,18 @@ static struct hlsl_ir_deref *new_array_deref(struct hlsl_ir_node *array, return NULL; } - if (!(array = get_var_deref(array))) + if (!(deref = d3dcompiler_alloc(sizeof(*deref)))) return NULL; - if (!(deref = d3dcompiler_alloc(sizeof(*deref)))) + if (!(src = get_var_deref(array))) + { + d3dcompiler_free(deref); return NULL; + } init_node(&deref->node, HLSL_IR_DEREF, data_type, loc); deref->src.type = HLSL_IR_DEREF_ARRAY; - deref->src.v.array.array = array; + deref->src.v.array.array = src; deref->src.v.array.index = index; return deref; } @@ -2689,9 +2734,9 @@ static struct hlsl_ir_var *hlsl_var_from_deref(const struct hlsl_deref *deref) 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_deref(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_deref(deref->v.record.record); } assert(0); return NULL; diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c index bbb17ab0b0a..f16b58b76b6 100644 --- a/dlls/d3dcompiler_43/utils.c +++ b/dlls/d3dcompiler_43/utils.c @@ -1413,29 +1413,6 @@ 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) -{ - struct hlsl_ir_deref *deref; - - if (lhs->type != HLSL_IR_DEREF) - { - hlsl_report_message(lhs->loc, HLSL_LEVEL_ERROR, "invalid lvalue"); - return FALSE; - } - - deref = deref_from_node(lhs); - - 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); - - assert(0); - return FALSE; -} - struct hlsl_ir_node *make_assignment(struct hlsl_ir_node *lhs, enum parse_assign_op assign_op, struct hlsl_ir_node *rhs) { @@ -1489,12 +1466,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; @@ -1859,13 +1830,13 @@ static void debug_dump_deref(const struct hlsl_deref *deref) wine_dbg_printf(")"); break; case HLSL_IR_DEREF_ARRAY: - debug_dump_src(deref->v.array.array); + debug_dump_deref(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_src(deref->v.record.record); + debug_dump_deref(deref->v.record.record); wine_dbg_printf(".%s", debugstr_a(deref->v.record.field->name)); break; } @@ -2216,8 +2187,23 @@ static void free_ir_constant(struct hlsl_ir_constant *constant) d3dcompiler_free(constant); } +static void free_deref(struct hlsl_deref *deref) +{ + if (deref->type == HLSL_IR_DEREF_RECORD) + { + free_deref(deref->v.record.record); + d3dcompiler_free(deref->v.record.record); + } + else if (deref->type == HLSL_IR_DEREF_ARRAY) + { + free_deref(deref->v.array.array); + d3dcompiler_free(deref->v.array.array); + } +} + static void free_ir_deref(struct hlsl_ir_deref *deref) { + free_deref(&deref->src); d3dcompiler_free(deref); } @@ -2238,6 +2224,7 @@ static void free_ir_expr(struct hlsl_ir_expr *expr) static void free_ir_assignment(struct hlsl_ir_assignment *assignment) { + free_deref(&assignment->lhs); d3dcompiler_free(assignment); } -- 2.26.0