[PATCH v4 0/3] MR345: vkd3d-shader/dxil: Fix a couple of issues.
-- v4: vkd3d-shader/dxil: Convert into an error the warning for an unhandled instrinsic. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345
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 b778f6abe..41b64b1b2 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> The thread group size is not yet emitted. --- libs/vkd3d-shader/dxil.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 41b64b1b2..453470184 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -2760,6 +2760,11 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t vkd3d_shader_parser_warning(&sm6->p, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_SHADER_TYPE, "Unknown shader type %#x.", version.type); } + if (version.type == VKD3D_SHADER_TYPE_COMPUTE) + { + FIXME("Compute shader compilation is not implemented yet.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } version.major = VKD3D_SM6_VERSION_MAJOR(version_token); version.minor = VKD3D_SM6_VERSION_MINOR(version_token); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 2 +- libs/vkd3d-shader/vkd3d_shader_private.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 453470184..e3a37f745 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -2303,7 +2303,7 @@ static void sm6_parser_decode_dx_op(struct sm6_parser *sm6, struct sm6_block *co if (op >= ARRAY_SIZE(sm6_dx_op_table) || !sm6_dx_op_table[op].operand_info) { FIXME("Unhandled dx intrinsic function id %u, '%s'.\n", op, name); - vkd3d_shader_parser_warning(&sm6->p, VKD3D_SHADER_WARNING_DXIL_UNHANDLED_INTRINSIC, + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_UNHANDLED_INTRINSIC, "Call to intrinsic function %s is unhandled.", name); sm6_parser_emit_unhandled(sm6, ins, dst); return; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 5fd930918..ad4e9b683 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -174,13 +174,13 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_DXIL_INVALID_TYPE_ID = 8010, VKD3D_SHADER_ERROR_DXIL_INVALID_MODULE = 8011, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND = 8012, + VKD3D_SHADER_ERROR_DXIL_UNHANDLED_INTRINSIC = 8013, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_MAGIC_NUMBER = 8300, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_SHADER_TYPE = 8301, VKD3D_SHADER_WARNING_DXIL_INVALID_BLOCK_LENGTH = 8302, VKD3D_SHADER_WARNING_DXIL_INVALID_MODULE_LENGTH = 8303, VKD3D_SHADER_WARNING_DXIL_IGNORING_OPERANDS = 8304, - VKD3D_SHADER_WARNING_DXIL_UNHANDLED_INTRINSIC = 8305, VKD3D_SHADER_ERROR_VSIR_NOT_IMPLEMENTED = 9000, VKD3D_SHADER_ERROR_VSIR_INVALID_HANDLER = 9001, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345
On Thu Sep 21 13:30:49 2023 +0000, Conor McCarthy wrote:
The current situation with metadata is a little complicated. Strictly speaking the signatures should be loaded from metadata because the DXBC signatures may not be identical, and the interpolation mode is not loaded either. There's no issue with simple signatures though. I think the best thing to do is upstream metadata and signature loads ASAP. `sm6_parser_emit_unhandled()` was useful during development, but yeah it should be an error now. I made the change for unhandled instructions.
After MR346 it remains impossible to actually compile any shader which needs an interpolation mode, and very unlikely to compile one which needs the DXIL signatures. Are there any remaining issues which would hold up MR320,345,346 in general? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_46573
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345
I made the change for unhandled instructions.
After MR346 it remains impossible to actually compile any shader which needs an interpolation mode, and very unlikely to compile one which needs the DXIL signatures.
Do we still need the special case for compute shaders in patch 2/3 then?
Are there any remaining issues which would hold up MR320,345,346 in general?
Not in general, no. I did have some comments on !346 specifically though, and it's marked as a draft, of course. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_46662
Do we still need the special case for compute shaders in patch 2/3 then?
That issue still exists. I spoke too soon about the DXIL signatures; one test shader fails on it. I commented on it in MR346. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_46762
Do we still need the special case for compute shaders in patch 2/3 then?
That issue still exists.
Why is that? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_46782
On Tue Sep 26 14:46:29 2023 +0000, Henri Verbeet wrote:
Do we still need the special case for compute shaders in patch 2/3 then?
That issue still exists. Why is that? The thread group size is still not read.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_46820
The thread group size is still not read.
Right, but if the affected shaders are already going to fail due to e.g. unimplemented instructions, that's somewhat moot. The practical thing to do in that case would be to implemented reading the thread group size before implementing the required instructions for those shaders, although perhaps ideally not reading a thread group size for shaders that require one would cause compilation to fail on its own. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/345#note_46827
participants (4)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)