[PATCH v2 0/3] MR574: vkd3d-shader/ir: Refactor VSIR passes.

[Giovanni Mascellani (＠giomasce)]

--
  v2: vkd3d-shader/ir: Remove DCL_TEMPS instructions.
 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/574

[PATCH v2 1/3] vkd3d-shader/ir: Refactor return code checking in vkd3d_shader_normalise().

[Conor McCarthy]

From: Conor McCarthy <cmccarthy(a)codeweavers.com>

We started with only one or two of these but it has become excessive.

Patch originally written by Conor McCarthy and updated by Giovanni Mascellani.
---
 libs/vkd3d-shader/ir.c | 47 +++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c
index d6978171b..d4cd0fa14 100644
--- a/libs/vkd3d-shader/ir.c
+++ b/libs/vkd3d-shader/ir.c
@@ -2236,38 +2236,43 @@ enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser,
     if (parser->shader_desc.is_dxil)
         return result;
 
-    if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL
-            && (result = remap_output_signature(parser, compile_info)) < 0)
-        return result;
+    if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL)
+    {
+        if ((result = remap_output_signature(parser, compile_info)) < 0)
+            return result;
+    }
 
-    if (parser->shader_version.type == VKD3D_SHADER_TYPE_HULL
-            && (result = instruction_array_flatten_hull_shader_phases(instructions)) >= 0)
+    if (parser->shader_version.type == VKD3D_SHADER_TYPE_HULL)
     {
-        result = instruction_array_normalise_hull_shader_control_point_io(instructions,
-                &parser->shader_desc.input_signature);
+        if ((result = instruction_array_flatten_hull_shader_phases(instructions)) < 0)
+            return result;
+
+        if ((result = instruction_array_normalise_hull_shader_control_point_io(instructions,
+                &parser->shader_desc.input_signature)) < 0)
+            return result;
     }
-    if (result >= 0)
-        result = shader_normalise_io_registers(parser);
 
-    if (result >= 0)
-        result = instruction_array_normalise_flat_constants(parser);
+    if ((result = shader_normalise_io_registers(parser)) < 0)
+        return result;
 
-    if (result >= 0)
-        remove_dead_code(parser);
+    if ((result = instruction_array_normalise_flat_constants(parser)) < 0)
+        return result;
 
-    if (result >= 0)
-        result = flatten_control_flow_constructs(parser);
+    remove_dead_code(parser);
 
-    if (result >= 0)
-        result = normalise_combined_samplers(parser);
+    if ((result = flatten_control_flow_constructs(parser)) < 0)
+        return result;
 
-    if (result >= 0 && TRACE_ON())
+    if ((result = normalise_combined_samplers(parser)) < 0)
+        return result;
+
+    if (TRACE_ON())
         vkd3d_shader_trace(instructions, &parser->shader_version);
 
-    if (result >= 0 && !parser->failed)
-        result = vsir_validate(parser);
+    if (!parser->failed && (result = vsir_validate(parser)) < 0)
+        return result;
 
-    if (result >= 0 && parser->failed)
+    if (parser->failed)
         result = VKD3D_ERROR_INVALID_SHADER;
 
     return result;
-- 
GitLab


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

[PATCH v2 2/3] vkd3d-shader/ir: Run tracing and validation for DXIL code too.

[Giovanni Mascellani]

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

---
 libs/vkd3d-shader/ir.c | 48 +++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c
index d4cd0fa14..57783de68 100644
--- a/libs/vkd3d-shader/ir.c
+++ b/libs/vkd3d-shader/ir.c
@@ -2233,38 +2233,38 @@ enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser,
     struct vkd3d_shader_instruction_array *instructions = &parser->instructions;
     enum vkd3d_result result = VKD3D_OK;
 
-    if (parser->shader_desc.is_dxil)
-        return result;
-
-    if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL)
+    if (!parser->shader_desc.is_dxil)
     {
-        if ((result = remap_output_signature(parser, compile_info)) < 0)
-            return result;
-    }
+        if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL)
+        {
+            if ((result = remap_output_signature(parser, compile_info)) < 0)
+                return result;
+        }
 
-    if (parser->shader_version.type == VKD3D_SHADER_TYPE_HULL)
-    {
-        if ((result = instruction_array_flatten_hull_shader_phases(instructions)) < 0)
-            return result;
+        if (parser->shader_version.type == VKD3D_SHADER_TYPE_HULL)
+        {
+            if ((result = instruction_array_flatten_hull_shader_phases(instructions)) < 0)
+                return result;
 
-        if ((result = instruction_array_normalise_hull_shader_control_point_io(instructions,
-                &parser->shader_desc.input_signature)) < 0)
-            return result;
-    }
+            if ((result = instruction_array_normalise_hull_shader_control_point_io(instructions,
+                    &parser->shader_desc.input_signature)) < 0)
+                return result;
+        }
 
