Re: [PATCH vkd3d v3 1/8] vkd3d-shader/hlsl: Allow offsets for scalar and vector variables.

[Matteo Bruni]

Replying here since I had this typed out already. Also, alas, I failed
again in hitting the send button right away...

On Wed, Mar 2, 2022 at 7:32 PM Francisco Casas  wrote:
> From: Giovanni Mascellani
>
> Signed-off-by: Francisco Casas
>
> v3:
> - In v3, this patch replaces
>   vkd3d-shader/hlsl: Move type_is_single_reg() to hlsl.h and
>   rename it to hlsl_type_uses_writemask().
> This in order to simplify numeric variable initializations later.

It also seems like a good idea in general.
> Signed-off-by: Francisco Casas
>
> libs/vkd3d-shader/hlsl.h         |  3 +--
>  libs/vkd3d-shader/hlsl_codegen.c | 22 +++++-----------------
>  libs/vkd3d-shader/hlsl_sm1.c     |  4 ++--
>  libs/vkd3d-shader/hlsl_sm4.c     | 14 +++++++++-----
>  4 files changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
> index 243ed72b..1fd5dc58 100644
> --- a/libs/vkd3d-shader/hlsl.h
> +++ b/libs/vkd3d-shader/hlsl.h
> @@ -793,8 +793,7 @@ unsigned int hlsl_swizzle_from_writemask(unsigned int writemask);
>
> bool hlsl_offset_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref, unsigned int *offset);
>  unsigned int hlsl_offset_from_deref_safe(struct hlsl_ctx *ctx, const struct hlsl_deref *deref);
> -struct hlsl_reg hlsl_reg_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref,
> -
>    const struct hlsl_type *type);
>
> +struct hlsl_reg hlsl_reg_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref);
>
> bool hlsl_fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context);
>
> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> index 837d39e3..d25e0ff5 100644
> --- a/libs/vkd3d-shader/hlsl_codegen.c
> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> @@ -1542,11 +1542,6 @@ static void allocate_objects(struct hlsl_ctx *ctx, enum hlsl_base_type type)
>      }
>  }
>
> -static bool type_is_single_reg(const struct hlsl_type *type)
> -{
> - return type->type == HLSL_CLASS_SCALAR || type->type == HLSL_CLASS_VECTOR;
>
> -}
> -
>
> bool hlsl_offset_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref, unsigned int *offset)
>  {
>      struct hlsl_ir_node *offset_node = deref->offset.node;
> @@ -1589,8 +1584,7 @@ unsigned int hlsl_offset_from_deref_safe(struct hlsl_ctx *ctx, const struct hlsl
>      return 0;
>  }
>
> -struct hlsl_reg hlsl_reg_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref,
> -
>    const struct hlsl_type *type)
>
> +struct hlsl_reg hlsl_reg_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref)
>  {
>      const struct hlsl_ir_var *var = deref->var;
>      struct hlsl_reg ret = var->reg;
> @@ -1598,16 +1592,10 @@ struct hlsl_reg hlsl_reg_from_deref(struct hlsl_ctx *ctx, const struct hlsl_dere
>  ret.id += offset / 4;
>
> - if (type_is_single_reg(var->data_type))
> - {
> -
>    assert(!offset);
> -
>    ret.writemask = var->reg.writemask;
> - }
> - else
> - {
> -
>    assert(type_is_single_reg(type));
> -
>    ret.writemask = ((1 << type->dimx) - 1) << (offset % 4);
> - }
> - ret.writemask = 0xf & (0xf << (offset % 4));
> - if (var->reg.writemask)
> -
>    ret.writemask = hlsl_combine_writemasks(var->reg.writemask, ret.writemask);
> - return ret;
>
> }

I guess an alternative might be to get rid of writemasks entirely, at
least at this "IR level". We'd split the instructions as needed to
always store into contiguous components and otherwise use the type to
figure out the size. The only downside would be that a coalescing pass
either loses some power or needs to happen on an IR with reintroduced
writemasks.

Just food for thought. This is certainly fine for the time being.