[PATCH 0/2] MR49: Always declare UAV images as typed if atomic ops occur.
Atomic ops on images with Unknown type will cause SPIR-V validation failure, and assertion failure in Mesa debug builds. D3D12 allows atomics on typed buffers, and this requires a distinction to be made between UAV reads and atomic ops. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/49
From: Conor McCarthy <cmccarthy(a)codeweavers.com> Until vkd3d-shader is patched, an atomic op on a typed buffer where StorageImageReadWithoutFormat is available will cause SPIR-V validation failure, and assertion in Mesa debug builds, because the image will be declared with Unknown format. --- tests/d3d12.c | 49 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/tests/d3d12.c b/tests/d3d12.c index 483c7888..94c5831c 100644 --- a/tests/d3d12.c +++ b/tests/d3d12.c @@ -23095,18 +23095,19 @@ static void test_atomic_instructions(void) ID3D12GraphicsCommandList *command_list; D3D12_ROOT_PARAMETER root_parameters[3]; ID3D12PipelineState *pipeline_state; + unsigned int i, j, value, expected; struct d3d12_resource_readback rb; struct test_context_desc desc; struct test_context context; ID3D12CommandQueue *queue; ID3D12Device *device; - unsigned int i, j; HRESULT hr; static const DWORD ps_atomics_code[] = { #if 0 RWByteAddressBuffer u; + RWBuffer<uint> u1; uint4 v; int4 i; @@ -23122,22 +23123,25 @@ static void test_atomic_instructions(void) u.InterlockedMax(6 * 4, v.x); u.InterlockedMin(7 * 4, v.x); u.InterlockedXor(8 * 4, v.x); + /* Test a typed buffer to ensure it's declared typed. */ + InterlockedAnd(u1[0], v.x); } #endif - 0x43425844, 0x24c6a30c, 0x2ce4437d, 0xdee8a0df, 0xd18cb4bc, 0x00000001, 0x000001ac, 0x00000003, + 0x43425844, 0x1cce10d2, 0x448e7426, 0xe840192e, 0xd0b11621, 0x00000001, 0x000001dc, 0x00000003, 0x0000002c, 0x0000003c, 0x0000004c, 0x4e475349, 0x00000008, 0x00000000, 0x00000008, 0x4e47534f, - 0x00000008, 0x00000000, 0x00000008, 0x58454853, 0x00000158, 0x00000050, 0x00000056, 0x0100086a, - 0x04000059, 0x00208e46, 0x00000000, 0x00000002, 0x0300009d, 0x0011e000, 0x00000000, 0x080000a9, - 0x0011e000, 0x00000000, 0x00004001, 0x00000000, 0x0020800a, 0x00000000, 0x00000000, 0x0b0000ac, - 0x0011e000, 0x00000000, 0x00004001, 0x00000004, 0x0020801a, 0x00000000, 0x00000000, 0x0020800a, - 0x00000000, 0x00000000, 0x080000ad, 0x0011e000, 0x00000000, 0x00004001, 0x00000008, 0x0020800a, - 0x00000000, 0x00000000, 0x080000aa, 0x0011e000, 0x00000000, 0x00004001, 0x0000000c, 0x0020800a, - 0x00000000, 0x00000000, 0x080000ae, 0x0011e000, 0x00000000, 0x00004001, 0x00000010, 0x0020800a, - 0x00000000, 0x00000001, 0x080000af, 0x0011e000, 0x00000000, 0x00004001, 0x00000014, 0x0020800a, - 0x00000000, 0x00000001, 0x080000b0, 0x0011e000, 0x00000000, 0x00004001, 0x00000018, 0x0020800a, - 0x00000000, 0x00000000, 0x080000b1, 0x0011e000, 0x00000000, 0x00004001, 0x0000001c, 0x0020800a, - 0x00000000, 0x00000000, 0x080000ab, 0x0011e000, 0x00000000, 0x00004001, 0x00000020, 0x0020800a, - 0x00000000, 0x00000000, 0x0100003e, + 0x00000008, 0x00000000, 0x00000008, 0x58454853, 0x00000188, 0x00000050, 0x00000062, 0x0100086a, + 0x04000059, 0x00208e46, 0x00000000, 0x00000002, 0x0300009d, 0x0011e000, 0x00000000, 0x0400089c, + 0x0011e000, 0x00000001, 0x00004444, 0x080000a9, 0x0011e000, 0x00000000, 0x00004001, 0x00000000, + 0x0020800a, 0x00000000, 0x00000000, 0x0b0000ac, 0x0011e000, 0x00000000, 0x00004001, 0x00000004, + 0x0020801a, 0x00000000, 0x00000000, 0x0020800a, 0x00000000, 0x00000000, 0x080000ad, 0x0011e000, + 0x00000000, 0x00004001, 0x00000008, 0x0020800a, 0x00000000, 0x00000000, 0x080000aa, 0x0011e000, + 0x00000000, 0x00004001, 0x0000000c, 0x0020800a, 0x00000000, 0x00000000, 0x080000ae, 0x0011e000, + 0x00000000, 0x00004001, 0x00000010, 0x0020800a, 0x00000000, 0x00000001, 0x080000af, 0x0011e000, + 0x00000000, 0x00004001, 0x00000014, 0x0020800a, 0x00000000, 0x00000001, 0x080000b0, 0x0011e000, + 0x00000000, 0x00004001, 0x00000018, 0x0020800a, 0x00000000, 0x00000000, 0x080000b1, 0x0011e000, + 0x00000000, 0x00004001, 0x0000001c, 0x0020800a, 0x00000000, 0x00000000, 0x080000ab, 0x0011e000, + 0x00000000, 0x00004001, 0x00000020, 0x0020800a, 0x00000000, 0x00000000, 0x080000a9, 0x0011e000, + 0x00000001, 0x00004001, 0x00000000, 0x0020800a, 0x00000000, 0x00000000, 0x0100003e, }; static const D3D12_SHADER_BYTECODE ps_atomics = {ps_atomics_code, sizeof(ps_atomics_code)}; static const DWORD cs_atomics_code[] = @@ -23292,6 +23296,19 @@ static void test_atomic_instructions(void) ID3D12GraphicsCommandList_SetPipelineState(command_list, context.pipeline_state); ID3D12GraphicsCommandList_DrawInstanced(command_list, 3, 1, 0, 0); + transition_sub_resource_state(command_list, cs_buffer, 0, + D3D12_RESOURCE_STATE_UNORDERED_ACCESS, D3D12_RESOURCE_STATE_COPY_SOURCE); + get_buffer_readback_with_command_list(cs_buffer, DXGI_FORMAT_R32_UINT, &rb, queue, command_list); + value = get_readback_uint(&rb.rb, 0, 0, 0); + expected = test->expected_result[0]; + ok(value == expected, "Got %#x, expected %#x.\n", value, expected); + release_resource_readback(&rb); + reset_command_list(command_list, context.allocator); + transition_sub_resource_state(command_list, cs_buffer, 0, + D3D12_RESOURCE_STATE_COPY_SOURCE, D3D12_RESOURCE_STATE_COPY_DEST); + upload_buffer_data(cs_buffer, 0, sizeof(test->input[0]), test->input, queue, command_list); + reset_command_list(command_list, context.allocator); + ID3D12GraphicsCommandList_SetComputeRootSignature(command_list, context.root_signature); ID3D12GraphicsCommandList_SetComputeRootUnorderedAccessView(command_list, 0, ID3D12Resource_GetGPUVirtualAddress(cs_buffer)); @@ -23308,10 +23325,10 @@ static void test_atomic_instructions(void) get_buffer_readback_with_command_list(ps_buffer, DXGI_FORMAT_R32_UINT, &rb, queue, command_list); for (j = 0; j < ARRAY_SIZE(instructions); ++j) { - unsigned int value = get_readback_uint(&rb.rb, j, 0, 0); - unsigned int expected = test->expected_result[j]; bool is_bug; + value = get_readback_uint(&rb.rb, j, 0, 0); + expected = test->expected_result[j]; is_bug = test->i.x < 0 && (!strcmp(instructions[j], "atomic_imax") || !strcmp(instructions[j], "atomic_imin")); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/49
From: Conor McCarthy <cmccarthy(a)codeweavers.com> Atomic ops on images with Unknown type will cause SPIR-V validation failure, and assertion failure in Mesa debug builds. D3D12 allows atomics on typed buffers, and this requires a distinction to be made between UAV reads and atomic ops. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53874 --- include/vkd3d_shader.h | 3 +++ libs/vkd3d-shader/spirv.c | 5 +++-- libs/vkd3d-shader/vkd3d_shader_main.c | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 7178ac5f..939ab42d 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -1263,6 +1263,9 @@ enum vkd3d_shader_descriptor_info_flag VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_READ = 0x00000002, /** The descriptor is a comparison sampler. */ VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_SAMPLER_COMPARISON_MODE = 0x00000004, + /** The descriptor is a UAV resource, on which the shader performs + * atomic ops. \since 1.6 */ + VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_ATOMICS = 0x00000008, VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_DESCRIPTOR_INFO_FLAG), }; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 734ba315..2d1691c6 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -5864,9 +5864,9 @@ static uint32_t spirv_compiler_get_image_type_id(struct spirv_compiler *compiler { struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; const struct vkd3d_shader_descriptor_info *d; + bool uav_read, uav_atomics; uint32_t sampled_type_id; SpvImageFormat format; - bool uav_read; format = SpvImageFormatUnknown; if (reg->type == VKD3DSPR_UAV) @@ -5874,7 +5874,8 @@ static uint32_t spirv_compiler_get_image_type_id(struct spirv_compiler *compiler d = spirv_compiler_get_descriptor_info(compiler, VKD3D_SHADER_DESCRIPTOR_TYPE_UAV, range); uav_read = !!(d->flags & VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_READ); - if (raw_structured || (uav_read && !compiler->uav_read_without_format)) + uav_atomics = !!(d->flags & VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_ATOMICS); + if (raw_structured || uav_atomics || (uav_read && !compiler->uav_read_without_format)) format = image_format_for_image_read(data_type); else if (uav_read) vkd3d_spirv_enable_capability(builder, SpvCapabilityStorageImageReadWithoutFormat); diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 5e25dde8..424082a5 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -692,6 +692,25 @@ static void vkd3d_shader_scan_record_uav_counter(struct vkd3d_shader_scan_contex d->flags |= VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_COUNTER; } +static bool vkd3d_shader_instruction_is_uav_atomic_op(const struct vkd3d_shader_instruction *instruction) +{ + enum vkd3d_shader_opcode handler_idx = instruction->handler_idx; + return (VKD3DSIH_ATOMIC_AND <= handler_idx && handler_idx <= VKD3DSIH_ATOMIC_XOR + && instruction->dst[0].reg.type == VKD3DSPR_UAV); +} + +static void vkd3d_shader_scan_record_uav_atomic_op(struct vkd3d_shader_scan_context *context, + const struct vkd3d_shader_register *reg) +{ + struct vkd3d_shader_descriptor_info *d; + + if (!context->scan_descriptor_info) + return; + + d = vkd3d_shader_scan_get_uav_descriptor_info(context, reg->idx[0].offset); + d->flags |= VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_ATOMICS; +} + static bool vkd3d_shader_scan_add_descriptor(struct vkd3d_shader_scan_context *context, enum vkd3d_shader_descriptor_type type, const struct vkd3d_shader_register_range *range, enum vkd3d_shader_resource_type resource_type, enum vkd3d_shader_resource_data_type resource_data_type, @@ -1019,6 +1038,9 @@ static int vkd3d_shader_scan_instruction(struct vkd3d_shader_scan_context *conte if (vkd3d_shader_instruction_is_uav_counter(instruction)) vkd3d_shader_scan_record_uav_counter(context, &instruction->src[0].reg); + if (vkd3d_shader_instruction_is_uav_atomic_op(instruction)) + vkd3d_shader_scan_record_uav_atomic_op(context, &instruction->dst[0].reg); + ++context->location.line; return VKD3D_OK; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/49
```diff +static bool vkd3d_shader_instruction_is_uav_atomic_op(const struct vkd3d_shader_instruction *instruction) +{ + enum vkd3d_shader_opcode handler_idx = instruction->handler_idx; + return (VKD3DSIH_ATOMIC_AND <= handler_idx && handler_idx <= VKD3DSIH_ATOMIC_XOR + && instruction->dst[0].reg.type == VKD3DSPR_UAV); +} ```
Should we take the IMM_ATOMIC_* operations into account here as well, like we do in vkd3d_shader_instruction_is_uav_read()? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/49#note_16179
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Henri Verbeet (＠hverbeet)