[PATCH 7/7] d3dcompiler: Store the "array" field in struct hlsl_deref as a struct hlsl_deref pointer.
Zebediah Figura
zfigura at codeweavers.com
Wed Mar 11 14:39:10 CDT 2020
On 3/11/20 12:57 PM, Matteo Bruni wrote:
> On Sat, Mar 7, 2020 at 12:25 AM Zebediah Figura <z.figura12 at gmail.com> wrote:
>>
>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
>> ---
>> 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.
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().
More information about the wine-devel
mailing list