[PATCH v2 0/2] MR182: vkd3d-shader/hlsl: Introduce hlsl_ir_index, part 2/2 (optional).

[Francisco Casas (＠fcasas)]

This patch makes index expressions on resources hlsl_ir_index nodes
instead of hlsl_ir_resource_load nodes, because it is not known if they
will be used later as the lhs of an hlsl_ir_resource_store.

For now, the only benefit is consistency.

--
  v2: vkd3d-shader/hlsl: Don't keep the implicit mipmap level on hlsl_ir_index.
 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/182

[PATCH v2 1/2] vkd3d-shader/hlsl: Use hlsl_ir_index for resource access.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

This patch makes index expressions on resources hlsl_ir_index nodes
instead of hlsl_ir_resource_load nodes, because it is not known if they
will be used later as the lhs of an hlsl_ir_resource_store.

For now, the only benefit is consistency.
---
 libs/vkd3d-shader/hlsl.c         |  5 ++++
 libs/vkd3d-shader/hlsl.h         |  1 +
 libs/vkd3d-shader/hlsl.y         | 42 +++++++++++++++++---------------
 libs/vkd3d-shader/hlsl_codegen.c | 20 ++++++++++++++-
 4 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index 32511590..5c271ed4 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -1362,6 +1362,11 @@ bool hlsl_index_is_noncontiguous(struct hlsl_ir_index *index)
     return type->class == HLSL_CLASS_MATRIX && !hlsl_type_is_row_major(type);
 }
 
