[PATCH vkd3d 1/5] vkd3d-shader: Find bindings for descriptor arrays.
Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/spirv.c | 51 +++++++++++++++++++++++++++------------ tests/d3d12.c | 1 + 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index bd231124..61f0d903 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -2460,15 +2460,28 @@ static void VKD3D_PRINTF_FUNC(3, 4) vkd3d_dxbc_compiler_error(struct vkd3d_dxbc_ compiler->failed = true; } +#define RANGE_STRING_SIZE 24 + +static void register_range_format_string(char range_str[RANGE_STRING_SIZE], + const struct vkd3d_shader_register_range *range) +{ + if (range->last != ~0u) + sprintf(range_str, "[%u:%u]", range->first, range->last); + else + sprintf(range_str, "[%u:*]", range->first); +} + static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor_binding( struct vkd3d_dxbc_compiler *compiler, const struct vkd3d_shader_register *reg, const struct vkd3d_shader_register_range *range, enum vkd3d_shader_resource_type resource_type, - bool is_uav_counter) + bool is_uav_counter, unsigned int *binding_base_idx) { const struct vkd3d_shader_interface_info *shader_interface = &compiler->shader_interface; + unsigned int register_last = (range->last == ~0u) ? range->first : range->last; enum vkd3d_shader_descriptor_type descriptor_type; enum vkd3d_shader_binding_flag resource_type_flag; struct vkd3d_shader_descriptor_binding binding; + char range_str[RANGE_STRING_SIZE]; unsigned int i; if (reg->type == VKD3DSPR_CONSTBUFFER) @@ -2519,6 +2532,7 @@ static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor range->first, range->space, current->binding.count); } + *binding_base_idx = current->register_index; return current->binding; } if (shader_interface->uav_counter_count) @@ -2541,31 +2555,26 @@ static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor continue; if (current->type != descriptor_type || current->register_space != range->space - || current->register_index != range->first) + || current->register_index > range->first + || current->binding.count <= register_last - current->register_index) 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, range->space, range->first, compiler->shader_type, current->binding.count); - } - + *binding_base_idx = current->register_index; 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, range->space, range->first, compiler->shader_type); + register_range_format_string(range_str, range); + FIXME("Could not find descriptor binding for type %#x, space %u, registers %s, shader type %#x.\n", + descriptor_type, range->space, range_str, 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, range->space, range->first, compiler->shader_type); + "Could not find descriptor binding for type %#x, space %u, registers %s, shader type %#x.", + descriptor_type, range->space, range_str, compiler->shader_type); } } done: + *binding_base_idx = range->first; binding.set = 0; binding.count = 1; binding.binding = compiler->binding_idx++; @@ -2586,8 +2595,18 @@ static void vkd3d_dxbc_compiler_emit_descriptor_binding_for_reg(struct vkd3d_dxb enum vkd3d_shader_resource_type resource_type, bool is_uav_counter) { struct vkd3d_shader_descriptor_binding binding; + unsigned int binding_base_idx; - binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, range, resource_type, is_uav_counter); + binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, range, + resource_type, is_uav_counter, &binding_base_idx); + if (binding_base_idx != range->first) + { + 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 base index %u.", + reg->type, range->space, range->first, compiler->shader_type, binding_base_idx); + } vkd3d_dxbc_compiler_emit_descriptor_binding(compiler, variable_id, &binding); } diff --git a/tests/d3d12.c b/tests/d3d12.c index 4397c063..424a57cf 100644 --- a/tests/d3d12.c +++ b/tests/d3d12.c @@ -33996,6 +33996,7 @@ static void test_resource_arrays(void) 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) -- 2.31.1
Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/spirv.c | 145 +++++++++++++++++++++-- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 2 files changed, 136 insertions(+), 10 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 61f0d903..25936849 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 @@ -1918,6 +1919,13 @@ struct vkd3d_symbol_combined_sampler unsigned int sampler_index; }; +struct vkd3d_symbol_descriptor_array +{ + uint32_t ptr_type_id; + unsigned int set; + unsigned int binding; +}; + struct vkd3d_symbol_register_data { SpvStorageClass storage_class; @@ -1946,6 +1954,13 @@ struct vkd3d_symbol_sampler_data struct vkd3d_shader_register_range range; }; +struct vkd3d_symbol_descriptor_array_data +{ + SpvStorageClass storage_class; + uint32_t contained_type_id; + unsigned int binding_base_idx; +}; + struct vkd3d_symbol { struct rb_entry entry; @@ -1956,6 +1971,7 @@ struct vkd3d_symbol VKD3D_SYMBOL_RESOURCE, VKD3D_SYMBOL_SAMPLER, VKD3D_SYMBOL_COMBINED_SAMPLER, + VKD3D_SYMBOL_DESCRIPTOR_ARRAY, } type; union @@ -1964,14 +1980,19 @@ struct vkd3d_symbol struct vkd3d_symbol_resource resource; struct vkd3d_symbol_sampler sampler; struct vkd3d_symbol_combined_sampler combined_sampler; + struct vkd3d_symbol_descriptor_array descriptor_array; } key; uint32_t id; + /* The array declaration which this symbol maps to, or NULL. */ + const struct vkd3d_symbol *descriptor_array; + union { struct vkd3d_symbol_register_data reg; struct vkd3d_symbol_resource_data resource; struct vkd3d_symbol_sampler_data sampler; + struct vkd3d_symbol_descriptor_array_data descriptor_array; } info; }; @@ -2009,6 +2030,7 @@ static void vkd3d_symbol_set_register_info(struct vkd3d_symbol *symbol, enum vkd3d_shader_component_type component_type, DWORD write_mask) { symbol->id = val_id; + symbol->descriptor_array = NULL; symbol->info.reg.storage_class = storage_class; symbol->info.reg.member_idx = 0; symbol->info.reg.component_type = component_type; @@ -2610,7 +2632,7 @@ static void vkd3d_dxbc_compiler_emit_descriptor_binding_for_reg(struct vkd3d_dxb vkd3d_dxbc_compiler_emit_descriptor_binding(compiler, variable_id, &binding); } -static void vkd3d_dxbc_compiler_put_symbol(struct vkd3d_dxbc_compiler *compiler, +static const struct vkd3d_symbol *vkd3d_dxbc_compiler_put_symbol(struct vkd3d_dxbc_compiler *compiler, const struct vkd3d_symbol *symbol) { struct vkd3d_symbol *s; @@ -2620,7 +2642,9 @@ static void vkd3d_dxbc_compiler_put_symbol(struct vkd3d_dxbc_compiler *compiler, { ERR("Failed to insert symbol entry (%s).\n", debug_vkd3d_symbol(symbol)); vkd3d_free(s); + return NULL; } + return s; } static uint32_t vkd3d_dxbc_compiler_get_constant(struct vkd3d_dxbc_compiler *compiler, @@ -3011,6 +3035,7 @@ static uint32_t vkd3d_dxbc_compiler_emit_register_addressing(struct vkd3d_dxbc_c struct vkd3d_shader_register_info { uint32_t id; + const struct vkd3d_symbol *descriptor_array; SpvStorageClass storage_class; enum vkd3d_shader_component_type component_type; unsigned int write_mask; @@ -3033,6 +3058,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->descriptor_array = NULL; register_info->member_idx = 0; register_info->component_type = VKD3D_SHADER_COMPONENT_FLOAT; register_info->write_mask = VKD3DSP_WRITEMASK_ALL; @@ -3052,6 +3078,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->descriptor_array = symbol->descriptor_array; register_info->storage_class = symbol->info.reg.storage_class; register_info->member_idx = symbol->info.reg.member_idx; register_info->component_type = symbol->info.reg.component_type; @@ -3078,17 +3105,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, + enum vkd3d_shader_resource_type resource_type) +{ + struct vkd3d_shader_register_index index = reg->idx[1]; + uint32_t index_id; + + if (index.rel_addr) + { + FIXME("Descriptor dynamic indexing is not supported.\n"); + vkd3d_dxbc_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_DESCRIPTOR_IDX_UNSUPPORTED, + "Cannot dynamically index a descriptor array of type %#x, id %u. " + "Dynamic indexing is not supported.", reg->type, reg->idx[0].offset); + } + + index.offset -= binding_base_idx; + index_id = vkd3d_dxbc_compiler_emit_register_addressing(compiler, &index); + + 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->descriptor_array) + indexes[index_count++] = vkd3d_dxbc_compiler_get_resource_index(compiler, reg, + register_info->descriptor_array->info.descriptor_array.binding_base_idx, + VKD3D_SHADER_RESOURCE_BUFFER); 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]); } @@ -5282,16 +5334,91 @@ static void vkd3d_dxbc_compiler_emit_push_constant_buffers(struct vkd3d_dxbc_com } } +static uint32_t vkd3d_dxbc_compiler_get_resource_variable(struct vkd3d_dxbc_compiler *compiler, + const struct vkd3d_symbol_descriptor_array_data *array_data, uint32_t ptr_type_id, + const struct vkd3d_shader_register *reg, const struct vkd3d_shader_descriptor_binding *binding, + const struct vkd3d_symbol **array_symbol) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + + if (array_data->contained_type_id) + { + struct vkd3d_symbol *array_symbol_entry; + struct vkd3d_symbol symbol; + 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. */ + symbol.type = VKD3D_SYMBOL_DESCRIPTOR_ARRAY; + memset(&symbol.key, 0, sizeof(symbol.key)); + symbol.key.descriptor_array.ptr_type_id = ptr_type_id; + symbol.key.descriptor_array.set = binding->set; + symbol.key.descriptor_array.binding = binding->binding; + if ((entry = rb_get(&compiler->symbol_table, &symbol))) + { + array_symbol_entry = RB_ENTRY_VALUE(entry, struct vkd3d_symbol, entry); + *array_symbol = array_symbol_entry; + return array_symbol_entry->id; + } + + symbol.id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream, + ptr_type_id, array_data->storage_class, 0); + symbol.descriptor_array = NULL; + vkd3d_dxbc_compiler_emit_descriptor_binding(compiler, symbol.id, binding); + vkd3d_dxbc_compiler_emit_register_debug_name(builder, symbol.id, reg); + + symbol.info.descriptor_array = *array_data; + *array_symbol = vkd3d_dxbc_compiler_put_symbol(compiler, &symbol); + + return symbol.id; + } + else + { + uint32_t var_id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream, + ptr_type_id, array_data->storage_class, 0); + vkd3d_dxbc_compiler_emit_descriptor_binding(compiler, var_id, binding); + vkd3d_dxbc_compiler_emit_register_debug_name(builder, var_id, reg); + *array_symbol = NULL; + 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, unsigned int binding_base_idx, + bool array_variable, const struct vkd3d_symbol **array_symbol) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + struct vkd3d_symbol_descriptor_array_data array_data; + uint32_t ptr_type_id; + + array_data.storage_class = storage_class; + array_data.contained_type_id = 0; + array_data.binding_base_idx = binding_base_idx; + if (array_variable) + { + array_data.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, array_data.storage_class, type_id); + + return vkd3d_dxbc_compiler_get_resource_variable(compiler, &array_data, ptr_type_id, reg, binding, array_symbol); +} + 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; const struct vkd3d_shader_constant_buffer *cb = &instruction->declaration.cb; struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + uint32_t vec4_id, array_type_id, length_id, struct_id, var_id; 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; + const struct vkd3d_symbol *array_symbol; struct vkd3d_symbol reg_symbol; + unsigned int binding_base_idx; assert(!(instruction->flags & ~VKD3DSI_INDEXED_DYNAMIC)); @@ -5321,18 +5448,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); - - vkd3d_dxbc_compiler_emit_descriptor_binding_for_reg(compiler, - var_id, reg, &cb->range, VKD3D_SHADER_RESOURCE_BUFFER, false); + binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, &cb->range, + VKD3D_SHADER_RESOURCE_BUFFER, false, &binding_base_idx); - 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, binding.count != 1 || cb->range.last == ~0u, &array_symbol); 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.descriptor_array = array_symbol; vkd3d_dxbc_compiler_put_symbol(compiler, ®_symbol); } diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index c5d1372a..f62d2d48 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -74,6 +74,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_SPV_DESCRIPTOR_BINDING_NOT_FOUND = 2000, VKD3D_SHADER_ERROR_SPV_INVALID_REGISTER_TYPE = 2001, VKD3D_SHADER_ERROR_SPV_INVALID_DESCRIPTOR_BINDING = 2002, + VKD3D_SHADER_ERROR_SPV_DESCRIPTOR_IDX_UNSUPPORTED = 2003, VKD3D_SHADER_ERROR_RS_OUT_OF_MEMORY = 3000, VKD3D_SHADER_ERROR_RS_INVALID_VERSION = 3001, -- 2.31.1
On Fri, 9 Jul 2021 at 07:47, Conor McCarthy <cmccarthy(a)codeweavers.com> wrote:
+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, + enum vkd3d_shader_resource_type resource_type) +{ + struct vkd3d_shader_register_index index = reg->idx[1]; + uint32_t index_id; + + if (index.rel_addr) + { + FIXME("Descriptor dynamic indexing is not supported.\n"); + vkd3d_dxbc_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_DESCRIPTOR_IDX_UNSUPPORTED, + "Cannot dynamically index a descriptor array of type %#x, id %u. " + "Dynamic indexing is not supported.", reg->type, reg->idx[0].offset); + } + + index.offset -= binding_base_idx; + index_id = vkd3d_dxbc_compiler_emit_register_addressing(compiler, &index); + + return index_id; +} + "resource_type" is unused here. Would it make sense to name this vkd3d_dxbc_compiler_get_descriptor_index() instead?
+static uint32_t vkd3d_dxbc_compiler_get_resource_variable(struct vkd3d_dxbc_compiler *compiler, + const struct vkd3d_symbol_descriptor_array_data *array_data, uint32_t ptr_type_id, + const struct vkd3d_shader_register *reg, const struct vkd3d_shader_descriptor_binding *binding, + const struct vkd3d_symbol **array_symbol) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + + if (array_data->contained_type_id) + { + struct vkd3d_symbol *array_symbol_entry; + struct vkd3d_symbol symbol; + 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. */ + symbol.type = VKD3D_SYMBOL_DESCRIPTOR_ARRAY; + memset(&symbol.key, 0, sizeof(symbol.key)); + symbol.key.descriptor_array.ptr_type_id = ptr_type_id; + symbol.key.descriptor_array.set = binding->set; + symbol.key.descriptor_array.binding = binding->binding; + if ((entry = rb_get(&compiler->symbol_table, &symbol))) + { + array_symbol_entry = RB_ENTRY_VALUE(entry, struct vkd3d_symbol, entry); + *array_symbol = array_symbol_entry; + return array_symbol_entry->id; + } + + symbol.id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream, + ptr_type_id, array_data->storage_class, 0); + symbol.descriptor_array = NULL; + vkd3d_dxbc_compiler_emit_descriptor_binding(compiler, symbol.id, binding); + vkd3d_dxbc_compiler_emit_register_debug_name(builder, symbol.id, reg); + + symbol.info.descriptor_array = *array_data; + *array_symbol = vkd3d_dxbc_compiler_put_symbol(compiler, &symbol); + + return symbol.id; + } + else + { + uint32_t var_id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream, + ptr_type_id, array_data->storage_class, 0); + vkd3d_dxbc_compiler_emit_descriptor_binding(compiler, var_id, binding); + vkd3d_dxbc_compiler_emit_register_debug_name(builder, var_id, reg); + *array_symbol = NULL; + return var_id; + } +} + It seems nicer to handle the non-arrayed case first, and then save an indentation level on the arrayed case. I.e.:
static uint32_t vkd3d_dxbc_compiler_get_resource_variable(...) { ... if (!array_data->contained_type_id) { ... return var_id; } ... return symbol.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, unsigned int binding_base_idx, + bool array_variable, const struct vkd3d_symbol **array_symbol) +{ + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + struct vkd3d_symbol_descriptor_array_data array_data; + uint32_t ptr_type_id; + + array_data.storage_class = storage_class; + array_data.contained_type_id = 0; + array_data.binding_base_idx = binding_base_idx; + if (array_variable) + { + array_data.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, array_data.storage_class, type_id); + + return vkd3d_dxbc_compiler_get_resource_variable(compiler, &array_data, ptr_type_id, reg, binding, array_symbol); +} + Would it make sense to name this vkd3d_dxbc_compiler_build_descriptor_variable() instead? I think we can just merge vkd3d_dxbc_compiler_get_resource_variable() into this function, which would then allow for some simplifications of the combined function.
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; const struct vkd3d_shader_constant_buffer *cb = &instruction->declaration.cb; struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + uint32_t vec4_id, array_type_id, length_id, struct_id, var_id; 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; + const struct vkd3d_symbol *array_symbol; struct vkd3d_symbol reg_symbol; + unsigned int binding_base_idx;
assert(!(instruction->flags & ~VKD3DSI_INDEXED_DYNAMIC));
@@ -5321,18 +5448,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); - - vkd3d_dxbc_compiler_emit_descriptor_binding_for_reg(compiler, - var_id, reg, &cb->range, VKD3D_SHADER_RESOURCE_BUFFER, false); + binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, &cb->range, + VKD3D_SHADER_RESOURCE_BUFFER, false, &binding_base_idx);
- 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, binding.count != 1 || cb->range.last == ~0u, &array_symbol);
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.descriptor_array = array_symbol; vkd3d_dxbc_compiler_put_symbol(compiler, ®_symbol); }
Somewhat similarly, if we passed a vkd3d_shader_register_range structure and resource type to vkd3d_dxbc_compiler_build_resource_variable(), we could move the vkd3d_dxbc_compiler_get_descriptor_binding() call into vkd3d_dxbc_compiler_build_resource_variable(), and then drop the "binding", "binding_base_idx", and "array_variable" parameters.
Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/spirv.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 25936849..945ccfef 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -5501,8 +5501,11 @@ 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; + const struct vkd3d_symbol *array_symbol; struct vkd3d_symbol reg_symbol; + unsigned int binding_base_idx; + uint32_t type_id, var_id; vkd3d_symbol_make_sampler(®_symbol, reg); reg_symbol.info.sampler.range = sampler->range; @@ -5511,19 +5514,17 @@ static void vkd3d_dxbc_compiler_emit_dcl_sampler(struct vkd3d_dxbc_compiler *com 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); + binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, &sampler->range, + VKD3D_SHADER_RESOURCE_NONE, false, &binding_base_idx); - vkd3d_dxbc_compiler_emit_descriptor_binding_for_reg(compiler, var_id, reg, - &sampler->range, VKD3D_SHADER_RESOURCE_NONE, false); - - 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, binding.count != 1 || sampler->range.last == ~0u, &array_symbol); 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.descriptor_array = array_symbol; vkd3d_dxbc_compiler_put_symbol(compiler, ®_symbol); } @@ -7821,10 +7822,24 @@ 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.descriptor_array) + { + const struct vkd3d_symbol_descriptor_array_data *array_data + = ®ister_info.descriptor_array->info.descriptor_array; + uint32_t ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, + register_info.storage_class, array_data->contained_type_id); + uint32_t array_idx = vkd3d_dxbc_compiler_get_resource_index(compiler, + sampler_reg, array_data->binding_base_idx, VKD3D_SHADER_RESOURCE_NONE); + 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); -- 2.31.1
Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- include/vkd3d_shader.h | 3 ++- libs/vkd3d-shader/spirv.c | 4 +++- libs/vkd3d-shader/vkd3d_shader_main.c | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 03225d37..77758a2b 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -1137,7 +1137,8 @@ struct vkd3d_shader_descriptor_info * \ref vkd3d_shader_descriptor_info_flag. */ unsigned int flags; - /** Size of this descriptor array, or 1 if a single descriptor. */ + /** Size of this descriptor array, or 1 if a single descriptor. + * For an unbounded array this value is ~0u. */ unsigned int count; }; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 945ccfef..31c9eb1e 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -5570,13 +5570,15 @@ static const struct vkd3d_shader_descriptor_info *vkd3d_dxbc_compiler_get_descri const struct vkd3d_shader_register_range *range) { const struct vkd3d_shader_scan_descriptor_info *descriptor_info = compiler->scan_descriptor_info; + unsigned int register_last = (range->last == ~0u) ? range->first : range->last; const struct vkd3d_shader_descriptor_info *d; unsigned int i; for (i = 0; i < descriptor_info->descriptor_count; ++i) { d = &descriptor_info->descriptors[i]; - if (d->type == type && d->register_space == range->space && d->register_index == range->first) + if (d->type == type && d->register_space == range->space && d->register_index <= range->first + && (d->count == ~0u || d->count > register_last - d->register_index)) return d; } diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 54654f3f..1a566a7b 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -601,7 +601,7 @@ static bool vkd3d_shader_scan_add_descriptor(struct vkd3d_shader_scan_context *c d->resource_type = resource_type; d->resource_data_type = resource_data_type; d->flags = flags; - d->count = 1; + d->count = (range->last == ~0u) ? ~0u : range->last - range->first + 1; ++info->descriptor_count; return true; -- 2.31.1
Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- include/vkd3d_shader.h | 6 ++-- libs/vkd3d-shader/spirv.c | 64 +++++++++++++++++++++++++++++++++------ 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 77758a2b..4593fcec 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -208,8 +208,10 @@ 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 for all descriptor types except UAV + * counters and combined resource/sampler descriptors. For such bindings + * this value must be 1. Dynamic array indexing is not supported. */ unsigned int count; }; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 31c9eb1e..ade9916f 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -5703,8 +5703,13 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp SpvStorageClass storage_class = SpvStorageClassUniformConstant; const struct vkd3d_shader_register *reg = &resource->reg.reg; const struct vkd3d_spirv_resource_type *resource_type_info; + struct vkd3d_symbol_descriptor_array_data array_data; + struct vkd3d_shader_descriptor_binding binding; enum vkd3d_shader_component_type sampled_type; + const struct vkd3d_symbol *array_symbol; 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; @@ -5724,6 +5729,9 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp return; } + binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, &resource->range, + 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; @@ -5744,16 +5752,22 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp { type_id = vkd3d_dxbc_compiler_get_image_type_id(compiler, reg, &resource->range, resource_type_info, sampled_type, structure_stride || raw, 0); + + if (binding.count != 1 || resource->range.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, - &resource->range, resource_type, false); - - vkd3d_dxbc_compiler_emit_register_debug_name(builder, var_id, reg); + array_data.contained_type_id = contained_type_id; + array_data.binding_base_idx = binding_base_idx; + array_data.storage_class = storage_class; + var_id = vkd3d_dxbc_compiler_get_resource_variable(compiler, &array_data, ptr_type_id, reg, &binding, + &array_symbol); if (is_uav) { @@ -5791,6 +5805,11 @@ 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: UAV counter arrays. */ + 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); @@ -5804,6 +5823,7 @@ 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.descriptor_array = array_symbol; resource_symbol.info.resource.range = resource->range; resource_symbol.info.resource.sampled_type = sampled_type; resource_symbol.info.resource.type_id = type_id; @@ -7798,9 +7818,26 @@ 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->descriptor_array) + { + const struct vkd3d_symbol_descriptor_array_data *array_data = &symbol->descriptor_array->info.descriptor_array; + uint32_t ptr_type_id, index_id; + + index_id = vkd3d_dxbc_compiler_get_resource_index(compiler, resource_reg, + array_data->binding_base_idx, symbol->info.resource.resource_type_info->resource_type); + + ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, array_data->storage_class, + array_data->contained_type_id); + image->image_type_id = array_data->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; @@ -7815,8 +7852,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.range, image->resource_type_info, -- 2.31.1
On Fri, 9 Jul 2021 at 07:48, Conor McCarthy <cmccarthy(a)codeweavers.com> wrote:
@@ -5724,6 +5729,9 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp return; }
+ binding = vkd3d_dxbc_compiler_get_descriptor_binding(compiler, reg, &resource->range, + 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; @@ -5744,16 +5752,22 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp { type_id = vkd3d_dxbc_compiler_get_image_type_id(compiler, reg, &resource->range, resource_type_info, sampled_type, structure_stride || raw, 0); + + if (binding.count != 1 || resource->range.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)); + } }
I think we would need this for the "compiler->ssbo_uavs" case as well.
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, - &resource->range, resource_type, false); - - vkd3d_dxbc_compiler_emit_register_debug_name(builder, var_id, reg); + array_data.contained_type_id = contained_type_id; + array_data.binding_base_idx = binding_base_idx; + array_data.storage_class = storage_class; + var_id = vkd3d_dxbc_compiler_get_resource_variable(compiler, &array_data, ptr_type_id, reg, &binding, + &array_symbol);
...but if we used vkd3d_dxbc_compiler_build_resource_variable() here, it would simply handle it for us.
if (is_uav) { @@ -5791,6 +5805,11 @@ 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: UAV counter arrays. */ + ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, storage_class, contained_type_id); + }
...and this change would more or less go away.
@@ -7815,8 +7852,15 @@ static void vkd3d_dxbc_compiler_prepare_image(struct vkd3d_dxbc_compiler *compil - 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; + }
This seems like an unrelated formatting change.
On Fri, 9 Jul 2021 at 07:47, Conor McCarthy <cmccarthy(a)codeweavers.com> wrote:
+#define RANGE_STRING_SIZE 24 + +static void register_range_format_string(char range_str[RANGE_STRING_SIZE], + const struct vkd3d_shader_register_range *range) +{ + if (range->last != ~0u) + sprintf(range_str, "[%u:%u]", range->first, range->last); + else + sprintf(range_str, "[%u:*]", range->first); +} + I'd prefer building that on top of the string buffer cache.
July 14, 2021 9:20 PM, "Henri Verbeet" <hverbeet(a)gmail.com> wrote:
+#define RANGE_STRING_SIZE 24 + +static void register_range_format_string(char range_str[RANGE_STRING_SIZE], + const struct vkd3d_shader_register_range *range) +{ + if (range->last != ~0u) + sprintf(range_str, "[%u:%u]", range->first, range->last); + else + sprintf(range_str, "[%u:*]", range->first); +} +
I'd prefer building that on top of the string buffer cache.
Where is this located?
On 7/14/21 10:42 PM, Conor McCarthy wrote:
July 14, 2021 9:20 PM, "Henri Verbeet" <hverbeet(a)gmail.com> wrote:
+#define RANGE_STRING_SIZE 24 + +static void register_range_format_string(char range_str[RANGE_STRING_SIZE], + const struct vkd3d_shader_register_range *range) +{ + if (range->last != ~0u) + sprintf(range_str, "[%u:%u]", range->first, range->last); + else + sprintf(range_str, "[%u:*]", range->first); +} +
I'd prefer building that on top of the string buffer cache.
Where is this located?
struct vkd3d_string_buffer_cache and the vkd3d_string_buffer_*() helpers. It's currently only used in the HLSL compiler (which has its own wrappers for it, because of memory allocation failure tracking), but should probably be used more generally.
participants (3)
-
Conor McCarthy -
Henri Verbeet -
Zebediah Figura (she/her)