[PATCH 5/5] d3dcompiler: Avoid using 1-dimensional vectors as expression types.
Zebediah Figura
zfigura at codeweavers.com
Fri Jun 26 12:14:12 CDT 2020
On 6/26/20 11:28 AM, Matteo Bruni wrote:
> On Fri, Jun 26, 2020 at 6:02 PM Zebediah Figura <zfigura at codeweavers.com> wrote:
>>
>> On 6/26/20 10:56 AM, Matteo Bruni wrote:
>>> On Tue, Jun 23, 2020 at 12:48 AM Zebediah Figura <z.figura12 at gmail.com> wrote:
>>>>
>>>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
>>>> ---
>>>> dlls/d3dcompiler_43/hlsl.y | 19 +++++++++++++++----
>>>> dlls/d3dcompiler_43/utils.c | 11 ++++++++---
>>>> 2 files changed, 23 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y
>>>> index 21828337939..27aab195b25 100644
>>>> --- a/dlls/d3dcompiler_43/hlsl.y
>>>> +++ b/dlls/d3dcompiler_43/hlsl.y
>>>> @@ -416,11 +416,17 @@ static struct hlsl_ir_swizzle *new_swizzle(DWORD s, unsigned int components,
>>>> struct hlsl_ir_node *val, struct source_location *loc)
>>>> {
>>>> struct hlsl_ir_swizzle *swizzle = d3dcompiler_alloc(sizeof(*swizzle));
>>>> + struct hlsl_type *data_type;
>>>>
>>>> if (!swizzle)
>>>> return NULL;
>>>> - init_node(&swizzle->node, HLSL_IR_SWIZZLE,
>>>> - new_hlsl_type(NULL, HLSL_CLASS_VECTOR, val->data_type->base_type, components, 1), *loc);
>>>> +
>>>> + if (components == 1)
>>>> + data_type = hlsl_ctx.builtin_types.scalar[val->data_type->base_type];
>>>> + else
>>>> + data_type = hlsl_ctx.builtin_types.vector[val->data_type->base_type][components - 1];
>>>> +
>>>> + init_node(&swizzle->node, HLSL_IR_SWIZZLE, data_type, *loc);
>>>> swizzle->val = val;
>>>> swizzle->swizzle = s;
>>>> return swizzle;
>>>> @@ -2488,6 +2494,7 @@ postfix_expr: primary_expr
>>>> for (i = 0; i < $4.args_count; ++i)
>>>> {
>>>> struct hlsl_ir_node *arg = $4.args[i];
>>>> + struct hlsl_type *data_type;
>>>> unsigned int width;
>>>>
>>>> if (arg->data_type->type == HLSL_CLASS_OBJECT)
>>>> @@ -2504,8 +2511,12 @@ postfix_expr: primary_expr
>>>> continue;
>>>> }
>>>>
>>>> - if (!(arg = add_implicit_conversion($4.instrs, arg,
>>>> - hlsl_ctx.builtin_types.vector[$2->base_type][width - 1], &arg->loc)))
>>>> + if (width == 1)
>>>> + data_type = hlsl_ctx.builtin_types.scalar[$2->base_type];
>>>> + else
>>>> + data_type = hlsl_ctx.builtin_types.vector[$2->base_type][width - 1];
>>>> +
>>>> + if (!(arg = add_implicit_conversion($4.instrs, arg, data_type, &arg->loc)))
>>>> continue;
>>>>
>>>> if (!(assignment = new_assignment(var, NULL, arg,
>>>> diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c
>>>> index 30aa9de1dd4..4f0556103ab 100644
>>>> --- a/dlls/d3dcompiler_43/utils.c
>>>> +++ b/dlls/d3dcompiler_43/utils.c
>>>> @@ -1294,7 +1294,7 @@ static struct hlsl_type *expr_common_type(struct hlsl_type *t1, struct hlsl_type
>>>> }
>>>> }
>>>>
>>>> - if (type == HLSL_CLASS_SCALAR)
>>>> + if (type == HLSL_CLASS_SCALAR || (type == HLSL_CLASS_VECTOR && dimx == 1))
>>>> return hlsl_ctx.builtin_types.scalar[base];
>>>> if (type == HLSL_CLASS_VECTOR)
>>>> return hlsl_ctx.builtin_types.vector[base][dimx - 1];
>>>> @@ -1496,9 +1496,14 @@ struct hlsl_ir_node *add_assignment(struct list *instrs, struct hlsl_ir_node *lh
>>>> d3dcompiler_free(assign);
>>>> return NULL;
>>>> }
>>>> - assert(swizzle_type->type == HLSL_CLASS_VECTOR);
>>>> + assert(swizzle_type->type == HLSL_CLASS_VECTOR || swizzle_type->type == HLSL_CLASS_SCALAR);
>>>> if (swizzle_type->dimx != width)
>>>> - swizzle->node.data_type = hlsl_ctx.builtin_types.vector[swizzle_type->base_type][width - 1];
>>>> + {
>>>> + if (width == 1)
>>>> + swizzle->node.data_type = hlsl_ctx.builtin_types.scalar[swizzle_type->base_type];
>>>> + else
>>>> + swizzle->node.data_type = hlsl_ctx.builtin_types.vector[swizzle_type->base_type][width - 1];
>>>> + }
>>>> rhs = &swizzle->node;
>>>> }
>>>> else
>>>
>>> What do we gain with this?
>>>
>>
>> Not having to deal with vec1 at codegen time, basically, or along
>> similar lines not having superfluous instructions to cast it to scalar.
>> Granted, it still exists, but you'd have to use it intentionally...
>
> My main objection is that, IIRC, we need to keep the distinction
> anyway for uniform variables (e.g. I guess it's visible in the
> constant table). At that point we could just keep it like that all the
> way through the compiler.
> WRT casts, they can be avoided by figuring out at "codegen" that they
> are actually the same type. Again, we probably need something along
> those lines anyway since, especially in SM1, we have to map pretty
> much everything to float eventually.
>
> Notice that I can still be convinced that this patch goes in the right
> direction...
>
It gets a little tricky, I think. Uniforms are one case and pretty
clear-cut, i.e. you declare something as "float" or "float1".
Arbitrary expressions are another. The language *does* seem to care
about these in at least one way I could find: you can index a vector
using [0], but not a scalar. According to this criterion, both swizzles
and expression argument coercion count as scalars rather than vectors.
I.e. the code
uniform float1 v;
return v.x[0];
and
uniform float1 v;
uniform float s;
return (v + s)[0];
will both throw an error.
The third case in the patch above, of course, can't be tested, as it
only has internal effect.
Certainly I agree that nothing after parse time should care about the
difference between float and float1, though. I guess we want to reduce
any such generated casts using a copy-prop pass.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200626/b67b5086/attachment.sig>
More information about the wine-devel
mailing list