Re: [PATCH vkd3d 3/6] vkd3d-shader/hlsl: Lower scalar-to-vector casts to swizzles.

[Matteo Bruni]

On Mon, Nov 29, 2021 at 6:06 PM Zebediah Figura  wrote:
> On 11/26/21 13:37, Matteo Bruni wrote:
> > On Tue, Nov 23, 2021 at 2:46 AM Zebediah Figura  wrote:
> > > Signed-off-by: Zebediah Figura
> > >
> > > Makefile.am                      |  1 -
> > >   libs/vkd3d-shader/hlsl_codegen.c | 33 ++++++++++++++++++++++++++++++++
> > >   2 files changed, 33 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index be3d8ec15..4010533af 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -290,7 +290,6 @@ XFAIL_TESTS = \
> > >          tests/conditional.shader_test \
> > >          tests/hlsl-array-dimension.shader_test \
> > >          tests/hlsl-bool-cast.shader_test \
> > > -
> > >   tests/hlsl-comma.shader_test \
> > >    tests/hlsl-cross.shader_test \
> > >    tests/hlsl-duplicate-modifiers.shader_test \
> > >    tests/hlsl-for.shader_test \
> > >
> > > diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> > > index 2c620c7d7..b69df47a0 100644
> > > --- a/libs/vkd3d-shader/hlsl_codegen.c
> > > +++ b/libs/vkd3d-shader/hlsl_codegen.c
> > > @@ -242,6 +242,38 @@ static void replace_and_remove_node(struct hlsl_ir_node *old, struct hlsl_ir_nod
> > >       hlsl_free_instr(old);
> > >   }
> > >
> > > +/* Lower casts from vec1 to vecN to swizzles. */
> > > +static bool lower_broadcasts(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context)
> > > +{
> > > - const struct hlsl_type *src_type, *dst_type;
> > > - struct hlsl_ir_expr *cast;
> > > -
> > > - if (instr->type != HLSL_IR_EXPR)
> > > -
> > >    return false;
> > > - cast = hlsl_ir_expr(instr);
> > > - src_type = cast->operands[0].node->data_type;
> > > - dst_type = cast->node.data_type;
> > > -
> > > - if (cast->op == HLSL_OP1_CAST
> > > -
> > >        && src_type->type <= HLSL_CLASS_VECTOR && dst_type->type <= HLSL_CLASS_VECTOR
> > > -
> > >        && src_type->dimx == 1)
> > > - {
> > > -
> > >    struct hlsl_ir_swizzle *swizzle;
> > > -
> > > -
> > >    if (!(swizzle = hlsl_new_swizzle(ctx, HLSL_SWIZZLE(X, X, X, X), dst_type->dimx, &cast->node, &cast->node.loc)))
> > > -
> > >        return false;
> > > -
> > >    list_add_after(&cast->node.entry, &swizzle->node.entry);
> > > -
> > > -
> > >    cast->node.data_type = hlsl_get_scalar_type(ctx, dst_type->base_type);
> > > -
> > >    replace_node(&cast->node, &swizzle->node);
> > > -
> > >    hlsl_src_remove(&swizzle->val);
> > > -
> > >    hlsl_src_from_node(&swizzle->val, &cast->node);
> >
> > This seems a bit opaque to me. What do you think about the attached
> > change on top of this patch?
>
> Unlike Giovanni I do think that helps :-)
>
> Although at this point I'm more and more inclined to agree with
> Giovanni's original suggestion, i.e. just create a new node anyway, and
> get rid of patch 2/6.

Seems fair. Let's see how bad the rebase will be...