-    if ((result = shader_normalise_io_registers(parser)) < 0)
-        return result;
+        if ((result = shader_normalise_io_registers(parser)) < 0)
+            return result;
 
-    if ((result = instruction_array_normalise_flat_constants(parser)) < 0)
-        return result;
+        if ((result = instruction_array_normalise_flat_constants(parser)) < 0)
+            return result;
 
-    remove_dead_code(parser);
+        remove_dead_code(parser);
 
-    if ((result = flatten_control_flow_constructs(parser)) < 0)
-        return result;
+        if ((result = flatten_control_flow_constructs(parser)) < 0)
+            return result;
 
-    if ((result = normalise_combined_samplers(parser)) < 0)
-        return result;
+        if ((result = normalise_combined_samplers(parser)) < 0)
+            return result;
+    }
 
     if (TRACE_ON())
         vkd3d_shader_trace(instructions, &parser->shader_version);
-- 
GitLab


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

[PATCH v2 3/3] vkd3d-shader/ir: Remove DCL_TEMPS instructions.

[Giovanni Mascellani]

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

We have to do work to keep it updated across passes and we never read it.
---
 libs/vkd3d-shader/ir.c    | 57 +++++++++++++--------------------------
 libs/vkd3d-shader/spirv.c |  1 -
 2 files changed, 18 insertions(+), 40 deletions(-)

diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c
index 57783de68..fb45e49ef 100644
--- a/libs/vkd3d-shader/ir.c
+++ b/libs/vkd3d-shader/ir.c
@@ -36,6 +36,19 @@ static void vkd3d_shader_instruction_make_nop(struct vkd3d_shader_instruction *i
     vsir_instruction_init(ins, &location, VKD3DSIH_NOP);
 }
 
