[PATCH 0/2] MR255: vkd3d-shader: Scan for the maximum temporary register in d3dbc and emit a spirv temp array accordingly.

[PATCH 1/2] vkd3d-shader/dxbc: Remove redundant zero-initialization of the vkd3d_shader_desc structure.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/dxbc.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c
index 3e3f06fae..9832c9f9b 100644
--- a/libs/vkd3d-shader/dxbc.c
+++ b/libs/vkd3d-shader/dxbc.c
@@ -529,12 +529,6 @@ int shader_extract_from_dxbc(const struct vkd3d_shader_code *dxbc,
 {
     int ret;

-    desc->byte_code = NULL;
-    desc->byte_code_size = 0;
-    memset(&desc->input_signature, 0, sizeof(desc->input_signature));
-    memset(&desc->output_signature, 0, sizeof(desc->output_signature));
-    memset(&desc->patch_constant_signature, 0, sizeof(desc->patch_constant_signature));
-
     ret = for_each_dxbc_section(dxbc, message_context, source_name, shdr_handler, desc);
     if (!desc->byte_code)
         ret = VKD3D_ERROR_INVALID_ARGUMENT;

[PATCH 2/2] vkd3d-shader: Scan for the maximum temporary register in d3dbc and emit a spirv temp array accordingly.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/d3dbc.c                | 15 ++++++++++++---
 libs/vkd3d-shader/spirv.c                | 10 ++++++----
 libs/vkd3d-shader/vkd3d_shader_private.h |  2 ++
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c
index d2a4666a5..8564ba1d9 100644
--- a/libs/vkd3d-shader/d3dbc.c
+++ b/libs/vkd3d-shader/d3dbc.c
@@ -729,6 +729,16 @@ static bool add_signature_element_from_semantic(struct vkd3d_shader_sm1_parser *
             semantic->usage_idx, sysval, reg->idx[0].offset, true, mask);
 }

+static void shader_sm1_scan_register(struct vkd3d_shader_sm1_parser *sm1, const struct vkd3d_shader_register *reg)
+{
+    uint32_t register_index = reg->idx[0].offset;
+
+    if (reg->type == VKD3DSPR_TEMP)
+        sm1->p.shader_desc.temp_count = max(sm1->p.shader_desc.temp_count, register_index + 1);
+
+    add_signature_element_from_register(sm1, reg);
+}
+
 /* Read a parameter token from the input stream, and possibly a relative
  * addressing token. */
 static void shader_sm1_read_param(struct vkd3d_shader_sm1_parser *sm1,
@@ -1083,7 +1093,7 @@ static void shader_sm1_read_instruction(struct vkd3d_shader_sm1_parser *sm1, str
         if (ins->dst_count)
         {
             shader_sm1_read_dst_param(sm1, &p, dst_param);
-            add_signature_element_from_register(sm1, &dst_param->reg, false, dst_param->write_mask);
+            shader_sm1_scan_register(sm1, &dst_param->reg);
         }

/* Predication token */
@@ -1094,8 +1104,7 @@ static void shader_sm1_read_instruction(struct vkd3d_shader_sm1_parser *sm1, str
         for (i = 0; i < ins->src_count; ++i)
         {
             shader_sm1_read_src_param(sm1, &p, &src_params[i]);
-            add_signature_element_from_register(sm1, &src_params[i].reg,
-                    false, mask_from_swizzle(src_params[i].swizzle));
+            shader_sm1_scan_register(sm1, &src_params[i].reg);
         }
     }

diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c
index 3542b5fac..c07607e1f 100644
--- a/libs/vkd3d-shader/spirv.c
+++ b/libs/vkd3d-shader/spirv.c
@@ -5258,8 +5258,7 @@ static void spirv_compiler_emit_dcl_global_flags(struct spirv_compiler *compiler
         WARN("Unhandled global flags %#x.\n", flags);
 }

-static void spirv_compiler_emit_dcl_temps(struct spirv_compiler *compiler,
-        const struct vkd3d_shader_instruction *instruction)
+static void spirv_compiler_emit_dcl_temps(struct spirv_compiler *compiler, uint32_t count)
 {
     struct vkd3d_spirv_builder *builder = &compiler->spirv_builder;
     size_t function_location;
@@ -5270,7 +5269,7 @@ static void spirv_compiler_emit_dcl_temps(struct spirv_compiler *compiler,
     vkd3d_spirv_begin_function_stream_insertion(builder, function_location);

assert(!compiler->temp_count);
-    compiler->temp_count = instruction->declaration.count;
+    compiler->temp_count = count;
     for (i = 0; i < compiler->temp_count; ++i)
     {
         id = spirv_compiler_emit_variable(compiler, &builder->function_stream,
@@ -9104,7 +9103,7 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler,
             spirv_compiler_emit_dcl_global_flags(compiler, instruction);
             break;
         case VKD3DSIH_DCL_TEMPS:
-            spirv_compiler_emit_dcl_temps(compiler, instruction);
+            spirv_compiler_emit_dcl_temps(compiler, instruction->declaration.count);
             break;
         case VKD3DSIH_DCL_INDEXABLE_TEMP:
             spirv_compiler_emit_dcl_indexable_temp(compiler, instruction);
@@ -9448,6 +9447,9 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler,
     enum vkd3d_result result = VKD3D_OK;
     unsigned int i;

+    if (parser->shader_desc.temp_count)
+        spirv_compiler_emit_dcl_temps(compiler, parser->shader_desc.temp_count);
+
     compiler->location.column = 0;
     compiler->location.line = 1;

diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h
index 528a66517..590912948 100644
--- a/libs/vkd3d-shader/vkd3d_shader_private.h
+++ b/libs/vkd3d-shader/vkd3d_shader_private.h
@@ -817,6 +817,8 @@ struct vkd3d_shader_desc
     struct shader_signature input_signature;
     struct shader_signature output_signature;
     struct shader_signature patch_constant_signature;
+
+    uint32_t temp_count;
 };

struct vkd3d_shader_register_semantic

[Zebediah Figura (＠zfigura)]

Alternatively, we could emit dcl_temps in the lowering passes (though this requires a bit more boilerplate) or translate dcl_temps to the temp_count field at scanning time for sm4.

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/d3dbc.c:
>          semantic->usage_idx, sysval, reg->idx[0].offset, true, mask);
>
> }
>
> +static void shader_sm1_scan_register(struct vkd3d_shader_sm1_parser *sm1, const struct vkd3d_shader_register *reg)
> +{
> - uint32_t register_index = reg->idx[0].offset;
> -
> - if (reg->type == VKD3DSPR_TEMP)
> -
>    sm1->p.shader_desc.temp_count = max(sm1->p.shader_desc.temp_count, register_index + 1);
> -
> - add_signature_element_from_register(sm1, reg);

```
../vkd3d/libs/vkd3d-shader/d3dbc.c: In function 'shader_sm1_scan_register':
../vkd3d/libs/vkd3d-shader/d3dbc.c:739:5: error: too few arguments to function 'add_signature_element_from_register'
  739 |     add_signature_element_from_register(sm1, reg);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../vkd3d/libs/vkd3d-shader/d3dbc.c:584:13: note: declared here
  584 | static bool add_signature_element_from_register(struct vkd3d_shader_sm1_parser *sm1,
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

[Henri Verbeet (＠hverbeet)]

> Alternatively, we could emit dcl_temps in the lowering passes (though this requires a bit more boilerplate)

Yeah. I don't hate this series, but I think that with the direction we've been going in lately we'll generally want to do some kind of raising pass(es) to insert the various dcl_ instructions and get the IR produced by d3dbc sources closer to what the SPIR-V backend expects, instead of making the SPIR-V backend able to handle d3dbc sources as-is.

[Zebediah Figura (＠zfigura)]

> > Alternatively, we could emit dcl_temps in the lowering passes (though this requires a bit more boilerplate)
>
> Yeah. I don't hate this series, but I think that with the direction we've been going in lately we'll generally want to do some kind of raising pass(es) to insert the various dcl_ instructions and get the IR produced by d3dbc sources closer to what the SPIR-V backend expects, instead of making the SPIR-V backend able to handle d3dbc sources as-is.

I feel like the other alternative I mentioned is better, honestly, i.e. scanning both sm1 registers and sm4 dcl_temps to produce a "temp_count". Do you have a preference for normalizing in the direction of sm4 specifically?

[Henri Verbeet (＠hverbeet)]

Not specifically, no; I think resolving dcl_temps to "temp_count" would be fine too.