[PATCH 5/5] d3dcompiler: Avoid using 1-dimensional vectors as expression types.

[Matteo Bruni]

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...