[PATCH 0/1] 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.

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

[PATCH 1/1] 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

[Zebediah Figura (＠zfigura)]

I do like this for consistency, yes.

Should we generate index instructions without the implicit zero mipmap level?

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