+bool hlsl_index_is_resource_access(struct hlsl_ir_index *index)
+{
+    return index->val.node->data_type->class == HLSL_CLASS_OBJECT;
+}
+
 struct hlsl_ir_node *hlsl_new_index(struct hlsl_ctx *ctx, struct hlsl_ir_node *val,
         struct hlsl_ir_node *idx, const struct vkd3d_shader_location *loc)
 {
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
index 974252d4..1d02d7a3 100644
--- a/libs/vkd3d-shader/hlsl.h
+++ b/libs/vkd3d-shader/hlsl.h
@@ -1091,6 +1091,7 @@ struct hlsl_ir_store *hlsl_new_store_component(struct hlsl_ctx *ctx, struct hlsl
         const struct hlsl_deref *lhs, unsigned int comp, struct hlsl_ir_node *rhs);
 
 bool hlsl_index_is_noncontiguous(struct hlsl_ir_index *index);
+bool hlsl_index_is_resource_access(struct hlsl_ir_index *index);
 
 struct hlsl_ir_node *hlsl_new_index(struct hlsl_ctx *ctx, struct hlsl_ir_node *val,
         struct hlsl_ir_node *idx, const struct vkd3d_shader_location *loc);
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 194d21f4..13d2aab0 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -759,9 +759,7 @@ static bool add_array_access(struct hlsl_ctx *ctx, struct list *instrs, struct h
             && (expr_type->base_type == HLSL_TYPE_TEXTURE || expr_type->base_type == HLSL_TYPE_UAV)
             && expr_type->sampler_dim != HLSL_SAMPLER_DIM_GENERIC)
     {
-        struct hlsl_resource_load_params load_params = {.type = HLSL_RESOURCE_LOAD};
         unsigned int dim_count = hlsl_sampler_dim_count(expr_type->sampler_dim);
-        struct hlsl_ir_resource_load *resource_load;
 
         if (index_type->class > HLSL_CLASS_VECTOR || index_type->dimx != dim_count)
         {
@@ -781,13 +779,10 @@ static bool add_array_access(struct hlsl_ctx *ctx, struct list *instrs, struct h
         if (!(index = add_zero_mipmap_level(ctx, instrs, index, dim_count, loc)))
             return false;
 
-        load_params.format = expr_type->e.resource_format;
-        load_params.resource = array;
-        load_params.coords = index;
-
-        if (!(resource_load = hlsl_new_resource_load(ctx, &load_params, loc)))
+        if (!(return_index = hlsl_new_index(ctx, array, index, loc)))
             return false;
-        list_add_tail(instrs, &resource_load->node.entry);
+        list_add_tail(instrs, &return_index->entry);
+
         return true;
     }
 
@@ -1742,7 +1737,7 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in
     if (!(rhs = add_implicit_conversion(ctx, instrs, rhs, lhs_type, &rhs->loc)))
         return NULL;
 
-    while (lhs->type != HLSL_IR_LOAD && lhs->type != HLSL_IR_RESOURCE_LOAD && lhs->type != HLSL_IR_INDEX)
+    while (lhs->type != HLSL_IR_LOAD && lhs->type != HLSL_IR_INDEX)
     {
         if (lhs->type == HLSL_IR_EXPR && hlsl_ir_expr(lhs)->op == HLSL_OP1_CAST)
         {
@@ -1779,17 +1774,19 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in
         }
     }
 
-    if (lhs->type == HLSL_IR_RESOURCE_LOAD)
+    if (lhs->type == HLSL_IR_INDEX && hlsl_index_is_resource_access(hlsl_ir_index(lhs)))
     {
-        struct hlsl_ir_resource_load *load = hlsl_ir_resource_load(lhs);
+        struct hlsl_ir_node *load_coords = hlsl_ir_index(lhs)->idx.node;
         struct hlsl_ir_resource_store *store;
+        struct hlsl_deref resource_deref;
         struct hlsl_type *resource_type;
         struct hlsl_ir_swizzle *coords;
         unsigned int dim_count;
 
-        /* Such an lvalue was produced by an index expression. */
-        assert(load->load_type == HLSL_RESOURCE_LOAD);
-        resource_type = hlsl_deref_get_type(ctx, &load->resource);
+        if (!hlsl_init_deref_from_index_chain(ctx, &resource_deref, hlsl_ir_index(lhs)->val.node))
+            return NULL;
+
+        resource_type = hlsl_deref_get_type(ctx, &resource_deref);
         assert(resource_type->class == HLSL_CLASS_OBJECT);
         assert(resource_type->base_type == HLSL_TYPE_TEXTURE || resource_type->base_type == HLSL_TYPE_UAV);
 
@@ -1804,16 +1801,23 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in
                     "Resource store expressions must write to all components.");
 
         /* Remove the (implicit) mipmap level from the load expression. */
-        assert(load->coords.node->data_type->class == HLSL_CLASS_VECTOR);
-        assert(load->coords.node->data_type->base_type == HLSL_TYPE_UINT);
-        assert(load->coords.node->data_type->dimx == dim_count + 1);
-        if (!(coords = hlsl_new_swizzle(ctx, HLSL_SWIZZLE(X, Y, Z, W), dim_count, load->coords.node, &lhs->loc)))
+        assert(load_coords->data_type->class == HLSL_CLASS_VECTOR);
+        assert(load_coords->data_type->base_type == HLSL_TYPE_UINT);
+        assert(load_coords->data_type->dimx == dim_count + 1);
+        if (!(coords = hlsl_new_swizzle(ctx, HLSL_SWIZZLE(X, Y, Z, W), dim_count, load_coords, &lhs->loc)))
+        {
+            hlsl_cleanup_deref(&resource_deref);
             return NULL;
+        }
         list_add_tail(instrs, &coords->node.entry);
 
-        if (!(store = hlsl_new_resource_store(ctx, &load->resource, &coords->node, rhs, &lhs->loc)))
+        if (!(store = hlsl_new_resource_store(ctx, &resource_deref, &coords->node, rhs, &lhs->loc)))
+        {
+            hlsl_cleanup_deref(&resource_deref);
             return NULL;
+        }
         list_add_tail(instrs, &store->node.entry);
+        hlsl_cleanup_deref(&resource_deref);
     }
     else if (lhs->type == HLSL_IR_INDEX && hlsl_index_is_noncontiguous(hlsl_ir_index(lhs)))
     {
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 42f8ab3b..3ee173b3 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -736,7 +736,8 @@ static bool lower_calls(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *
  * record access before knowing if they will be used in the lhs of an assignment --in which case
  * they are lowered into a deref-- or as the load of an element within a larger value.
  * For the latter case, this pass takes care of lowering hlsl_ir_indexes into individual
- * hlsl_ir_loads. */
+ * hlsl_ir_loads, or individual hlsl_ir_resource_loads, in case the indexing is a
+ * resource access. */
 static bool lower_index_loads(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context)
 {
     struct hlsl_deref var_deref;
@@ -751,6 +752,23 @@ static bool lower_index_loads(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr,
     index = hlsl_ir_index(instr);
     val = index->val.node;
 
+    if (hlsl_index_is_resource_access(index))
+    {
+        struct hlsl_resource_load_params params = {0};
+        struct hlsl_ir_resource_load *load;
+
+        params.type = HLSL_RESOURCE_LOAD;
+        params.resource = val;
+        params.coords = index->idx.node;
+        params.format = val->data_type->e.resource_format;
+
+        if (!(load = hlsl_new_resource_load(ctx, ¶ms, &instr->loc)))
+            return false;
+        list_add_before(&instr->entry, &load->node.entry);
+        hlsl_replace_node(instr, &load->node);
+        return true;
+    }
+
     if (!(var = hlsl_new_synthetic_var(ctx, "index-val", val->data_type, &instr->loc)))
         return false;
     hlsl_init_simple_deref_from_var(&var_deref, var);
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/182

[PATCH v2 2/2] vkd3d-shader/hlsl: Don't keep the implicit mipmap level on hlsl_ir_index.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

---
 libs/vkd3d-shader/hlsl.y         | 54 +++-----------------------------
 libs/vkd3d-shader/hlsl_codegen.c | 47 ++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 50 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 13d2aab0..f58a8cda 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -716,39 +716,6 @@ static struct hlsl_ir_node *add_binary_arithmetic_expr(struct hlsl_ctx *ctx, str
         enum hlsl_ir_expr_op op, struct hlsl_ir_node *arg1, struct hlsl_ir_node *arg2,
         const struct vkd3d_shader_location *loc);
 
-static struct hlsl_ir_node *add_zero_mipmap_level(struct hlsl_ctx *ctx, struct list *instrs,
-        struct hlsl_ir_node *index, unsigned int dim_count, const struct vkd3d_shader_location *loc)
-{
-    struct hlsl_ir_load *coords_load;
-    struct hlsl_deref coords_deref;
-    struct hlsl_ir_constant *zero;
-    struct hlsl_ir_store *store;
-    struct hlsl_ir_var *coords;
-
-    if (!(coords = hlsl_new_synthetic_var(ctx, "coords",
-            hlsl_get_vector_type(ctx, HLSL_TYPE_UINT, dim_count + 1), loc)))
-        return NULL;
-
-    hlsl_init_simple_deref_from_var(&coords_deref, coords);
-    if (!(store = hlsl_new_store_index(ctx, &coords_deref, NULL, index, (1u << dim_count) - 1, loc)))
-        return NULL;
-    list_add_tail(instrs, &store->node.entry);
-
-    if (!(zero = hlsl_new_uint_constant(ctx, 0, loc)))
-        return NULL;
-    list_add_tail(instrs, &zero->node.entry);
-
-    if (!(store = hlsl_new_store_index(ctx, &coords_deref, NULL, &zero->node, 1u << dim_count, loc)))
-        return NULL;
-    list_add_tail(instrs, &store->node.entry);
-
-    if (!(coords_load = hlsl_new_var_load(ctx, coords, loc)))
-        return NULL;
-    list_add_tail(instrs, &coords_load->node.entry);
-
-    return &coords_load->node;
-}
-
 static bool add_array_access(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *array,
         struct hlsl_ir_node *index, const struct vkd3d_shader_location *loc)
 {
@@ -776,9 +743,6 @@ static bool add_array_access(struct hlsl_ctx *ctx, struct list *instrs, struct h
                 hlsl_get_vector_type(ctx, HLSL_TYPE_UINT, dim_count), &index->loc)))
             return false;
 
-        if (!(index = add_zero_mipmap_level(ctx, instrs, index, dim_count, loc)))
-            return false;
-
         if (!(return_index = hlsl_new_index(ctx, array, index, loc)))
             return false;
         list_add_tail(instrs, &return_index->entry);
@@ -1776,11 +1740,10 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in
 
     if (lhs->type == HLSL_IR_INDEX && hlsl_index_is_resource_access(hlsl_ir_index(lhs)))
     {
-        struct hlsl_ir_node *load_coords = hlsl_ir_index(lhs)->idx.node;
+        struct hlsl_ir_node *coords = hlsl_ir_index(lhs)->idx.node;
         struct hlsl_ir_resource_store *store;
         struct hlsl_deref resource_deref;
         struct hlsl_type *resource_type;
-        struct hlsl_ir_swizzle *coords;
         unsigned int dim_count;
 
         if (!hlsl_init_deref_from_index_chain(ctx, &resource_deref, hlsl_ir_index(lhs)->val.node))
@@ -1800,18 +1763,11 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in
             hlsl_error(ctx, &lhs->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_WRITEMASK,
                     "Resource store expressions must write to all components.");
 
-        /* Remove the (implicit) mipmap level from the load expression. */
-        assert(load_coords->data_type->class == HLSL_CLASS_VECTOR);
-        assert(load_coords->data_type->base_type == HLSL_TYPE_UINT);
-        assert(load_coords->data_type->dimx == dim_count + 1);
-        if (!(coords = hlsl_new_swizzle(ctx, HLSL_SWIZZLE(X, Y, Z, W), dim_count, load_coords, &lhs->loc)))
-        {
-            hlsl_cleanup_deref(&resource_deref);
-            return NULL;
-        }
-        list_add_tail(instrs, &coords->node.entry);
+        assert(coords->data_type->class == HLSL_CLASS_VECTOR);
+        assert(coords->data_type->base_type == HLSL_TYPE_UINT);
+        assert(coords->data_type->dimx == dim_count);
 
-        if (!(store = hlsl_new_resource_store(ctx, &resource_deref, &coords->node, rhs, &lhs->loc)))
+        if (!(store = hlsl_new_resource_store(ctx, &resource_deref, coords, rhs, &lhs->loc)))
         {
             hlsl_cleanup_deref(&resource_deref);
             return NULL;
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 3ee173b3..3536e8cc 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -732,6 +732,42 @@ static bool lower_calls(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *
     return true;
 }
 
+static struct hlsl_ir_node *add_zero_mipmap_level(struct hlsl_ctx *ctx, struct hlsl_ir_node *index,
+        const struct vkd3d_shader_location *loc)
+{
+    unsigned int dim_count = index->data_type->dimx;
+    struct hlsl_ir_load *coords_load;
+    struct hlsl_deref coords_deref;
+    struct hlsl_ir_constant *zero;
+    struct hlsl_ir_store *store;
+    struct hlsl_ir_var *coords;
+
+    assert(dim_count < 4);
+
+    if (!(coords = hlsl_new_synthetic_var(ctx, "coords",
+            hlsl_get_vector_type(ctx, HLSL_TYPE_UINT, dim_count + 1), loc)))
+        return NULL;
+
+    hlsl_init_simple_deref_from_var(&coords_deref, coords);
+    if (!(store = hlsl_new_store_index(ctx, &coords_deref, NULL, index, (1u << dim_count) - 1, loc)))
+        return NULL;
+    list_add_after(&index->entry, &store->node.entry);
+
+    if (!(zero = hlsl_new_uint_constant(ctx, 0, loc)))
+        return NULL;
+    list_add_after(&store->node.entry, &zero->node.entry);
+
+    if (!(store = hlsl_new_store_index(ctx, &coords_deref, NULL, &zero->node, 1u << dim_count, loc)))
+        return NULL;
+    list_add_after(&zero->node.entry, &store->node.entry);
+
+    if (!(coords_load = hlsl_new_var_load(ctx, coords, loc)))
+        return NULL;
+    list_add_after(&store->node.entry, &coords_load->node.entry);
+
+    return &coords_load->node;
+}
+
 /* hlsl_ir_index nodes are a parse-time construct used to represent array indexing and struct
  * record access before knowing if they will be used in the lhs of an assignment --in which case
  * they are lowered into a deref-- or as the load of an element within a larger value.
@@ -754,12 +790,21 @@ static bool lower_index_loads(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr,
 
     if (hlsl_index_is_resource_access(index))
     {
+        unsigned int dim_count = hlsl_sampler_dim_count(val->data_type->sampler_dim);
+        struct hlsl_ir_node *coords = index->idx.node;
         struct hlsl_resource_load_params params = {0};
         struct hlsl_ir_resource_load *load;
 
+        assert(coords->data_type->class == HLSL_CLASS_VECTOR);
+        assert(coords->data_type->base_type == HLSL_TYPE_UINT);
+        assert(coords->data_type->dimx == dim_count);
+
+        if (!(coords = add_zero_mipmap_level(ctx, coords, &instr->loc)))
+            return false;
+
         params.type = HLSL_RESOURCE_LOAD;
         params.resource = val;
-        params.coords = index->idx.node;
+        params.coords = coords;
         params.format = val->data_type->e.resource_format;
 
         if (!(load = hlsl_new_resource_load(ctx, ¶ms, &instr->loc)))
-- 
GitLab

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/182

[Francisco Casas (＠fcasas)]

On Fri Apr 28 17:13:29 2023 +0000, Zebediah Figura wrote:
> I do like this for consistency, yes.
> Should we generate index instructions without the implicit zero mipmap level?
> I think it is a good idea. It saves us from having to remove it on resource stores.

I added a second patch for that purpose.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/182#note_31528

[PATCH v2 0/2] MR182: vkd3d-shader/hlsl: Introduce hlsl_ir_index, part 2/2 (optional). - approved

[Zebediah Figura (＠zfigura)]

This merge request was approved by Zebediah Figura.
-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/182

[PATCH v2 0/2] MR182: vkd3d-shader/hlsl: Introduce hlsl_ir_index, part 2/2 (optional). - approved

[Giovanni Mascellani (＠giomasce)]

This merge request was approved by Giovanni Mascellani.
-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/182