[PATCH 0/2] MR345: vkd3d-shader/dxil: Fix a couple of issues.

[PATCH 1/2] vkd3d-shader/dxil: Do not access null code blocks on failure.

[Conor McCarthy]

From: Conor McCarthy

---
 libs/vkd3d-shader/dxil.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c
index 666d8b08..5f332f59 100644
--- a/libs/vkd3d-shader/dxil.c
+++ b/libs/vkd3d-shader/dxil.c
@@ -2418,11 +2418,11 @@ static enum vkd3d_result sm6_parser_function_init(struct sm6_parser *sm6, const
         struct sm6_function *function)
 {
     struct vkd3d_shader_instruction *ins;
+    size_t i, block_idx, block_count;
     const struct dxil_record *record;
     bool ret_found, is_terminator;
     struct sm6_block *code_block;
     struct sm6_value *dst;
-    size_t i, block_idx;

if (sm6->function_count)
     {
@@ -2448,12 +2448,12 @@ static enum vkd3d_result sm6_parser_function_init(struct sm6_parser *sm6, const
         return VKD3D_ERROR_INVALID_SHADER;
     }

-    if (!(function->block_count = block->records[0]->operands[0]))
+    if (!(block_count = block->records[0]->operands[0]))
     {
         WARN("Function contains no blocks.\n");
         return VKD3D_ERROR_INVALID_SHADER;
     }
-    if (function->block_count > 1)
+    if (block_count > 1)
     {
         FIXME("Branched shaders are not supported yet.\n");
         return VKD3D_ERROR_INVALID_SHADER;
@@ -2464,6 +2464,7 @@ static enum vkd3d_result sm6_parser_function_init(struct sm6_parser *sm6, const
         ERR("Failed to allocate code block.\n");
         return VKD3D_ERROR_OUT_OF_MEMORY;
     }
+    function->block_count = block_count;
     code_block = function->blocks[0];

sm6->cur_max_value = function->value_count;

[PATCH 2/2] vkd3d-shader/dxil: Emit a default thread group size for computer shaders.

[Conor McCarthy]

From: Conor McCarthy

Prevents a RADV crash on pipeline creation.
---
 libs/vkd3d-shader/dxil.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c
index 5f332f59..d1a4c619 100644
--- a/libs/vkd3d-shader/dxil.c
+++ b/libs/vkd3d-shader/dxil.c
@@ -2607,6 +2607,19 @@ static enum vkd3d_result sm6_parser_module_init(struct sm6_parser *sm6, const st
     return VKD3D_OK;
 }

+static void sm6_parser_emit_thread_group(struct sm6_parser *sm6)
+{
+    struct vkd3d_shader_thread_group_size thread_group_size = {1, 1, 1};
+    struct vkd3d_shader_instruction *ins;
+
+    ins = sm6_parser_add_instruction(sm6, VKD3DSIH_DCL_THREAD_GROUP);
+    ins->declaration.thread_group_size = thread_group_size;
+
+    FIXME("Dimensions are defaulting to 1.\n");
+
+    ins->declaration.thread_group_size = thread_group_size;
+}
+
 static void sm6_type_table_cleanup(struct sm6_type *types, size_t count)
 {
     size_t i;
@@ -2879,6 +2892,9 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t

sm6_parser_init_output_signature(sm6, output_signature);

+    if (version.type == VKD3D_SHADER_TYPE_COMPUTE)
+        sm6_parser_emit_thread_group(sm6);
+
     if ((ret = sm6_parser_module_init(sm6, &sm6->root_block, 0)) < 0)
     {
         if (ret == VKD3D_ERROR_OUT_OF_MEMORY)

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
>  return VKD3D_OK;
>
> }
>
> +static void sm6_parser_emit_thread_group(struct sm6_parser *sm6)
> +{
> - struct vkd3d_shader_thread_group_size thread_group_size = {1, 1, 1};
> - struct vkd3d_shader_instruction *ins;
> -
> - ins = sm6_parser_add_instruction(sm6, VKD3DSIH_DCL_THREAD_GROUP);
> - ins->declaration.thread_group_size = thread_group_size;
> -
> - FIXME("Dimensions are defaulting to 1.\n");
> -
> - ins->declaration.thread_group_size = thread_group_size;

That looks like a duplicate.

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
>  sm6_parser_init_output_signature(sm6, output_signature);
>
> - if (version.type == VKD3D_SHADER_TYPE_COMPUTE)
> -
>    sm6_parser_emit_thread_group(sm6);

I'm not sure I understand what's to be gained here: this look like a workaround for a specific crash of a Vulkan implementation, but SM6 support is still so early that there is basically no compute shader that you can throw at it. So why should we address this specific crash?

[Henri Verbeet (＠hverbeet)]

Mostly what Giovanni already said. Specifically:
> +static void sm6_parser_emit_thread_group(struct sm6_parser *sm6)
> +{
> +    struct vkd3d_shader_thread_group_size thread_group_size = {1, 1, 1};
> +    struct vkd3d_shader_instruction *ins;
> +
> +    ins = sm6_parser_add_instruction(sm6, VKD3DSIH_DCL_THREAD_GROUP);
> +    ins->declaration.thread_group_size = thread_group_size;
> +
> +    FIXME("Dimensions are defaulting to 1.\n");
> +
> +    ins->declaration.thread_group_size = thread_group_size;
> +}

Is this the expected behaviour for dxil compute shaders that don't explicitly declare a thread group size? In that case we may want a vkd3d_shader_parser_warning(), but certainly not a FIXME. If such shaders are instead simply invalid, we should reject them.

[Conor McCarthy (＠cmccarthy)]

The thread group size is not parsed at all, so this is just to prevent a crash. I'd rather not do it, but unless we avoid SM6 compute shaders in shader runner for now, I see no way around it.

[Henri Verbeet (＠hverbeet)]

> The thread group size is not parsed at all, so this is just to prevent a crash. I'd rather not do it, but unless we avoid SM6 compute shaders in shader runner for now, I see no way around it.

Why do those shaders end up compiling at all?