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

[Francisco Casas]

From: Giovanni Mascellani <gmascellani(a)codeweavers.com>

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>

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

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>
---
 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;
 }
 
diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c
index 90ec6058..f661dbc2 100644
--- a/libs/vkd3d-shader/hlsl_sm1.c
+++ b/libs/vkd3d-shader/hlsl_sm1.c
@@ -663,7 +663,7 @@ static void write_sm1_expr(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b
 static void write_sm1_load(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_node *instr)
 {
     const struct hlsl_ir_load *load = hlsl_ir_load(instr);
-    const struct hlsl_reg reg = hlsl_reg_from_deref(ctx, &load->src, instr->data_type);
+    const struct hlsl_reg reg = hlsl_reg_from_deref(ctx, &load->src);
     struct sm1_instruction sm1_instr =
     {
         .opcode = D3DSIO_MOV,
@@ -707,7 +707,7 @@ static void write_sm1_store(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *
 {
     const struct hlsl_ir_store *store = hlsl_ir_store(instr);
     const struct hlsl_ir_node *rhs = store->rhs.node;
-    const struct hlsl_reg reg = hlsl_reg_from_deref(ctx, &store->lhs, rhs->data_type);
+    const struct hlsl_reg reg = hlsl_reg_from_deref(ctx, &store->lhs);
     struct sm1_instruction sm1_instr =
     {
         .opcode = D3DSIO_MOV,
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 6996bb2d..802db3c1 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -870,6 +870,8 @@ static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *r
 
         if (hlsl_sm4_register_from_semantic(ctx, &var->semantic, false, ®->type, swizzle_type, &has_idx))
         {
+            unsigned int offset = hlsl_offset_from_deref_safe(ctx, deref);
+
             if (has_idx)
             {
                 reg->idx[0] = var->semantic.index;
@@ -877,11 +879,11 @@ static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *r
             }
 
             reg->dim = VKD3D_SM4_DIMENSION_VEC4;
-            *writemask = (1u << data_type->dimx) - 1;
+            *writemask = ((1u << data_type->dimx) - 1) << (offset % 4);
         }
         else
         {
-            struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(ctx, deref, data_type);
+            struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(ctx, deref);
 
             assert(hlsl_reg.allocated);
             reg->type = VKD3D_SM4_RT_INPUT;
@@ -899,6 +901,8 @@ static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *r
 
         if (hlsl_sm4_register_from_semantic(ctx, &var->semantic, true, ®->type, swizzle_type, &has_idx))
         {
+            unsigned int offset = hlsl_offset_from_deref_safe(ctx, deref);
+
             if (has_idx)
             {
                 reg->idx[0] = var->semantic.index;
@@ -909,11 +913,11 @@ static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *r
                 reg->dim = VKD3D_SM4_DIMENSION_SCALAR;
             else
                 reg->dim = VKD3D_SM4_DIMENSION_VEC4;
-            *writemask = (1u << data_type->dimx) - 1;
+            *writemask = ((1u << data_type->dimx) - 1) << (offset % 4);
         }
         else
         {
-            struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(ctx, deref, data_type);
+            struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(ctx, deref);
 
             assert(hlsl_reg.allocated);
             reg->type = VKD3D_SM4_RT_OUTPUT;
@@ -925,7 +929,7 @@ static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *r
     }
     else
     {
-        struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(ctx, deref, data_type);
+        struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(ctx, deref);
 
         assert(hlsl_reg.allocated);
         reg->type = VKD3D_SM4_RT_TEMP;
-- 
2.25.1

[Zebediah Figura (she/her)]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>

[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 <fcasas(a)codeweavers.com> wrote:
> From: Giovanni Mascellani <gmascellani(a)codeweavers.com>
>
> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>
>
> ---
> 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 <fcasas(a)codeweavers.com>
> ---
>  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.