[PATCH vkd3d v2 1/3] tests: Add test for uniform dynamic indexing of bounded descriptor arrays.
From: Henri Verbeet <hverbeet(a)codeweavers.com> Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- tests/d3d12.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 194 insertions(+), 1 deletion(-) diff --git a/tests/d3d12.c b/tests/d3d12.c index f289a10d..0cdb22e0 100644 --- a/tests/d3d12.c +++ b/tests/d3d12.c @@ -740,7 +740,7 @@ static ID3D12PipelineState *create_compute_pipeline_state_(unsigned int line, ID ID3D12RootSignature *root_signature, const D3D12_SHADER_BYTECODE cs) { D3D12_COMPUTE_PIPELINE_STATE_DESC pipeline_state_desc; - ID3D12PipelineState *pipeline_state; + ID3D12PipelineState *pipeline_state = NULL; HRESULT hr; memset(&pipeline_state_desc, 0, sizeof(pipeline_state_desc)); @@ -33817,6 +33817,198 @@ static void test_hull_shader_patch_constant_inputs(void) destroy_test_context(&context); } +static void test_resource_arrays(void) +{ + ID3D12Resource *input_buffers[8], *output_buffers[6]; + D3D12_ROOT_SIGNATURE_DESC root_signature_desc; + D3D12_UNORDERED_ACCESS_VIEW_DESC uav_desc; + D3D12_SHADER_RESOURCE_VIEW_DESC srv_desc; + ID3D12GraphicsCommandList *command_list; + unsigned int descriptor_count; + struct test_context context; + ID3D12DescriptorHeap *heap; + ID3D12CommandQueue *queue; + ID3D12Device *device; + unsigned int i; + HRESULT hr; + + static const D3D12_DESCRIPTOR_RANGE descriptor_ranges[] = + { + {D3D12_DESCRIPTOR_RANGE_TYPE_SRV, 8, 2, 2, 0}, + {D3D12_DESCRIPTOR_RANGE_TYPE_UAV, 6, 1, 3, D3D12_DESCRIPTOR_RANGE_OFFSET_APPEND}, + }; + + static const struct uvec4 cb_data[] = + { + {0, 0}, + {1, 1}, + {0, 5}, + {1, 4}, + {2, 0}, + {3, 1}, + }; + + static const D3D12_ROOT_PARAMETER root_parameters[] = + { + {D3D12_ROOT_PARAMETER_TYPE_DESCRIPTOR_TABLE, + .DescriptorTable = {ARRAY_SIZE(descriptor_ranges), descriptor_ranges}}, + {D3D12_ROOT_PARAMETER_TYPE_32BIT_CONSTANTS, .Constants = {2, 1, ARRAY_SIZE(cb_data) * 4}}, + }; + + static const DWORD cs_code[] = + { +#if 0 + cbuffer cb : register(b2, space1) + { + uint2 c[6]; + } + + Buffer<uint> t1[2] : register(t2, space2); + Buffer<uint> t2[] : register(t4, space2); + + RWBuffer<uint> u1[2] : register(u1, space3); + RWBuffer<uint> u2[] : register(u3, space3); + + [numthreads(1, 1, 1)] + void main() + { + u1[c[0].x][0] = t1[c[0].y][0]; + u1[c[1].x][0] = t1[c[1].y][0]; + u2[c[2].x][0] = t2[c[2].y][0]; + u2[c[3].x][0] = t2[c[3].y][0]; + u2[c[4].x][0] = t2[c[4].y][0]; + u2[c[5].x][0] = t2[c[5].y][0]; + } +#endif + 0x43425844, 0xef615fd3, 0x708c1d93, 0xdb9908b4, 0xb1853d57, 0x00000001, 0x000004c8, 0x00000003, + 0x0000002c, 0x0000003c, 0x0000004c, 0x4e475349, 0x00000008, 0x00000000, 0x00000008, 0x4e47534f, + 0x00000008, 0x00000000, 0x00000008, 0x58454853, 0x00000474, 0x00050051, 0x0000011d, 0x0100086a, + 0x07000059, 0x00308e46, 0x00000000, 0x00000002, 0x00000002, 0x00000006, 0x00000001, 0x07000858, + 0x00307e46, 0x00000000, 0x00000002, 0x00000003, 0x00004444, 0x00000002, 0x07000858, 0x00307e46, + 0x00000001, 0x00000004, 0xffffffff, 0x00004444, 0x00000002, 0x0700089c, 0x0031ee46, 0x00000000, + 0x00000001, 0x00000002, 0x00004444, 0x00000003, 0x0700089c, 0x0031ee46, 0x00000001, 0x00000003, + 0xffffffff, 0x00004444, 0x00000003, 0x02000068, 0x00000001, 0x0400009b, 0x00000001, 0x00000001, + 0x00000001, 0x07000036, 0x00100012, 0x00000000, 0x0030801a, 0x00000000, 0x00000002, 0x00000000, + 0x0d00002d, 0x00100012, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, + 0x06207e46, 0x00000000, 0x00000002, 0x0010000a, 0x00000000, 0x07000036, 0x00100022, 0x00000000, + 0x0030800a, 0x00000000, 0x00000002, 0x00000000, 0x0d0000a4, 0x0621e0f2, 0x00000000, 0x00000001, + 0x0010001a, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00100006, + 0x00000000, 0x07000036, 0x00100012, 0x00000000, 0x0030801a, 0x00000000, 0x00000002, 0x00000001, + 0x0d00002d, 0x00100012, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, + 0x06207e46, 0x00000000, 0x00000002, 0x0010000a, 0x00000000, 0x07000036, 0x00100022, 0x00000000, + 0x0030800a, 0x00000000, 0x00000002, 0x00000001, 0x0d0000a4, 0x0621e0f2, 0x00000000, 0x00000001, + 0x0010001a, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00100006, + 0x00000000, 0x07000036, 0x00100012, 0x00000000, 0x0030801a, 0x00000000, 0x00000002, 0x00000002, + 0x0d00002d, 0x00100012, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, + 0x06207e46, 0x00000001, 0x00000004, 0x0010000a, 0x00000000, 0x07000036, 0x00100022, 0x00000000, + 0x0030800a, 0x00000000, 0x00000002, 0x00000002, 0x0d0000a4, 0x0621e0f2, 0x00000001, 0x00000003, + 0x0010001a, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00100006, + 0x00000000, 0x07000036, 0x00100012, 0x00000000, 0x0030801a, 0x00000000, 0x00000002, 0x00000003, + 0x0d00002d, 0x00100012, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, + 0x06207e46, 0x00000001, 0x00000004, 0x0010000a, 0x00000000, 0x07000036, 0x00100022, 0x00000000, + 0x0030800a, 0x00000000, 0x00000002, 0x00000003, 0x0d0000a4, 0x0621e0f2, 0x00000001, 0x00000003, + 0x0010001a, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00100006, + 0x00000000, 0x07000036, 0x00100012, 0x00000000, 0x0030801a, 0x00000000, 0x00000002, 0x00000004, + 0x0d00002d, 0x00100012, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, + 0x06207e46, 0x00000001, 0x00000004, 0x0010000a, 0x00000000, 0x07000036, 0x00100022, 0x00000000, + 0x0030800a, 0x00000000, 0x00000002, 0x00000004, 0x0d0000a4, 0x0621e0f2, 0x00000001, 0x00000003, + 0x0010001a, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00100006, + 0x00000000, 0x07000036, 0x00100012, 0x00000000, 0x0030801a, 0x00000000, 0x00000002, 0x00000005, + 0x0d00002d, 0x00100012, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, + 0x06207e46, 0x00000001, 0x00000004, 0x0010000a, 0x00000000, 0x07000036, 0x00100022, 0x00000000, + 0x0030800a, 0x00000000, 0x00000002, 0x00000005, 0x0d0000a4, 0x0621e0f2, 0x00000001, 0x00000003, + 0x0010001a, 0x00000000, 0x00004002, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00100006, + 0x00000000, 0x0100003e, + }; + + static const uint32_t srv_data[] = {0x1, 0x4, 0x9, 0x10, 0x19, 0x24, 0x31, 0x40}; + static const uint32_t uav_data[] = {0x1, 0x4, 0x40, 0x31, 0x9, 0x10}; + + if (!init_compute_test_context(&context)) + return; + device = context.device; + command_list = context.list; + queue = context.queue; + + memset(&root_signature_desc, 0, sizeof(root_signature_desc)); + root_signature_desc.NumParameters = ARRAY_SIZE(root_parameters); + root_signature_desc.pParameters = root_parameters; + hr = create_root_signature(device, &root_signature_desc, &context.root_signature); + ok(hr == S_OK, "Failed to create root signature, hr %#x.\n", hr); + + for (i = 0, descriptor_count = 0; i < ARRAY_SIZE(descriptor_ranges); ++i) + { + descriptor_count += descriptor_ranges[i].NumDescriptors; + } + heap = create_gpu_descriptor_heap(device, D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV, descriptor_count); + + for (i = 0; i < ARRAY_SIZE(input_buffers); ++i) + { + input_buffers[i] = create_default_buffer(device, sizeof(uint32_t), + D3D12_RESOURCE_FLAG_NONE, D3D12_RESOURCE_STATE_COPY_DEST); + upload_buffer_data(input_buffers[i], 0, sizeof(srv_data[i]), &srv_data[i], queue, command_list); + reset_command_list(command_list, context.allocator); + transition_resource_state(command_list, input_buffers[i], + D3D12_RESOURCE_STATE_COPY_DEST, D3D12_RESOURCE_STATE_NON_PIXEL_SHADER_RESOURCE); + + memset(&srv_desc, 0, sizeof(srv_desc)); + srv_desc.Format = DXGI_FORMAT_R32_UINT; + srv_desc.ViewDimension = D3D12_SRV_DIMENSION_BUFFER; + srv_desc.Shader4ComponentMapping = D3D12_DEFAULT_SHADER_4_COMPONENT_MAPPING; + srv_desc.Buffer.FirstElement = 0; + srv_desc.Buffer.NumElements = 1; + ID3D12Device_CreateShaderResourceView(device, input_buffers[i], &srv_desc, + get_cpu_descriptor_handle(&context, heap, i)); + } + + for (i = 0; i < ARRAY_SIZE(output_buffers); ++i) + { + output_buffers[i] = create_default_buffer(device, sizeof(uint32_t), + D3D12_RESOURCE_FLAG_ALLOW_UNORDERED_ACCESS, D3D12_RESOURCE_STATE_UNORDERED_ACCESS); + + memset(&uav_desc, 0, sizeof(uav_desc)); + uav_desc.Format = DXGI_FORMAT_R32_UINT; + uav_desc.ViewDimension = D3D12_UAV_DIMENSION_BUFFER; + uav_desc.Buffer.FirstElement = 0; + uav_desc.Buffer.NumElements = 1; + ID3D12Device_CreateUnorderedAccessView(device, output_buffers[i], NULL, &uav_desc, + get_cpu_descriptor_handle(&context, heap, ARRAY_SIZE(input_buffers) + i)); + } + + todo + context.pipeline_state = create_compute_pipeline_state(device, context.root_signature, + shader_bytecode(cs_code, sizeof(cs_code))); + if (!context.pipeline_state) + { + skip("Shader descriptor arrays are not supported.\n"); + goto done; + } + + ID3D12GraphicsCommandList_SetPipelineState(command_list, context.pipeline_state); + ID3D12GraphicsCommandList_SetComputeRootSignature(command_list, context.root_signature); + ID3D12GraphicsCommandList_SetComputeRootDescriptorTable(command_list, + 0, get_gpu_descriptor_handle(&context, heap, 0)); + ID3D12GraphicsCommandList_SetComputeRoot32BitConstants(command_list, 1, ARRAY_SIZE(cb_data) * 4, cb_data, 0); + + ID3D12GraphicsCommandList_Dispatch(command_list, 1, 1, 1); + + for (i = 0; i < ARRAY_SIZE(output_buffers); ++i) + { + transition_sub_resource_state(command_list, output_buffers[i], 0, + D3D12_RESOURCE_STATE_UNORDERED_ACCESS, D3D12_RESOURCE_STATE_COPY_SOURCE); + check_buffer_uint(output_buffers[i], queue, command_list, uav_data[i], 0); + reset_command_list(command_list, context.allocator); + } + +done: + for (i = 0; i < ARRAY_SIZE(output_buffers); ++i) + ID3D12Resource_Release(output_buffers[i]); + for (i = 0; i < ARRAY_SIZE(input_buffers); ++i) + ID3D12Resource_Release(input_buffers[i]); + ID3D12DescriptorHeap_Release(heap); + destroy_test_context(&context); +} + START_TEST(d3d12) { parse_args(argc, argv); @@ -33985,4 +34177,5 @@ START_TEST(d3d12) run_test(test_sampler_register_space); run_test(test_hull_shader_relative_addressing); run_test(test_hull_shader_patch_constant_inputs); + run_test(test_resource_arrays); } -- 2.31.1
Based on register loading code from Henri Verbeet <hverbeet(a)codeweavers.com> Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxbc.c | 121 ++++++++++++++++------- libs/vkd3d-shader/vkd3d_shader_main.c | 17 ++-- libs/vkd3d-shader/vkd3d_shader_private.h | 8 +- 3 files changed, 97 insertions(+), 49 deletions(-) diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index d2cf87e3..ad1db82a 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -622,12 +622,24 @@ static void shader_sm4_read_shader_data(struct vkd3d_shader_instruction *ins, ins->declaration.icb = &priv->icb; } -static unsigned int shader_sm4_map_resource_idx(struct vkd3d_shader_register *reg, const struct vkd3d_sm4_data *priv) +static void shader_sm4_read_register_indices(struct vkd3d_shader_resource *resource, + struct vkd3d_shader_instruction *ins, struct vkd3d_sm4_data *priv) { if (shader_is_sm_5_1(priv)) - return reg->idx[1].offset; + { + resource->register_first = resource->reg.reg.idx[1].offset; + resource->register_last = resource->reg.reg.idx[2].offset; + if (resource->register_last < resource->register_first) + { + FIXME("Invalid register range [%u:%u].\n", resource->register_first, resource->register_last); + ins->handler_idx = VKD3DSIH_INVALID; + } + } else - return reg->idx[0].offset; + { + resource->register_first = resource->reg.reg.idx[0].offset; + resource->register_last = resource->register_first; + } } static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins, @@ -635,6 +647,7 @@ static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins, struct vkd3d_sm4_data *priv) { struct vkd3d_shader_semantic *semantic = &ins->declaration.semantic; + struct vkd3d_shader_resource *resource = &semantic->resource; enum vkd3d_sm4_resource_type resource_type; const DWORD *end = &tokens[token_count]; enum vkd3d_sm4_data_type data_type; @@ -653,8 +666,7 @@ static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins, semantic->resource_type = resource_type_table[resource_type]; } reg_data_type = opcode == VKD3D_SM4_OP_DCL_RESOURCE ? VKD3D_DATA_RESOURCE : VKD3D_DATA_UAV; - shader_sm4_read_dst_param(priv, &tokens, end, reg_data_type, &semantic->resource.reg); - semantic->resource.register_index = shader_sm4_map_resource_idx(&semantic->resource.reg.reg, priv); + shader_sm4_read_dst_param(priv, &tokens, end, reg_data_type, &resource->reg); components = *tokens++; for (i = 0; i < VKD3D_VEC4_SIZE; i++) @@ -675,23 +687,21 @@ static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins, if (reg_data_type == VKD3D_DATA_UAV) ins->flags = (opcode_token & VKD3D_SM5_UAV_FLAGS_MASK) >> VKD3D_SM5_UAV_FLAGS_SHIFT; - shader_sm4_read_register_space(priv, &tokens, end, &semantic->resource.register_space); + shader_sm4_read_register_space(priv, &tokens, end, &resource->register_space); + shader_sm4_read_register_indices(resource, ins, priv); } static void shader_sm4_read_dcl_constant_buffer(struct vkd3d_shader_instruction *ins, DWORD opcode, DWORD opcode_token, const DWORD *tokens, unsigned int token_count, struct vkd3d_sm4_data *priv) { + struct vkd3d_shader_constant_buffer *cb = &ins->declaration.cb; const DWORD *end = &tokens[token_count]; - shader_sm4_read_src_param(priv, &tokens, end, VKD3D_DATA_FLOAT, &ins->declaration.cb.src); - ins->declaration.cb.register_index = shader_sm4_map_resource_idx(&ins->declaration.cb.src.reg, priv); if (opcode_token & VKD3D_SM4_INDEX_TYPE_MASK) ins->flags |= VKD3DSI_INDEXED_DYNAMIC; - ins->declaration.cb.size = ins->declaration.cb.src.reg.idx[2].offset; - ins->declaration.cb.register_space = 0; - + shader_sm4_read_src_param(priv, &tokens, end, VKD3D_DATA_FLOAT, &cb->src); if (shader_is_sm_5_1(priv)) { if (tokens >= end) @@ -700,8 +710,22 @@ static void shader_sm4_read_dcl_constant_buffer(struct vkd3d_shader_instruction return; } - ins->declaration.cb.size = *tokens++; - shader_sm4_read_register_space(priv, &tokens, end, &ins->declaration.cb.register_space); + cb->size = *tokens++; + shader_sm4_read_register_space(priv, &tokens, end, &cb->register_space); + cb->register_first = cb->src.reg.idx[1].offset; + cb->register_last = cb->src.reg.idx[2].offset; + if (cb->register_last < cb->register_first) + { + FIXME("Invalid register range [%u:%u].\n", cb->register_first, cb->register_last); + ins->handler_idx = VKD3DSIH_INVALID; + } + } + else + { + cb->size = cb->src.reg.idx[2].offset; + cb->register_space = 0; + cb->register_first = cb->src.reg.idx[0].offset; + cb->register_last = cb->register_first; } } @@ -709,14 +733,31 @@ static void shader_sm4_read_dcl_sampler(struct vkd3d_shader_instruction *ins, DWORD opcode, DWORD opcode_token, const DWORD *tokens, unsigned int token_count, struct vkd3d_sm4_data *priv) { + struct vkd3d_shader_sampler *sampler = &ins->declaration.sampler; const DWORD *end = &tokens[token_count]; ins->flags = (opcode_token & VKD3D_SM4_SAMPLER_MODE_MASK) >> VKD3D_SM4_SAMPLER_MODE_SHIFT; if (ins->flags & ~VKD3D_SM4_SAMPLER_COMPARISON) FIXME("Unhandled sampler mode %#x.\n", ins->flags); - shader_sm4_read_src_param(priv, &tokens, end, VKD3D_DATA_SAMPLER, &ins->declaration.sampler.src); - ins->declaration.sampler.register_index = shader_sm4_map_resource_idx(&ins->declaration.sampler.src.reg, priv); - shader_sm4_read_register_space(priv, &tokens, end, &ins->declaration.sampler.register_space); + + shader_sm4_read_src_param(priv, &tokens, end, VKD3D_DATA_SAMPLER, &sampler->src); + if (shader_is_sm_5_1(priv)) + { + shader_sm4_read_register_space(priv, &tokens, end, &sampler->register_space); + sampler->register_first = sampler->src.reg.idx[1].offset; + sampler->register_last = sampler->src.reg.idx[2].offset; + if (sampler->register_last < sampler->register_first) + { + FIXME("Invalid register range [%u:%u].\n", sampler->register_first, sampler->register_last); + ins->handler_idx = VKD3DSIH_INVALID; + } + } + else + { + sampler->register_space = 0; + sampler->register_first = sampler->src.reg.idx[0].offset; + sampler->register_last = sampler->register_first; + } } static void shader_sm4_read_dcl_index_range(struct vkd3d_shader_instruction *ins, @@ -912,29 +953,32 @@ static void shader_sm5_read_dcl_uav_raw(struct vkd3d_shader_instruction *ins, DWORD opcode, DWORD opcode_token, const DWORD *tokens, unsigned int token_count, struct vkd3d_sm4_data *priv) { - struct vkd3d_shader_raw_resource *resource = &ins->declaration.raw_resource; + struct vkd3d_shader_resource *resource = &ins->declaration.raw_resource.resource; const DWORD *end = &tokens[token_count]; - shader_sm4_read_dst_param(priv, &tokens, end, VKD3D_DATA_UAV, &resource->resource.reg); - resource->resource.register_index = shader_sm4_map_resource_idx(&resource->resource.reg.reg, priv); ins->flags = (opcode_token & VKD3D_SM5_UAV_FLAGS_MASK) >> VKD3D_SM5_UAV_FLAGS_SHIFT; - shader_sm4_read_register_space(priv, &tokens, end, &resource->resource.register_space); + + shader_sm4_read_dst_param(priv, &tokens, end, VKD3D_DATA_UAV, &resource->reg); + shader_sm4_read_register_space(priv, &tokens, end, &resource->register_space); + shader_sm4_read_register_indices(resource, ins, priv); } static void shader_sm5_read_dcl_uav_structured(struct vkd3d_shader_instruction *ins, DWORD opcode, DWORD opcode_token, const DWORD *tokens, unsigned int token_count, struct vkd3d_sm4_data *priv) { - struct vkd3d_shader_structured_resource *resource = &ins->declaration.structured_resource; + struct vkd3d_shader_structured_resource *structured = &ins->declaration.structured_resource; + struct vkd3d_shader_resource *resource = &structured->resource; const DWORD *end = &tokens[token_count]; - shader_sm4_read_dst_param(priv, &tokens, end, VKD3D_DATA_UAV, &resource->resource.reg); - resource->resource.register_index = shader_sm4_map_resource_idx(&resource->resource.reg.reg, priv); ins->flags = (opcode_token & VKD3D_SM5_UAV_FLAGS_MASK) >> VKD3D_SM5_UAV_FLAGS_SHIFT; - resource->byte_stride = *tokens++; - if (resource->byte_stride % 4) - FIXME("Byte stride %u is not multiple of 4.\n", resource->byte_stride); - shader_sm4_read_register_space(priv, &tokens, end, &resource->resource.register_space); + + shader_sm4_read_dst_param(priv, &tokens, end, VKD3D_DATA_UAV, &resource->reg); + structured->byte_stride = *tokens++; + if (structured->byte_stride % 4) + FIXME("Byte stride %u is not multiple of 4.\n", structured->byte_stride); + shader_sm4_read_register_space(priv, &tokens, end, &resource->register_space); + shader_sm4_read_register_indices(resource, ins, priv); } static void shader_sm5_read_dcl_tgsm_raw(struct vkd3d_shader_instruction *ins, @@ -963,27 +1007,28 @@ static void shader_sm5_read_dcl_resource_structured(struct vkd3d_shader_instruct DWORD opcode, DWORD opcode_token, const DWORD *tokens, unsigned int token_count, struct vkd3d_sm4_data *priv) { - struct vkd3d_shader_structured_resource *resource = &ins->declaration.structured_resource; + struct vkd3d_shader_structured_resource *structured = &ins->declaration.structured_resource; + struct vkd3d_shader_resource *resource = &structured->resource; const DWORD *end = &tokens[token_count]; - shader_sm4_read_dst_param(priv, &tokens, end, VKD3D_DATA_RESOURCE, &resource->resource.reg); - resource->resource.register_index = shader_sm4_map_resource_idx(&resource->resource.reg.reg, priv); - resource->byte_stride = *tokens++; - if (resource->byte_stride % 4) - FIXME("Byte stride %u is not multiple of 4.\n", resource->byte_stride); - shader_sm4_read_register_space(priv, &tokens, end, &resource->resource.register_space); + shader_sm4_read_dst_param(priv, &tokens, end, VKD3D_DATA_RESOURCE, &resource->reg); + structured->byte_stride = *tokens++; + if (structured->byte_stride % 4) + FIXME("Byte stride %u is not multiple of 4.\n", structured->byte_stride); + shader_sm4_read_register_space(priv, &tokens, end, &resource->register_space); + shader_sm4_read_register_indices(resource, ins, priv); } static void shader_sm5_read_dcl_resource_raw(struct vkd3d_shader_instruction *ins, DWORD opcode, DWORD opcode_token, const DWORD *tokens, unsigned int token_count, struct vkd3d_sm4_data *priv) { - struct vkd3d_shader_raw_resource *resource = &ins->declaration.raw_resource; + struct vkd3d_shader_resource *resource = &ins->declaration.raw_resource.resource; const DWORD *end = &tokens[token_count]; - shader_sm4_read_dst_param(priv, &tokens, end, VKD3D_DATA_RESOURCE, &resource->resource.reg); - resource->resource.register_index = shader_sm4_map_resource_idx(&resource->resource.reg.reg, priv); - shader_sm4_read_register_space(priv, &tokens, end, &resource->resource.register_space); + shader_sm4_read_dst_param(priv, &tokens, end, VKD3D_DATA_RESOURCE, &resource->reg); + shader_sm4_read_register_space(priv, &tokens, end, &resource->register_space); + shader_sm4_read_register_indices(resource, ins, priv); } static void shader_sm5_read_sync(struct vkd3d_shader_instruction *ins, diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 2308b894..31029ad9 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -580,9 +580,9 @@ static void vkd3d_shader_scan_record_uav_counter(struct vkd3d_shader_scan_contex } static bool vkd3d_shader_scan_add_descriptor(struct vkd3d_shader_scan_context *context, - enum vkd3d_shader_descriptor_type type, unsigned int register_space, unsigned int register_index, - enum vkd3d_shader_resource_type resource_type, enum vkd3d_shader_resource_data_type resource_data_type, - unsigned int flags) + enum vkd3d_shader_descriptor_type type, unsigned int register_space, unsigned int register_first, + unsigned int register_last, enum vkd3d_shader_resource_type resource_type, + enum vkd3d_shader_resource_data_type resource_data_type, unsigned int flags) { struct vkd3d_shader_scan_descriptor_info *info = context->scan_descriptor_info; struct vkd3d_shader_descriptor_info *d; @@ -597,11 +597,11 @@ static bool vkd3d_shader_scan_add_descriptor(struct vkd3d_shader_scan_context *c d = &info->descriptors[info->descriptor_count]; d->type = type; d->register_space = register_space; - d->register_index = register_index; + d->register_index = register_first; d->resource_type = resource_type; d->resource_data_type = resource_data_type; d->flags = flags; - d->count = 1; + d->count = (register_last == ~0u) ? ~0u : register_last - register_first + 1; ++info->descriptor_count; return true; @@ -633,7 +633,7 @@ static void vkd3d_shader_scan_constant_buffer_declaration(struct vkd3d_shader_sc return; vkd3d_shader_scan_add_descriptor(context, VKD3D_SHADER_DESCRIPTOR_TYPE_CBV, cb->register_space, - cb->register_index, VKD3D_SHADER_RESOURCE_BUFFER, VKD3D_SHADER_RESOURCE_DATA_UINT, 0); + cb->register_first, cb->register_last, VKD3D_SHADER_RESOURCE_BUFFER, VKD3D_SHADER_RESOURCE_DATA_UINT, 0); } static void vkd3d_shader_scan_sampler_declaration(struct vkd3d_shader_scan_context *context, @@ -650,7 +650,8 @@ static void vkd3d_shader_scan_sampler_declaration(struct vkd3d_shader_scan_conte else flags = 0; vkd3d_shader_scan_add_descriptor(context, VKD3D_SHADER_DESCRIPTOR_TYPE_SAMPLER, sampler->register_space, - sampler->register_index, VKD3D_SHADER_RESOURCE_NONE, VKD3D_SHADER_RESOURCE_DATA_UINT, flags); + sampler->register_first, sampler->register_last, VKD3D_SHADER_RESOURCE_NONE, + VKD3D_SHADER_RESOURCE_DATA_UINT, flags); } static void vkd3d_shader_scan_resource_declaration(struct vkd3d_shader_scan_context *context, @@ -667,7 +668,7 @@ static void vkd3d_shader_scan_resource_declaration(struct vkd3d_shader_scan_cont else type = VKD3D_SHADER_DESCRIPTOR_TYPE_SRV; vkd3d_shader_scan_add_descriptor(context, type, resource->register_space, - resource->register_index, resource_type, resource_data_type, 0); + resource->register_first, resource->register_last, resource_type, resource_data_type, 0); if (type == VKD3D_SHADER_DESCRIPTOR_TYPE_UAV) vkd3d_shader_scan_add_uav_range(context, resource->reg.reg.idx[0].offset, context->scan_descriptor_info->descriptor_count - 1); diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 6d756e40..54930fa2 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -640,7 +640,7 @@ struct vkd3d_shader_resource { struct vkd3d_shader_dst_param reg; unsigned int register_space; - unsigned int register_index; + unsigned int register_first, register_last; }; enum vkd3d_decl_usage @@ -715,14 +715,16 @@ struct vkd3d_shader_register_semantic struct vkd3d_shader_sampler { struct vkd3d_shader_src_param src; - unsigned int register_space, register_index; + unsigned int register_space; + unsigned int register_first, register_last; }; struct vkd3d_shader_constant_buffer { struct vkd3d_shader_src_param src; unsigned int size; - unsigned int register_space, register_index; + unsigned int register_space; + unsigned int register_first, register_last; }; struct vkd3d_shader_structured_resource -- 2.31.1
On Thu, 3 Jun 2021 at 07:02, Conor McCarthy <cmccarthy(a)codeweavers.com> wrote:
-static unsigned int shader_sm4_map_resource_idx(struct vkd3d_shader_register *reg, const struct vkd3d_sm4_data *priv) +static void shader_sm4_read_register_indices(struct vkd3d_shader_resource *resource, + struct vkd3d_shader_instruction *ins, struct vkd3d_sm4_data *priv) { if (shader_is_sm_5_1(priv)) - return reg->idx[1].offset; + { + resource->register_first = resource->reg.reg.idx[1].offset; + resource->register_last = resource->reg.reg.idx[2].offset; + if (resource->register_last < resource->register_first) + { + FIXME("Invalid register range [%u:%u].\n", resource->register_first, resource->register_last); + ins->handler_idx = VKD3DSIH_INVALID; + } + } else - return reg->idx[0].offset; + { + resource->register_first = resource->reg.reg.idx[0].offset; + resource->register_last = resource->register_first; + } }
I don't think shader_sm4_read_register_indices() is a particularly great name for what this does. In particular, actual register indices (i.e., the "idx" field in the vkd3d_shader_register structure) are read as part of shader_sm4_read_param(), and this resolves descriptor/resource array ranges instead. Error reporting should happen through vkd3d_shader_verror() instead, so that we can report a proper compiler message to the application. (See also e.g. hlsl_error(), preproc_error(), vkd3d_dxbc_compiler_error(), etc. for examples.) We do of course have existing code in the SM4 parser that still uses VKD3DSIH_INVALID and/or FIXMEs/ERRs; it would be nice to get those cleaned up as well.
static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins, @@ -635,6 +647,7 @@ static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins, struct vkd3d_sm4_data *priv) { struct vkd3d_shader_semantic *semantic = &ins->declaration.semantic; + struct vkd3d_shader_resource *resource = &semantic->resource; enum vkd3d_sm4_resource_type resource_type; const DWORD *end = &tokens[token_count]; enum vkd3d_sm4_data_type data_type; @@ -653,8 +666,7 @@ static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins, semantic->resource_type = resource_type_table[resource_type]; } reg_data_type = opcode == VKD3D_SM4_OP_DCL_RESOURCE ? VKD3D_DATA_RESOURCE : VKD3D_DATA_UAV; - shader_sm4_read_dst_param(priv, &tokens, end, reg_data_type, &semantic->resource.reg); - semantic->resource.register_index = shader_sm4_map_resource_idx(&semantic->resource.reg.reg, priv); + shader_sm4_read_dst_param(priv, &tokens, end, reg_data_type, &resource->reg);
components = *tokens++; for (i = 0; i < VKD3D_VEC4_SIZE; i++) @@ -675,23 +687,21 @@ static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins, if (reg_data_type == VKD3D_DATA_UAV) ins->flags = (opcode_token & VKD3D_SM5_UAV_FLAGS_MASK) >> VKD3D_SM5_UAV_FLAGS_SHIFT;
- shader_sm4_read_register_space(priv, &tokens, end, &semantic->resource.register_space); + shader_sm4_read_register_space(priv, &tokens, end, &resource->register_space); + shader_sm4_read_register_indices(resource, ins, priv); }
An easy way to split this patch would be to introduce the changes for each instruction as a separate patch.
@@ -640,7 +640,7 @@ struct vkd3d_shader_resource { struct vkd3d_shader_dst_param reg; unsigned int register_space; - unsigned int register_index; + unsigned int register_first, register_last; };
enum vkd3d_decl_usage @@ -715,14 +715,16 @@ struct vkd3d_shader_register_semantic struct vkd3d_shader_sampler { struct vkd3d_shader_src_param src; - unsigned int register_space, register_index; + unsigned int register_space; + unsigned int register_first, register_last; };
struct vkd3d_shader_constant_buffer { struct vkd3d_shader_src_param src; unsigned int size; - unsigned int register_space, register_index; + unsigned int register_space; + unsigned int register_first, register_last; };
This will fail to compile; there are references to "register_index" in e.g. spirv.c. In any case, it seems like it may be helpful to introduce a structure like this: struct vkd3d_shader_descriptor_range { unsigned int space; unsigned int first, last; }; You could then do something like the following instead of shader_sm4_read_register(): static void shader_sm4_resolve_descriptor_range(struct vkd3d_sm4_data *priv, const struct vkd3d_shader_register *reg, struct vkd3d_shader_descriptor_range *range) { [...] } and also use that structure for e.g. vkd3d_shader_scan_add_descriptor(). It would also be nice to make the corresponding changes to trace.c as well, so that descriptor ranges are also properly disassembled.
A SPIR-V array is declared if binding.count > 1 or the dxbc declares an unbounded array. Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- include/vkd3d_shader.h | 6 +- libs/vkd3d-shader/spirv.c | 356 ++++++++++++++++++++++++++++++------- libs/vkd3d/device.c | 4 + libs/vkd3d/vkd3d_private.h | 2 +- 4 files changed, 300 insertions(+), 68 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 03225d37..922d0275 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -208,8 +208,9 @@ struct vkd3d_shader_descriptor_binding /** The binding index of the descriptor. */ unsigned int binding; /** - * The size of this descriptor array. Descriptor arrays are not supported in - * this version of vkd3d-shader, and therefore this value must be 1. + * The size of this descriptor array. Descriptor arrays are supported in + * this version of vkd3d-shader, but arrayed UAV counters and non-uniform + * array indexing are not. */ unsigned int count; }; @@ -573,6 +574,7 @@ enum vkd3d_shader_spirv_extension { VKD3D_SHADER_SPIRV_EXTENSION_NONE, VKD3D_SHADER_SPIRV_EXTENSION_EXT_DEMOTE_TO_HELPER_INVOCATION, + VKD3D_SHADER_SPIRV_EXTENSION_EXT_DESCRIPTOR_INDEXING, VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_SPIRV_EXTENSION), }; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 0e75b0ae..cf2e5829 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -1,5 +1,6 @@ /* * Copyright 2017 Józef Kucia for CodeWeavers + * Copyright 2021 Conor McCarthy for Codeweavers * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -335,6 +336,7 @@ struct vkd3d_spirv_builder uint64_t capability_mask; uint64_t capability_draw_parameters : 1; uint64_t capability_demote_to_helper_invocation : 1; + uint64_t capability_descriptor_indexing : 1; uint32_t ext_instr_set_glsl_450; uint32_t invocation_count; SpvExecutionModel execution_model; @@ -391,6 +393,11 @@ static void vkd3d_spirv_enable_capability(struct vkd3d_spirv_builder *builder, } } +static void vkd3d_spirv_enable_descriptor_indexing(struct vkd3d_spirv_builder *builder) +{ + builder->capability_descriptor_indexing = 1; +} + static uint32_t vkd3d_spirv_get_glsl_std450_instr_set(struct vkd3d_spirv_builder *builder) { if (!builder->ext_instr_set_glsl_450) @@ -1808,6 +1815,8 @@ static bool vkd3d_spirv_compile_module(struct vkd3d_spirv_builder *builder, vkd3d_spirv_build_op_extension(&stream, "SPV_KHR_shader_draw_parameters"); if (builder->capability_demote_to_helper_invocation) vkd3d_spirv_build_op_extension(&stream, "SPV_EXT_demote_to_helper_invocation"); + if (builder->capability_descriptor_indexing) + vkd3d_spirv_build_op_extension(&stream, "SPV_EXT_descriptor_indexing"); if (builder->ext_instr_set_glsl_450) vkd3d_spirv_build_op_ext_inst_import(&stream, builder->ext_instr_set_glsl_450, "GLSL.std.450"); @@ -1926,6 +1935,7 @@ struct vkd3d_symbol_register_data unsigned int write_mask; uint32_t dcl_mask; unsigned int structure_stride; + unsigned int binding_base_idx; bool is_aggregate; /* An aggregate, i.e. a structure or an array. */ bool is_dynamically_indexed; /* If member_idx is a variable ID instead of a constant. */ }; @@ -1936,6 +1946,9 @@ struct vkd3d_symbol_resource_data unsigned int register_index; enum vkd3d_shader_component_type sampled_type; uint32_t type_id; + uint32_t contained_type_id; + unsigned int binding_base_idx; + SpvStorageClass storage_class; const struct vkd3d_spirv_resource_type *resource_type_info; unsigned int structure_stride; bool raw; @@ -1977,6 +1990,35 @@ struct vkd3d_symbol } info; }; +struct vkd3d_array_variable_key +{ + uint32_t ptr_type_id; + unsigned int set; + unsigned int binding; +}; + +struct vkd3d_array_variable +{ + struct rb_entry entry; + struct vkd3d_array_variable_key key; + uint32_t id; +}; + +static int vkd3d_array_variable_compare(const void *key, const struct rb_entry *entry) +{ + const struct vkd3d_array_variable *a = key; + const struct vkd3d_array_variable *b = RB_ENTRY_VALUE(entry, const struct vkd3d_array_variable, entry); + + return memcmp(&a->key, &b->key, sizeof(a->key)); +} + +static void vkd3d_array_variable_free(struct rb_entry *entry, void *context) +{ + struct vkd3d_array_variable *s = RB_ENTRY_VALUE(entry, struct vkd3d_array_variable, entry); + + vkd3d_free(s); +} + static int vkd3d_symbol_compare(const void *key, const struct rb_entry *entry) { const struct vkd3d_symbol *a = key; @@ -2012,6 +2054,7 @@ static void vkd3d_symbol_set_register_info(struct vkd3d_symbol *symbol, { symbol->id = val_id; symbol->info.reg.storage_class = storage_class; + symbol->info.reg.binding_base_idx = ~0u; symbol->info.reg.member_idx = 0; symbol->info.reg.component_type = component_type; symbol->info.reg.write_mask = write_mask; @@ -2077,6 +2120,14 @@ static const char *debug_vkd3d_symbol(const struct vkd3d_symbol *symbol) } } +static const char *debug_register_range(unsigned int register_first, unsigned int register_last, bool bounded) +{ + if (bounded) + return vkd3d_dbg_sprintf("[%u:%u]", register_first, register_last); + else + return vkd3d_dbg_sprintf("[%u:*]", register_first); +} + struct vkd3d_if_cf_info { size_t stream_location; @@ -2164,11 +2215,13 @@ struct vkd3d_dxbc_compiler bool ssbo_uavs; struct rb_tree symbol_table; + struct rb_tree array_var_table; uint32_t temp_id; unsigned int temp_count; struct vkd3d_hull_shader_variables hs; uint32_t sample_positions_id; + struct vkd3d_shader_version shader_version; enum vkd3d_shader_type shader_type; unsigned int branch_id; @@ -2216,6 +2269,11 @@ struct vkd3d_dxbc_compiler enum vkd3d_shader_compile_option_formatting_flags formatting; }; +static bool shader_is_sm_5_1(const struct vkd3d_dxbc_compiler *compiler) +{ + return (((unsigned int)compiler->shader_version.major << 8) + compiler->shader_version.minor) >= 0x501; +} + static bool is_control_point_phase(const struct vkd3d_shader_phase *phase) { return phase && phase->type == VKD3DSIH_HS_CONTROL_POINT_PHASE; @@ -2309,7 +2367,9 @@ struct vkd3d_dxbc_compiler *vkd3d_dxbc_compiler_create(const struct vkd3d_shader } rb_init(&compiler->symbol_table, vkd3d_symbol_compare); + rb_init(&compiler->array_var_table, vkd3d_array_variable_compare); + compiler->shader_version = *shader_version; compiler->shader_type = shader_version->type; compiler->input_signature = &shader_desc->input_signature; @@ -2404,9 +2464,12 @@ static struct vkd3d_push_constant_buffer_binding *vkd3d_dxbc_compiler_find_push_ const struct vkd3d_dxbc_compiler *compiler, const struct vkd3d_shader_constant_buffer *cb) { unsigned int register_space = cb->register_space; - unsigned int reg_idx = cb->register_index; + unsigned int reg_idx = cb->register_first; unsigned int i; + if (cb->register_first != cb->register_last) + return NULL; + for (i = 0; i < compiler->shader_interface.push_constant_buffer_count; ++i) { struct vkd3d_push_constant_buffer_binding *current = &compiler->push_constants[i]; @@ -2431,7 +2494,10 @@ static bool vkd3d_dxbc_compiler_has_combined_sampler(const struct vkd3d_dxbc_com if (!shader_interface->combined_sampler_count) return false; - if (resource && resource->reg.reg.type == VKD3DSPR_UAV) + if (resource && (resource->reg.reg.type == VKD3DSPR_UAV || resource->register_last != resource->register_first)) + return false; + + if (sampler && sampler->register_first != sampler->register_last) return false; for (i = 0; i < shader_interface->combined_sampler_count; ++i) @@ -2442,9 +2508,9 @@ static bool vkd3d_dxbc_compiler_has_combined_sampler(const struct vkd3d_dxbc_com continue; if ((!resource || (combined_sampler->resource_space == resource->register_space - && combined_sampler->resource_index == resource->register_index)) + && combined_sampler->resource_index == resource->register_first)) && (!sampler || (combined_sampler->sampler_space == sampler->register_space - && combined_sampler->sampler_index == sampler->register_index))) + && combined_sampler->sampler_index == sampler->register_first))) return true; } @@ -2464,12 +2530,14 @@ static void VKD3D_PRINTF_FUNC(3, 4) vkd3d_dxbc_compiler_error(struct vkd3d_dxbc_ static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor_binding( struct vkd3d_dxbc_compiler *compiler, const struct vkd3d_shader_register *reg, unsigned int register_space, - unsigned int reg_idx, enum vkd3d_shader_resource_type resource_type, bool is_uav_counter) + unsigned int register_first, unsigned int register_last, enum vkd3d_shader_resource_type resource_type, + bool is_uav_counter, unsigned int *binding_base_idx) { const struct vkd3d_shader_interface_info *shader_interface = &compiler->shader_interface; enum vkd3d_shader_descriptor_type descriptor_type; enum vkd3d_shader_binding_flag resource_type_flag; struct vkd3d_shader_descriptor_binding binding; + bool bounded = true; unsigned int i; if (reg->type == VKD3DSPR_CONSTBUFFER) @@ -2491,6 +2559,12 @@ static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor resource_type_flag = resource_type == VKD3D_SHADER_RESOURCE_BUFFER ? VKD3D_SHADER_BINDING_FLAG_BUFFER : VKD3D_SHADER_BINDING_FLAG_IMAGE; + if (register_last == ~0u) + { + bounded = false; + register_last = register_first; + } + if (is_uav_counter) { assert(descriptor_type == VKD3D_SHADER_DESCRIPTOR_TYPE_UAV); @@ -2501,32 +2575,37 @@ static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor if (!vkd3d_dxbc_compiler_check_shader_visibility(compiler, current->shader_visibility)) continue; - if (current->register_space != register_space || current->register_index != reg_idx) + if (current->register_space != register_space || current->register_index > register_first + || current->register_index + current->binding.count <= register_last) continue; if (current->offset) { FIXME("Atomic counter offsets are not supported yet.\n"); vkd3d_dxbc_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INVALID_DESCRIPTOR_BINDING, - "Descriptor binding for UAV counter %u, space %u has unsupported ‘offset’ %u.", - reg_idx, register_space, current->offset); + "Descriptor binding for UAV counter %s, space %u has unsupported ‘offset’ %u.", + debug_register_range(register_first, register_last, bounded), + register_space, current->offset); } - if (current->binding.count != 1) + if (current->binding.count != 1 || !bounded) { FIXME("Descriptor arrays are not supported.\n"); vkd3d_dxbc_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INVALID_DESCRIPTOR_BINDING, "Descriptor binding for UAV counter %u, space %u has unsupported ‘count’ %u.", - reg_idx, register_space, current->binding.count); + register_first, register_space, current->binding.count); } + *binding_base_idx = ~0u; return current->binding; } if (shader_interface->uav_counter_count) { - FIXME("Could not find descriptor binding for UAV counter %u, space %u.\n", reg_idx, register_space); + FIXME("Could not find descriptor binding for UAV counters %s, space %u.\n", + debug_register_range(register_first, register_last, bounded), register_space); vkd3d_dxbc_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_DESCRIPTOR_BINDING_NOT_FOUND, - "Could not find descriptor binding for UAV counter %u, space %u.", reg_idx, register_space); + "Could not find descriptor binding for UAV counters %s, space %u.", + debug_register_range(register_first, register_last, bounded), register_space); } } else @@ -2542,31 +2621,27 @@ static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor continue; if (current->type != descriptor_type || current->register_space != register_space - || current->register_index != reg_idx) + || current->register_index > register_first + || current->register_index + current->binding.count <= register_last) continue; - if (current->binding.count != 1) - { - FIXME("Descriptor arrays are not supported.\n"); - vkd3d_dxbc_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INVALID_DESCRIPTOR_BINDING, - "Descriptor binding for type %#x, space %u, register %u, " - "shader type %#x has unsupported ‘count’ %u.", - descriptor_type, register_space, reg_idx, compiler->shader_type, current->binding.count); - } - + *binding_base_idx = (current->binding.count != 1 || !bounded) ? current->register_index : ~0u; return current->binding; } if (shader_interface->binding_count) { - FIXME("Could not find binding for type %#x, space %u, register %u, shader type %#x.\n", - descriptor_type, register_space, reg_idx, compiler->shader_type); + FIXME("Could not find binding for type %#x, space %u, registers %s, shader type %#x.\n", + descriptor_type, register_space, debug_register_range(register_first, register_last, bounded), + compiler->shader_type); vkd3d_dxbc_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_DESCRIPTOR_BINDING_NOT_FOUND, - "Could not find descriptor binding for type %#x, space %u, register %u, shader type %#x.", - descriptor_type, register_space, reg_idx, compiler->shader_type); + "Could not find descriptor binding for type %#x, space %u, registers %s, shader type %#x.", + descriptor_type, register_space, debug_register_range(register_first, register_last, bounded), + compiler->shader_type); } } done: + *binding_base_idx = ~0u; binding.set = 0; binding.count = 1; binding.binding = compiler->binding_idx++; @@ -2584,12 +2659,14 @@ static void vkd3d_dxbc_compiler_emit_descriptor_binding(struct vkd3d_dxbc_compil static void vkd3d_dxbc_compiler_emit_descriptor_binding_for_reg(struct vkd3d_dxbc_compiler *compiler, uint32_t variable_id, const struct vkd3d_shader_register *reg, unsigned int register_space, - unsigned int register_index, enum vkd3d_shader_resource_type resource_type, bool is_uav_counter) + unsigned int register_first, unsigned int register_last, enum vkd3d_shader_resource_type resource_type, + bool is_uav_counter) { struct vkd3d_shader_descriptor_binding binding; + unsigned int binding_base_idx; /* Not used because UAV counter arrays are not implemented. */ binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, register_space, - register_index, resource_type, is_uav_counter); + register_first, register_last, resource_type, is_uav_counter, &binding_base_idx); vkd3d_dxbc_compiler_emit_descriptor_binding(compiler, variable_id, &binding); } @@ -2814,6 +2891,66 @@ static uint32_t vkd3d_dxbc_compiler_emit_array_variable(struct vkd3d_dxbc_compil return vkd3d_spirv_build_op_variable(builder, stream, ptr_type_id, storage_class, 0); } +static uint32_t vkd3d_dxbc_compiler_get_resource_variable(struct vkd3d_dxbc_compiler *compiler, + SpvStorageClass storage_class, uint32_t ptr_type_id, const struct vkd3d_shader_register *reg, + const struct vkd3d_shader_descriptor_binding *binding, bool array_variable) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + + if (array_variable) + { + struct vkd3d_array_variable *array_var_entry; + struct vkd3d_array_variable array_var; + struct rb_entry *entry; + + /* Declare one array variable per Vulkan binding, and use it for all array declarations + * which map to it. In this case ptr_type_id must point to an array type. */ + array_var.key.ptr_type_id = ptr_type_id; + array_var.key.set = binding->set; + array_var.key.binding = binding->binding; + if ((entry = rb_get(&compiler->array_var_table, &array_var))) + { + array_var_entry = RB_ENTRY_VALUE(entry, struct vkd3d_array_variable, entry); + return array_var_entry->id; + } + + array_var.id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream, + ptr_type_id, storage_class, 0); + vkd3d_dxbc_compiler_emit_descriptor_binding(compiler, array_var.id, binding); + vkd3d_dxbc_compiler_emit_register_debug_name(builder, array_var.id, reg); + + array_var_entry = vkd3d_malloc(sizeof(*array_var_entry)); + memcpy(array_var_entry, &array_var, sizeof(*array_var_entry)); + rb_put(&compiler->array_var_table, array_var_entry, &array_var_entry->entry); + + return array_var.id; + } + else + { + uint32_t var_id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream, + ptr_type_id, storage_class, 0); + vkd3d_dxbc_compiler_emit_descriptor_binding(compiler, var_id, binding); + vkd3d_dxbc_compiler_emit_register_debug_name(builder, var_id, reg); + return var_id; + } +} + +static uint32_t vkd3d_dxbc_compiler_build_resource_variable(struct vkd3d_dxbc_compiler *compiler, + SpvStorageClass storage_class, uint32_t type_id, const struct vkd3d_shader_register *reg, + const struct vkd3d_shader_descriptor_binding *binding, bool array_variable) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + uint32_t ptr_type_id; + + if (array_variable) + type_id = vkd3d_spirv_get_op_type_array(builder, type_id, + vkd3d_dxbc_compiler_get_constant_uint(compiler, binding->count)); + ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, storage_class, type_id); + + return vkd3d_dxbc_compiler_get_resource_variable(compiler, storage_class, + ptr_type_id, reg, binding, array_variable); +} + static const struct vkd3d_shader_parameter *vkd3d_dxbc_compiler_get_shader_parameter( struct vkd3d_dxbc_compiler *compiler, enum vkd3d_shader_parameter_name name) { @@ -2997,6 +3134,7 @@ struct vkd3d_shader_register_info SpvStorageClass storage_class; enum vkd3d_shader_component_type component_type; unsigned int write_mask; + unsigned int binding_base_idx; uint32_t member_idx; unsigned int structure_stride; bool is_aggregate; @@ -3016,6 +3154,7 @@ static bool vkd3d_dxbc_compiler_get_register_info(const struct vkd3d_dxbc_compil assert(reg->idx[0].offset < compiler->temp_count); register_info->id = compiler->temp_id + reg->idx[0].offset; register_info->storage_class = SpvStorageClassFunction; + register_info->binding_base_idx = ~0u; register_info->member_idx = 0; register_info->component_type = VKD3D_SHADER_COMPONENT_FLOAT; register_info->write_mask = VKD3DSP_WRITEMASK_ALL; @@ -3036,6 +3175,7 @@ static bool vkd3d_dxbc_compiler_get_register_info(const struct vkd3d_dxbc_compil symbol = RB_ENTRY_VALUE(entry, struct vkd3d_symbol, entry); register_info->id = symbol->id; register_info->storage_class = symbol->info.reg.storage_class; + register_info->binding_base_idx = symbol->info.reg.binding_base_idx; register_info->member_idx = symbol->info.reg.member_idx; register_info->component_type = symbol->info.reg.component_type; register_info->write_mask = symbol->info.reg.write_mask; @@ -3061,17 +3201,42 @@ static bool register_is_descriptor(const struct vkd3d_shader_register *reg) } } +static uint32_t vkd3d_dxbc_compiler_get_resource_index(struct vkd3d_dxbc_compiler *compiler, + const struct vkd3d_shader_register *reg, unsigned int binding_base_idx) +{ + uint32_t index_id; + + if (shader_is_sm_5_1(compiler)) + { + struct vkd3d_shader_register_index index = reg->idx[1]; + + if (index.rel_addr) + vkd3d_spirv_enable_descriptor_indexing(&compiler->spirv_builder); + + index.offset -= binding_base_idx; + index_id = vkd3d_dxbc_compiler_emit_register_addressing(compiler, &index); + } + else + { + index_id = vkd3d_dxbc_compiler_get_constant_uint(compiler, reg->idx[0].offset - binding_base_idx); + } + + return index_id; +} + static void vkd3d_dxbc_compiler_emit_dereference_register(struct vkd3d_dxbc_compiler *compiler, const struct vkd3d_shader_register *reg, struct vkd3d_shader_register_info *register_info) { struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; unsigned int component_count, index_count = 0; uint32_t type_id, ptr_type_id; - uint32_t indexes[2]; + uint32_t indexes[3]; if (reg->type == VKD3DSPR_CONSTBUFFER) { assert(!reg->idx[0].rel_addr); + if (register_info->binding_base_idx != ~0u) + indexes[index_count++] = vkd3d_dxbc_compiler_get_resource_index(compiler, reg, register_info->binding_base_idx); indexes[index_count++] = vkd3d_dxbc_compiler_get_constant_uint(compiler, register_info->member_idx); indexes[index_count++] = vkd3d_dxbc_compiler_emit_register_addressing(compiler, ®->idx[2]); } @@ -5268,13 +5433,15 @@ static void vkd3d_dxbc_compiler_emit_push_constant_buffers(struct vkd3d_dxbc_com static void vkd3d_dxbc_compiler_emit_dcl_constant_buffer(struct vkd3d_dxbc_compiler *compiler, const struct vkd3d_shader_instruction *instruction) { - uint32_t vec4_id, array_type_id, length_id, struct_id, pointer_type_id, var_id; + uint32_t vec4_id, array_type_id, length_id, struct_id, var_id; const struct vkd3d_shader_constant_buffer *cb = &instruction->declaration.cb; struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; const SpvStorageClass storage_class = SpvStorageClassUniform; const struct vkd3d_shader_register *reg = &cb->src.reg; struct vkd3d_push_constant_buffer_binding *push_cb; + struct vkd3d_shader_descriptor_binding binding; struct vkd3d_symbol reg_symbol; + unsigned int binding_base_idx; assert(!(instruction->flags & ~VKD3DSI_INDEXED_DYNAMIC)); @@ -5304,18 +5471,16 @@ static void vkd3d_dxbc_compiler_emit_dcl_constant_buffer(struct vkd3d_dxbc_compi vkd3d_spirv_build_op_member_decorate1(builder, struct_id, 0, SpvDecorationOffset, 0); vkd3d_spirv_build_op_name(builder, struct_id, "cb%u_struct", cb->size); - pointer_type_id = vkd3d_spirv_get_op_type_pointer(builder, storage_class, struct_id); - var_id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream, - pointer_type_id, storage_class, 0); + binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, cb->register_space, + cb->register_first, cb->register_last, VKD3D_SHADER_RESOURCE_BUFFER, false, &binding_base_idx); - vkd3d_dxbc_compiler_emit_descriptor_binding_for_reg(compiler, - var_id, reg, cb->register_space, cb->register_index, VKD3D_SHADER_RESOURCE_BUFFER, false); - - vkd3d_dxbc_compiler_emit_register_debug_name(builder, var_id, reg); + var_id = vkd3d_dxbc_compiler_build_resource_variable(compiler, storage_class, struct_id, + reg, &binding, binding_base_idx != ~0u || cb->register_last == ~0u); vkd3d_symbol_make_register(®_symbol, reg); vkd3d_symbol_set_register_info(®_symbol, var_id, storage_class, VKD3D_SHADER_COMPONENT_FLOAT, VKD3DSP_WRITEMASK_ALL); + reg_symbol.info.reg.binding_base_idx = binding_base_idx; vkd3d_dxbc_compiler_put_symbol(compiler, ®_symbol); } @@ -5359,30 +5524,30 @@ static void vkd3d_dxbc_compiler_emit_dcl_sampler(struct vkd3d_dxbc_compiler *com const SpvStorageClass storage_class = SpvStorageClassUniformConstant; struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; const struct vkd3d_shader_register *reg = &sampler->src.reg; - uint32_t type_id, ptr_type_id, var_id; + struct vkd3d_shader_descriptor_binding binding; + unsigned int binding_base_idx; + uint32_t type_id, var_id; struct vkd3d_symbol reg_symbol; vkd3d_symbol_make_sampler(®_symbol, reg); reg_symbol.info.sampler.register_space = sampler->register_space; - reg_symbol.info.sampler.register_index = sampler->register_index; + reg_symbol.info.sampler.register_index = sampler->register_first; vkd3d_dxbc_compiler_put_symbol(compiler, ®_symbol); if (vkd3d_dxbc_compiler_has_combined_sampler(compiler, NULL, sampler)) return; - type_id = vkd3d_spirv_get_op_type_sampler(builder); - ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, storage_class, type_id); - var_id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream, - ptr_type_id, storage_class, 0); - - vkd3d_dxbc_compiler_emit_descriptor_binding_for_reg(compiler, var_id, reg, - sampler->register_space, sampler->register_index, VKD3D_SHADER_RESOURCE_NONE, false); + binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, sampler->register_space, + sampler->register_first, sampler->register_last, VKD3D_SHADER_RESOURCE_NONE, false, &binding_base_idx); - vkd3d_dxbc_compiler_emit_register_debug_name(builder, var_id, reg); + type_id = vkd3d_spirv_get_op_type_sampler(builder); + var_id = vkd3d_dxbc_compiler_build_resource_variable(compiler, storage_class, + type_id, reg, &binding, binding_base_idx != ~0u || sampler->register_last == ~0u); vkd3d_symbol_make_register(®_symbol, reg); vkd3d_symbol_set_register_info(®_symbol, var_id, storage_class, VKD3D_SHADER_COMPONENT_FLOAT, VKD3DSP_WRITEMASK_ALL); + reg_symbol.info.reg.binding_base_idx = binding_base_idx; vkd3d_dxbc_compiler_put_symbol(compiler, ®_symbol); } @@ -5434,7 +5599,8 @@ static const struct vkd3d_shader_descriptor_info *vkd3d_dxbc_compiler_get_descri for (i = 0; i < descriptor_info->descriptor_count; ++i) { d = &descriptor_info->descriptors[i]; - if (d->type == type && d->register_space == register_space && d->register_index == register_index) + if (d->type == type && d->register_space == register_space && d->register_index <= register_index + && (d->count == ~0u || d->register_index + d->count > register_index)) return d; } @@ -5537,6 +5703,9 @@ static void vkd3d_dxbc_compiler_emit_combined_sampler_declarations(struct vkd3d_ symbol.info.resource.register_index = resource_index; symbol.info.resource.sampled_type = sampled_type; symbol.info.resource.type_id = image_type_id; + symbol.info.resource.contained_type_id = 0; + symbol.info.resource.binding_base_idx = ~0u; + symbol.info.resource.storage_class = storage_class; symbol.info.resource.resource_type_info = resource_type_info; symbol.info.resource.structure_stride = structure_stride; symbol.info.resource.raw = raw; @@ -5555,9 +5724,13 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp const struct vkd3d_shader_register *reg = &resource->reg.reg; const struct vkd3d_spirv_resource_type *resource_type_info; unsigned int register_space = resource->register_space; - unsigned int register_index = resource->register_index; + unsigned int register_first = resource->register_first; + unsigned int register_last = resource->register_last; + struct vkd3d_shader_descriptor_binding binding; enum vkd3d_shader_component_type sampled_type; struct vkd3d_symbol resource_symbol; + uint32_t contained_type_id = 0; + unsigned int binding_base_idx; bool is_uav; is_uav = reg->type == VKD3DSPR_UAV; @@ -5573,10 +5746,13 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp if (vkd3d_dxbc_compiler_has_combined_sampler(compiler, resource, NULL)) { vkd3d_dxbc_compiler_emit_combined_sampler_declarations(compiler, reg, register_space, - register_index, resource_type, sampled_type, structure_stride, raw, resource_type_info); + register_first, resource_type, sampled_type, structure_stride, raw, resource_type_info); return; } + binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, register_space, + register_first, register_last, resource_type, false, &binding_base_idx); + if (compiler->ssbo_uavs && is_uav && resource_type == VKD3D_SHADER_RESOURCE_BUFFER) { uint32_t array_type_id, struct_id; @@ -5596,24 +5772,27 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp else { type_id = vkd3d_dxbc_compiler_get_image_type_id(compiler, reg, register_space, - register_index, resource_type_info, sampled_type, structure_stride || raw, 0); + register_first, resource_type_info, sampled_type, structure_stride || raw, 0); + + if (binding_base_idx != ~0u || register_last == ~0u) + { + contained_type_id = type_id; + type_id = vkd3d_spirv_get_op_type_array(builder, type_id, + vkd3d_dxbc_compiler_get_constant_uint(compiler, binding.count)); + } } ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, storage_class, type_id); - var_id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream, - ptr_type_id, storage_class, 0); - vkd3d_dxbc_compiler_emit_descriptor_binding_for_reg(compiler, var_id, reg, - register_space, register_index, resource_type, false); - - vkd3d_dxbc_compiler_emit_register_debug_name(builder, var_id, reg); + var_id = vkd3d_dxbc_compiler_get_resource_variable(compiler, storage_class, + ptr_type_id, reg, &binding, binding_base_idx != ~0u || register_last == ~0u); if (is_uav) { const struct vkd3d_shader_descriptor_info *d; d = vkd3d_dxbc_compiler_get_descriptor_info(compiler, - VKD3D_SHADER_DESCRIPTOR_TYPE_UAV, register_space, register_index); + VKD3D_SHADER_DESCRIPTOR_TYPE_UAV, register_space, register_first); if (!(d->flags & VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_READ)) vkd3d_spirv_build_op_decorate(builder, var_id, SpvDecorationNonReadable, NULL, 0); @@ -5644,12 +5823,18 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp storage_class = SpvStorageClassUniform; ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, storage_class, struct_id); } + else if (contained_type_id) + { + /* TODO: Not needed after UAV counter arrays are implemented. */ + ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, storage_class, contained_type_id); + } counter_var_id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream, ptr_type_id, storage_class, 0); vkd3d_dxbc_compiler_emit_descriptor_binding_for_reg(compiler, - counter_var_id, reg, register_space, register_index, resource_type, true); + counter_var_id, reg, register_space, register_first, register_last, + resource_type, true); vkd3d_spirv_build_op_name(builder, counter_var_id, "u%u_counter", reg->idx[0].offset); } @@ -5658,9 +5843,12 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp vkd3d_symbol_make_resource(&resource_symbol, reg); resource_symbol.id = var_id; resource_symbol.info.resource.register_space = register_space; - resource_symbol.info.resource.register_index = register_index; + resource_symbol.info.resource.register_index = register_first; resource_symbol.info.resource.sampled_type = sampled_type; resource_symbol.info.resource.type_id = type_id; + resource_symbol.info.resource.contained_type_id = contained_type_id; + resource_symbol.info.resource.binding_base_idx = binding_base_idx; + resource_symbol.info.resource.storage_class = storage_class; resource_symbol.info.resource.resource_type_info = resource_type_info; resource_symbol.info.resource.structure_stride = structure_stride; resource_symbol.info.resource.raw = raw; @@ -7652,9 +7840,27 @@ static void vkd3d_dxbc_compiler_prepare_image(struct vkd3d_dxbc_compiler *compil if (!symbol) symbol = vkd3d_dxbc_compiler_find_resource(compiler, resource_reg); - image->id = symbol->id; + if (symbol->info.resource.binding_base_idx != ~0u) + { + uint32_t ptr_type_id, index_id; + + index_id = vkd3d_dxbc_compiler_get_resource_index(compiler, + resource_reg, symbol->info.resource.binding_base_idx); + + ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, + symbol->info.resource.storage_class, + symbol->info.resource.contained_type_id); + image->image_type_id = symbol->info.resource.contained_type_id; + + image->id = vkd3d_spirv_build_op_access_chain(builder, + ptr_type_id, symbol->id, &index_id, 1); + } + else + { + image->id = symbol->id; + image->image_type_id = symbol->info.resource.type_id; + } image->sampled_type = symbol->info.resource.sampled_type; - image->image_type_id = symbol->info.resource.type_id; image->resource_type_info = symbol->info.resource.resource_type_info; image->structure_stride = symbol->info.resource.structure_stride; image->raw = symbol->info.resource.raw; @@ -7669,8 +7875,15 @@ static void vkd3d_dxbc_compiler_prepare_image(struct vkd3d_dxbc_compiler *compil return; } - image->image_id = load ? vkd3d_spirv_build_op_load(builder, - image->image_type_id, image->id, SpvMemoryAccessMaskNone) : 0; + if (load) + { + image->image_id = vkd3d_spirv_build_op_load(builder, + image->image_type_id, image->id, SpvMemoryAccessMaskNone); + } + else + { + image->image_id = 0; + } image->image_type_id = vkd3d_dxbc_compiler_get_image_type_id(compiler, resource_reg, symbol->info.resource.register_space, symbol->info.resource.register_index, image->resource_type_info, @@ -7678,10 +7891,22 @@ static void vkd3d_dxbc_compiler_prepare_image(struct vkd3d_dxbc_compiler *compil if (sampled) { + struct vkd3d_shader_register_info register_info; assert(image->image_id); assert(sampler_reg); - sampler_var_id = vkd3d_dxbc_compiler_get_register_id(compiler, sampler_reg); + if (!vkd3d_dxbc_compiler_get_register_info(compiler, sampler_reg, ®ister_info)) + ERR("Failed to get sampler register info.\n"); + sampler_var_id = register_info.id; + if (register_info.binding_base_idx != ~0u) + { + uint32_t ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, + register_info.storage_class, vkd3d_spirv_get_op_type_sampler(builder)); + uint32_t array_idx = vkd3d_dxbc_compiler_get_resource_index(compiler, + sampler_reg, register_info.binding_base_idx); + sampler_var_id = vkd3d_spirv_build_op_access_chain(builder, ptr_type_id, + register_info.id, &array_idx, 1); + } sampler_id = vkd3d_spirv_build_op_load(builder, vkd3d_spirv_get_op_type_sampler(builder), sampler_var_id, SpvMemoryAccessMaskNone); @@ -9469,6 +9694,7 @@ void vkd3d_dxbc_compiler_destroy(struct vkd3d_dxbc_compiler *compiler) vkd3d_spirv_builder_free(&compiler->spirv_builder); rb_destroy(&compiler->symbol_table, vkd3d_symbol_free, NULL); + rb_destroy(&compiler->array_var_table, vkd3d_array_variable_free, NULL); vkd3d_free(compiler->shader_phases); vkd3d_free(compiler->spec_constants); diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c index a63fc92b..9bce22cf 100644 --- a/libs/vkd3d/device.c +++ b/libs/vkd3d/device.c @@ -1457,6 +1457,10 @@ static HRESULT vkd3d_init_device_caps(struct d3d12_device *device, vulkan_info->shader_extensions[0] = VKD3D_SHADER_SPIRV_EXTENSION_EXT_DEMOTE_TO_HELPER_INVOCATION; } + if (vulkan_info->EXT_descriptor_indexing) + vulkan_info->shader_extensions[vulkan_info->shader_extension_count++] + = VKD3D_SHADER_SPIRV_EXTENSION_EXT_DESCRIPTOR_INDEXING; + /* Disable unused Vulkan features. */ features->shaderTessellationAndGeometryPointSize = VK_FALSE; diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index b30e38e9..8dbb05b5 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -51,7 +51,7 @@ #define VKD3D_MAX_COMPATIBLE_FORMAT_COUNT 6u #define VKD3D_MAX_QUEUE_FAMILY_COUNT 3u -#define VKD3D_MAX_SHADER_EXTENSIONS 1u +#define VKD3D_MAX_SHADER_EXTENSIONS 2u #define VKD3D_MAX_SHADER_STAGES 5u #define VKD3D_MAX_VK_SYNC_OBJECTS 4u #define VKD3D_MAX_DESCRIPTOR_SETS 2u -- 2.31.1
On Thu, 3 Jun 2021 at 07:02, Conor McCarthy <cmccarthy(a)codeweavers.com> wrote:
include/vkd3d_shader.h | 6 +- libs/vkd3d-shader/spirv.c | 356 ++++++++++++++++++++++++++++++------- libs/vkd3d/device.c | 4 + libs/vkd3d/vkd3d_private.h | 2 +- 4 files changed, 300 insertions(+), 68 deletions(-)
The subject line already hints at it, but there's quite a lot going in this patch: - It changes the way resource/sampler descriptors are declared from register space + index to register space + range. - It introduces vkd3d_array_variable, and uses it to map descriptor ranges to SPIR-V variables. - It introduces "binding_base_idx" to account for differences between the descriptor range and binding start registers. - It introduces support for dynamic descriptor array indexing using SPV_EXT_descriptor_indexing. - It modifies libvkd3d to enable VKD3D_SHADER_SPIRV_EXTENSION_EXT_DESCRIPTOR_INDEXING when supported. Please split those into separate patches.
@@ -208,8 +208,9 @@ struct vkd3d_shader_descriptor_binding /** The binding index of the descriptor. */ unsigned int binding; /** - * The size of this descriptor array. Descriptor arrays are not supported in - * this version of vkd3d-shader, and therefore this value must be 1. + * The size of this descriptor array. Descriptor arrays are supported in + * this version of vkd3d-shader, but arrayed UAV counters and non-uniform + * array indexing are not. */ unsigned int count; }; So in practical terms, UAV counter (and combined resource/sampler?) descriptor arrays are not supported in this version of vkd3d-shader, and therefore this value must be 1 for those bindings.
@@ -1808,6 +1815,8 @@ static bool vkd3d_spirv_compile_module(struct vkd3d_spirv_builder *builder, vkd3d_spirv_build_op_extension(&stream, "SPV_KHR_shader_draw_parameters"); if (builder->capability_demote_to_helper_invocation) vkd3d_spirv_build_op_extension(&stream, "SPV_EXT_demote_to_helper_invocation"); + if (builder->capability_descriptor_indexing) + vkd3d_spirv_build_op_extension(&stream, "SPV_EXT_descriptor_indexing");
...but if the extension is required but not supported, we should fail shader compilation.
@@ -1926,6 +1935,7 @@ struct vkd3d_symbol_register_data unsigned int write_mask; uint32_t dcl_mask; unsigned int structure_stride; + unsigned int binding_base_idx; bool is_aggregate; /* An aggregate, i.e. a structure or an array. */ bool is_dynamically_indexed; /* If member_idx is a variable ID instead of a constant. */ }; @@ -1936,6 +1946,9 @@ struct vkd3d_symbol_resource_data unsigned int register_index; enum vkd3d_shader_component_type sampled_type; uint32_t type_id; + uint32_t contained_type_id; + unsigned int binding_base_idx; + SpvStorageClass storage_class; const struct vkd3d_spirv_resource_type *resource_type_info; unsigned int structure_stride; bool raw;
Do we need "binding_base_idx" to be part of vkd3d_symbol_register_data and vkd3d_symbol_resource_data? My first thought would be that that should be part of the vkd3d_array_variable structure, and vkd3d_symbol_resource_data should just point to that. Similar concerns probably apply to "contained_type_id" and "storage_class".
+struct vkd3d_array_variable_key +{ + uint32_t ptr_type_id; + unsigned int set; + unsigned int binding; +}; + +struct vkd3d_array_variable +{ + struct rb_entry entry; + struct vkd3d_array_variable_key key; + uint32_t id; +}; + So this is essentially storing a vkd3d_shader_descriptor_binding structure and the corresponding SPIR-V variable, right? Should this perhaps be called something like "vkd3d_symbol_descriptor_binding" and be stored in the existing "symbol_table" tree?
@@ -2464,12 +2530,14 @@ static void VKD3D_PRINTF_FUNC(3, 4) vkd3d_dxbc_compiler_error(struct vkd3d_dxbc_
static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor_binding( struct vkd3d_dxbc_compiler *compiler, const struct vkd3d_shader_register *reg, unsigned int register_space, - unsigned int reg_idx, enum vkd3d_shader_resource_type resource_type, bool is_uav_counter) + unsigned int register_first, unsigned int register_last, enum vkd3d_shader_resource_type resource_type, + bool is_uav_counter, unsigned int *binding_base_idx) { const struct vkd3d_shader_interface_info *shader_interface = &compiler->shader_interface; enum vkd3d_shader_descriptor_type descriptor_type; enum vkd3d_shader_binding_flag resource_type_flag; struct vkd3d_shader_descriptor_binding binding; + bool bounded = true; unsigned int i;
if (reg->type == VKD3DSPR_CONSTBUFFER) @@ -2491,6 +2559,12 @@ static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor resource_type_flag = resource_type == VKD3D_SHADER_RESOURCE_BUFFER ? VKD3D_SHADER_BINDING_FLAG_BUFFER : VKD3D_SHADER_BINDING_FLAG_IMAGE;
+ if (register_last == ~0u) + { + bounded = false; + register_last = register_first; + } + Setting "register_last" = "register_first" is perhaps convenient for the range checks below, but would also change the range reported in compiler messages.
@@ -2501,32 +2575,37 @@ static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor if (!vkd3d_dxbc_compiler_check_shader_visibility(compiler, current->shader_visibility)) continue;
- if (current->register_space != register_space || current->register_index != reg_idx) + if (current->register_space != register_space || current->register_index > register_first + || current->register_index + current->binding.count <= register_last) continue;
I thought UAV counter descriptor arrays were unsupported?
if (current->offset) { FIXME("Atomic counter offsets are not supported yet.\n"); vkd3d_dxbc_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INVALID_DESCRIPTOR_BINDING, - "Descriptor binding for UAV counter %u, space %u has unsupported ‘offset’ %u.", - reg_idx, register_space, current->offset); + "Descriptor binding for UAV counter %s, space %u has unsupported ‘offset’ %u.", + debug_register_range(register_first, register_last, bounded), + register_space, current->offset); }
Something similar came up for the HLSL compiler not too long ago. We shouldn't use debug utilities (specifically, vkd3d_dbg_sprintf()) for non-debug purposes.
@@ -2542,31 +2621,27 @@ static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor continue;
if (current->type != descriptor_type || current->register_space != register_space - || current->register_index != reg_idx) + || current->register_index > register_first + || current->register_index + current->binding.count <= register_last) continue;
"current->register_index + current->binding.count" can overflow.
- if (current->binding.count != 1) - { - FIXME("Descriptor arrays are not supported.\n"); - vkd3d_dxbc_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INVALID_DESCRIPTOR_BINDING, - "Descriptor binding for type %#x, space %u, register %u, " - "shader type %#x has unsupported ‘count’ %u.", - descriptor_type, register_space, reg_idx, compiler->shader_type, current->binding.count); - } - + *binding_base_idx = (current->binding.count != 1 || !bounded) ? current->register_index : ~0u; return current->binding; Is the special case of "binding_base_idx == ~-0u" really needed? It seems users of "binding_base_idx" should be able to handle the actual value just fine.
+static uint32_t vkd3d_dxbc_compiler_get_resource_index(struct vkd3d_dxbc_compiler *compiler, + const struct vkd3d_shader_register *reg, unsigned int binding_base_idx) +{ + uint32_t index_id; + + if (shader_is_sm_5_1(compiler)) + { + struct vkd3d_shader_register_index index = reg->idx[1]; + + if (index.rel_addr) + vkd3d_spirv_enable_descriptor_indexing(&compiler->spirv_builder); + + index.offset -= binding_base_idx; + index_id = vkd3d_dxbc_compiler_emit_register_addressing(compiler, &index); + } + else + { + index_id = vkd3d_dxbc_compiler_get_constant_uint(compiler, reg->idx[0].offset - binding_base_idx); + } + + return index_id; +} + We've been able to avoid version checks like this so far, and it's not immediately obvious to me why we need this one.
June 15, 2021 12:39 AM, "Henri Verbeet" <hverbeet(a)gmail.com> wrote:
Do we need "binding_base_idx" to be part of vkd3d_symbol_register_data and vkd3d_symbol_resource_data? My first thought would be that that should be part of the vkd3d_array_variable structure, and vkd3d_symbol_resource_data should just point to that. Similar concerns probably apply to "contained_type_id" and "storage_class".
Multiple symbols can map to one array variable, and each can have a different base index. However, "contained_type_id" and "storage_class" can be stored in the array variable structure.
So this is essentially storing a vkd3d_shader_descriptor_binding structure and the corresponding SPIR-V variable, right? Should this perhaps be called something like "vkd3d_symbol_descriptor_binding" and be stored in the existing "symbol_table" tree?
Yes, I can declare a new symbol type and store it in "symbol_table".
Is the special case of "binding_base_idx == ~-0u" really needed? It seems users of "binding_base_idx" should be able to handle the actual value just fine.
The special case can be eliminated by always declaring an array even for single bindings. It means a little extra SPIR-V, and an extra entry in the symbol table, but accessing an array[1] at constant index 0 should ultimately compile to the same machine code as a scalar variable.
+ if (shader_is_sm_5_1(compiler)) + { + struct vkd3d_shader_register_index index = reg->idx[1]; + + if (index.rel_addr) + vkd3d_spirv_enable_descriptor_indexing(&compiler->spirv_builder); + + index.offset -= binding_base_idx; + index_id = vkd3d_dxbc_compiler_emit_register_addressing(compiler, &index); + } + else + { + index_id = vkd3d_dxbc_compiler_get_constant_uint(compiler, reg->idx[0].offset - binding_base_idx); + } + + return index_id; +} +
We've been able to avoid version checks like this so far, and it's not immediately obvious to me why we need this one.
I can replace it with an index variable in the compiler struct for accessing reg->idx. That would be simpler and eliminate the branching.
On Tue, 15 Jun 2021 at 09:12, Conor McCarthy <cmccarthy(a)codeweavers.com> wrote:
June 15, 2021 12:39 AM, "Henri Verbeet" <hverbeet(a)gmail.com> wrote:
Do we need "binding_base_idx" to be part of vkd3d_symbol_register_data and vkd3d_symbol_resource_data? My first thought would be that that should be part of the vkd3d_array_variable structure, and vkd3d_symbol_resource_data should just point to that. Similar concerns probably apply to "contained_type_id" and "storage_class".
Multiple symbols can map to one array variable, and each can have a different base index.
Is that really true? E.g., suppose you have a vkd3d_shader_descriptor_binding like this: {.register_index = 2, .binding.count = 8, ...} and then two vkd3d_shader_descriptor_range's like this: {.first = 3, .last = 5, ...} {.first = 6, .last = 8, ...} The "binding_base_idx" for those would be 2 in either case. (And at least in the current version of the patch, it's assigned "current->register_index" in vkd3d_dxbc_compiler_get_descriptor_binding() if it's not ~0u)
Is the special case of "binding_base_idx == ~-0u" really needed? It seems users of "binding_base_idx" should be able to handle the actual value just fine.
The special case can be eliminated by always declaring an array even for single bindings. It means a little extra SPIR-V, and an extra entry in the symbol table, but accessing an array[1] at constant index 0 should ultimately compile to the same machine code as a scalar variable.
That's one possibility, but it would also be possible to just compare the "count" field from the vkd3d_shader_descriptor_binding structure against 1, instead of comparing "binding_base_idx" against ~0u. (I.e., when storing a vkd3d_shader_descriptor_binding structure in the vkd3d_array_variable/vkd3d_symbol_descriptor_binding structure.)
+ if (shader_is_sm_5_1(compiler)) + { + struct vkd3d_shader_register_index index = reg->idx[1]; + + if (index.rel_addr) + vkd3d_spirv_enable_descriptor_indexing(&compiler->spirv_builder); + + index.offset -= binding_base_idx; + index_id = vkd3d_dxbc_compiler_emit_register_addressing(compiler, &index); + } + else + { + index_id = vkd3d_dxbc_compiler_get_constant_uint(compiler, reg->idx[0].offset - binding_base_idx); + } + + return index_id; +} +
We've been able to avoid version checks like this so far, and it's not immediately obvious to me why we need this one.
I can replace it with an index variable in the compiler struct for accessing reg->idx. That would be simpler and eliminate the branching.
That should work. Alternatively though, it shouldn't be hard to fix things up to match shader model 5.1 in the frontend, similar to what we do with shader_sm4_map_resource_idx(), which would allow you to just always use the shader model 5.1 path here in the backend.
June 15, 2021 8:36 PM, "Henri Verbeet" <hverbeet(a)gmail.com> wrote:
Multiple symbols can map to one array variable, and each can have a different base index.
Is that really true? E.g., suppose you have a vkd3d_shader_descriptor_binding like this:
You're right, I had it the wrong way around. The base index of the binding can be stored along with the other data in the symbol table.
That's one possibility, but it would also be possible to just compare the "count" field from the vkd3d_shader_descriptor_binding structure against 1, instead of comparing "binding_base_idx" against ~0u. (I.e., when storing a vkd3d_shader_descriptor_binding structure in the vkd3d_array_variable/vkd3d_symbol_descriptor_binding structure.)
The one potential problem I see is it's possible to declare in a shader an unbounded resource array which maps to a range containing only one descriptor, and then dynamically index it with a variable that's always zero. Unlikely to occur, but the dynamic indexing would fail because with binding.count == 1, a scalar variable is declared.
On Tue, 15 Jun 2021 at 14:55, Conor McCarthy <cmccarthy(a)codeweavers.com> wrote:
June 15, 2021 8:36 PM, "Henri Verbeet" <hverbeet(a)gmail.com> wrote:
That's one possibility, but it would also be possible to just compare the "count" field from the vkd3d_shader_descriptor_binding structure against 1, instead of comparing "binding_base_idx" against ~0u. (I.e., when storing a vkd3d_shader_descriptor_binding structure in the vkd3d_array_variable/vkd3d_symbol_descriptor_binding structure.)
The one potential problem I see is it's possible to declare in a shader an unbounded resource array which maps to a range containing only one descriptor, and then dynamically index it with a variable that's always zero. Unlikely to occur, but the dynamic indexing would fail because with binding.count == 1, a scalar variable is declared.
That's a good point, although also somewhat orthogonal; in the current version of the patch vkd3d_dxbc_compiler_get_resource_variable() uses the "array_variable" parameter to decide whether to create a scalar or array variable. Its callers all use some variant of "binding_base_idx != ~0u || register_last == ~0u" to set that, which could be trivially replaced with something like "binding.count > 1 || register_last == ~0u", because vkd3d_dxbc_compiler_get_resource_variable() also takes "binding" as an argument. It could be argued that callers should pass a vkd3d_shader_descriptor_range structure to vkd3d_dxbc_compiler_get_resource_variable() instead of the "array_variable" parameter, in which case vkd3d_dxbc_compiler_get_resource_variable() would be able to resolve whether or not to create an array variable for itself, using something like "binding->count > 1 || range->last == ~0u". There's certainly something to be said for the simplicity of always creating array variables though.
June 15, 2021 12:39 AM, "Henri Verbeet" <hverbeet(a)gmail.com> wrote:
- if (current->register_space != register_space || current->register_index != reg_idx) + if (current->register_space != register_space || current->register_index > register_first + || current->register_index + current->binding.count <= register_last) continue;
I thought UAV counter descriptor arrays were unsupported?
This ensures the correct error message is returned, i.e. "Descriptor arrays are not supported" instead of "Could not find binding." It's possible to separately check register_last == register_first, but this would require yet another error message. But I should probably add register_last to the existing message.
June 15, 2021 12:39 AM, "Henri Verbeet" <hverbeet(a)gmail.com> wrote:
+ || current->register_index > register_first + || current->register_index + current->binding.count <= register_last) continue;
"current->register_index + current->binding.count" can overflow.
The result would be it doesn't match any binding. We could emit an error, but I think overflow should be checked in d3d12_root_signature_append_vk_binding() in state.c. Other uses of vkd3d-shader could send this type of invalid binding, but I think the resulting "Could not find binding" is accurate enough.
On Wed, 16 Jun 2021 at 08:14, Conor McCarthy <cmccarthy(a)codeweavers.com> wrote:
June 15, 2021 12:39 AM, "Henri Verbeet" <hverbeet(a)gmail.com> wrote:
+ || current->register_index > register_first + || current->register_index + current->binding.count <= register_last) continue;
"current->register_index + current->binding.count" can overflow.
The result would be it doesn't match any binding. We could emit an error, but I think overflow should be checked in d3d12_root_signature_append_vk_binding() in state.c. Other uses of vkd3d-shader could send this type of invalid binding, but I think the resulting "Could not find binding" is accurate enough.
Well, it would be a nominally valid binding. Setting "binding.count = ~0u" in particular would trigger this, but even if the caller set a binding count that's larger than the Vulkan descriptor limits, that would be no reason for vkd3d-shader to reject the binding. Beyond that, I think it's simply good practice to write range checks like this in a way that avoids the overflow. I.e., "current->binding.count <= register_last - current->register_index".
On Thu, 3 Jun 2021 at 07:02, Conor McCarthy <cmccarthy(a)codeweavers.com> wrote:
+ todo + context.pipeline_state = create_compute_pipeline_state(device, context.root_signature, + shader_bytecode(cs_code, sizeof(cs_code))); + if (!context.pipeline_state) + { + skip("Shader descriptor arrays are not supported.\n"); + goto done; + } + Pipeline creation appears to succeed here:
d3d12:33980: Todo succeeded: Failed to create compute pipeline state, hr 0. d3d12:33999: Test failed: Got 0x00000000, expected 0x00000004 at (0, 0, 0). d3d12:33999: Test failed: Got 0x00000009, expected 0x00000040 at (0, 0, 0). d3d12:33999: Test failed: Got 0x00000000, expected 0x00000031 at (0, 0, 0). d3d12:33999: Test failed: Got 0x00000000, expected 0x00000009 at (0, 0, 0). d3d12:33999: Test failed: Got 0x00000000, expected 0x00000010 at (0, 0, 0).
participants (2)
-
Conor McCarthy -
Henri Verbeet