[PATCH 0/2] MR345: vkd3d-shader/dxil: Fix a couple of issues.
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- 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; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345
From: Conor McCarthy <cmccarthy(a)codeweavers.com> 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) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345
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. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_45342
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? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_45343
Mostly what Giovanni already said. Specifically:
```diff +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. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_45347
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. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_45353
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? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_45354
participants (4)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)