+static void remove_dcl_temps(struct vkd3d_shader_parser *parser)
+{
+    unsigned int i;
+
+    for (i = 0; i < parser->instructions.count; ++i)
+    {
+        struct vkd3d_shader_instruction *ins = &parser->instructions.elements[i];
+
+        if (ins->handler_idx == VKD3DSIH_DCL_TEMPS)
+            vkd3d_shader_instruction_make_nop(ins);
+    }
+}
+
 static void shader_register_eliminate_phase_addressing(struct vkd3d_shader_register *reg,
         unsigned int instance_id)
 {
@@ -143,9 +156,6 @@ struct hull_flattener
 {
     struct vkd3d_shader_instruction_array instructions;
 
-    unsigned int max_temp_count;
-    unsigned int temp_dcl_idx;
-
     unsigned int instance_count;
     unsigned int phase_body_idx;
     enum vkd3d_shader_opcode phase;
@@ -203,21 +213,6 @@ static void flattener_eliminate_phase_related_dcls(struct hull_flattener *normal
         vkd3d_shader_instruction_make_nop(ins);
         return;
     }
-    else if (ins->handler_idx == VKD3DSIH_DCL_TEMPS && normaliser->phase != VKD3DSIH_INVALID)
-    {
-        /* Leave only the first temp declaration and set it to the max count later. */
-        if (!normaliser->max_temp_count)
-        {
-            normaliser->max_temp_count = ins->declaration.count;
-            normaliser->temp_dcl_idx = index;
-        }
-        else
-        {
-            normaliser->max_temp_count = max(normaliser->max_temp_count, ins->declaration.count);
-            vkd3d_shader_instruction_make_nop(ins);
-        }
-        return;
-    }
 
     if (normaliser->phase == VKD3DSIH_INVALID || shader_instruction_is_dcl(ins))
         return;
@@ -371,9 +366,6 @@ static enum vkd3d_result instruction_array_flatten_hull_shader_phases(struct vkd
 
     if (flattener.phase != VKD3DSIH_INVALID)
     {
-        if (flattener.temp_dcl_idx)
-            instructions->elements[flattener.temp_dcl_idx].declaration.count = flattener.max_temp_count;
-
         if (!shader_instruction_array_reserve(&flattener.instructions, flattener.instructions.count + 1))
             return VKD3D_ERROR_OUT_OF_MEMORY;
         vsir_instruction_init(&instructions->elements[instructions->count++], &flattener.last_ret_location, VKD3DSIH_RET);
@@ -570,9 +562,6 @@ struct io_normaliser
     struct shader_signature *output_signature;
     struct shader_signature *patch_constant_signature;
 
-    unsigned int max_temp_count;
-    unsigned int temp_dcl_idx;
-
     unsigned int instance_count;
     unsigned int phase_body_idx;
     enum vkd3d_shader_opcode phase;
@@ -2233,6 +2222,8 @@ enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser,
     struct vkd3d_shader_instruction_array *instructions = &parser->instructions;
     enum vkd3d_result result = VKD3D_OK;
 
+    remove_dcl_temps(parser);
+
     if (!parser->shader_desc.is_dxil)
     {
         if (parser->shader_version.type != VKD3D_SHADER_TYPE_PIXEL)
@@ -2284,7 +2275,6 @@ struct validation_context
     size_t instruction_idx;
     bool invalid_instruction_idx;
     bool dcl_temps_found;
-    unsigned int temp_count;
     enum vkd3d_shader_opcode phase;
 
     struct validation_context_temp_data
@@ -2339,11 +2329,7 @@ static void vsir_validate_src_param(struct validation_context *ctx,
 static void vsir_validate_register(struct validation_context *ctx,
         const struct vkd3d_shader_register *reg)
 {
-    unsigned int i, temp_count = ctx->temp_count;
-
-    /* SM1-3 shaders do not include a DCL_TEMPS instruction. */
-    if (ctx->parser->shader_version.major <= 3)
-        temp_count = ctx->parser->shader_desc.temp_count;
+    unsigned int i;
 
     if (reg->type >= VKD3DSPR_COUNT)
         validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_REGISTER_TYPE, "Invalid register type %#x.",
@@ -2388,18 +2374,13 @@ static void vsir_validate_register(struct validation_context *ctx,
             if (reg->idx[0].rel_addr)
                 validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_INDEX, "Non-NULL relative address for a TEMP register.");
 
-            if (reg->idx[0].offset >= temp_count)
+            if (reg->idx[0].offset >= ctx->parser->shader_desc.temp_count)
             {
                 validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_INDEX, "TEMP register index %u exceeds the maximum count %u.",
-                        reg->idx[0].offset, temp_count);
+                        reg->idx[0].offset, ctx->parser->shader_desc.temp_count);
                 break;
             }
 
-            /* parser->shader_desc.temp_count might be smaller then
-             * temp_count if the parser made a mistake; we still don't
-             * want to overflow the array. */
-            if (reg->idx[0].offset >= ctx->parser->shader_desc.temp_count)
-                break;
             data = &ctx->temps[reg->idx[0].offset];
 
             if (reg->dimension == VSIR_DIMENSION_NONE)
@@ -2641,7 +2622,6 @@ static void vsir_validate_instruction(struct validation_context *ctx)
                         instruction->handler_idx);
             ctx->phase = instruction->handler_idx;
             ctx->dcl_temps_found = false;
-            ctx->temp_count = 0;
             return;
 
         default:
@@ -2664,7 +2644,6 @@ static void vsir_validate_instruction(struct validation_context *ctx)
                 validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_DCL_TEMPS, "Invalid DCL_TEMPS count %u, expected at most %u.",
                         instruction->declaration.count, ctx->parser->shader_desc.temp_count);
             ctx->dcl_temps_found = true;
-            ctx->temp_count = instruction->declaration.count;
             break;
 
         case VKD3DSIH_IF:
diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c
index 5798f262c..9b6cf6133 100644
--- a/libs/vkd3d-shader/spirv.c
+++ b/libs/vkd3d-shader/spirv.c
@@ -9577,7 +9577,6 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler,
         case VKD3DSIH_DCL_RESOURCE_RAW:
         case VKD3DSIH_DCL_RESOURCE_STRUCTURED:
         case VKD3DSIH_DCL_SAMPLER:
-        case VKD3DSIH_DCL_TEMPS:
         case VKD3DSIH_DCL_UAV_RAW:
         case VKD3DSIH_DCL_UAV_STRUCTURED:
         case VKD3DSIH_DCL_UAV_TYPED:
-- 
GitLab

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

[Giovanni Mascellani (＠giomasce)]

On Mon Jan 22 21:14:17 2024 +0000, Giovanni Mascellani wrote:
> changed this line in [version 2 of the diff](/wine/vkd3d/-/merge_requests/574/diffs?diff_id=94949&start_sha=5636617b19d2d798a952dc17568e3d3ddb3e4e84#4fae87cf1e8d06a5c7871f244e3dd75ef2dac299_2639_2642)
> On second thought, I noticed that it was easy to keep the duplicate DCL_TEMPS check, so I added it back. The general point is still valid, but in this specific case there is indeed no reason to kill correct code.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/574#note_58486

[PATCH v2 0/3] MR574: vkd3d-shader/ir: Refactor VSIR passes. - approved

[Francisco Casas (＠fcasas)]

This merge request was approved by Francisco Casas.
-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/574