From 343c7942e150bbc6b78ed17c0d0b5041521131b0 Mon Sep 17 00:00:00 2001 From: "Anna (navi) Figueiredo Gomes" Date: Mon, 19 Aug 2024 19:48:10 +0200 Subject: [PATCH] vkd3d-shader/spirv: Avoid decorating variables multiple times with NonReadable. The existing code reuses the same SPIR-V variable for all descriptors mapped to the same Vulkan binding, and applies the NonReadable decoration based on the VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_READ only. This potentially causes the decoration to be applied twice, should two non-read descriptors be mapped to the same variable, which isn't allowed in SPIR-V, and the validator complains. --- libs/vkd3d-shader/spirv.c | 64 ++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index c90fe1fa..aa944d76 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -6076,6 +6076,26 @@ static void spirv_compiler_emit_push_constant_buffers(struct spirv_compiler *com } } +static const struct vkd3d_shader_descriptor_info1 *spirv_compiler_get_descriptor_info( + struct spirv_compiler *compiler, enum vkd3d_shader_descriptor_type type, + const struct vkd3d_shader_register_range *range) +{ + const struct vkd3d_shader_scan_descriptor_info1 *descriptor_info = compiler->scan_descriptor_info; + unsigned int register_last = (range->last == ~0u) ? range->first : range->last; + const struct vkd3d_shader_descriptor_info1 *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 + && (d->count == ~0u || d->count > register_last - d->register_index)) + return d; + } + + return NULL; +} + struct vkd3d_descriptor_variable_info { const struct vkd3d_symbol *array_symbol; @@ -6085,13 +6105,15 @@ struct vkd3d_descriptor_variable_info static uint32_t spirv_compiler_build_descriptor_variable(struct spirv_compiler *compiler, SpvStorageClass storage_class, uint32_t type_id, const struct vkd3d_shader_register *reg, const struct vkd3d_shader_register_range *range, enum vkd3d_shader_resource_type resource_type, - bool is_uav_counter, struct vkd3d_descriptor_variable_info *var_info) + bool is_uav, bool is_uav_counter, struct vkd3d_descriptor_variable_info *var_info) { struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; struct vkd3d_descriptor_binding_address binding_address; struct vkd3d_shader_descriptor_binding binding; + const struct vkd3d_shader_descriptor_info1 *d; uint32_t array_type_id, ptr_type_id, var_id; struct vkd3d_symbol symbol; + bool write_only = false; struct rb_entry *entry; binding = spirv_compiler_get_descriptor_binding(compiler, reg, range, @@ -6116,6 +6138,12 @@ static uint32_t spirv_compiler_build_descriptor_variable(struct spirv_compiler * array_type_id = vkd3d_spirv_get_op_type_runtime_array(builder, type_id); ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, storage_class, array_type_id); + if (is_uav) + { + d = spirv_compiler_get_descriptor_info(compiler, VKD3D_SHADER_DESCRIPTOR_TYPE_UAV, range); + write_only = !(d->flags & VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_READ); + } + /* Declare one array variable per Vulkan binding, and use it for * all array declarations which map to it. */ symbol.type = VKD3D_SYMBOL_DESCRIPTOR_ARRAY; @@ -6135,6 +6163,9 @@ static uint32_t spirv_compiler_build_descriptor_variable(struct spirv_compiler * spirv_compiler_emit_descriptor_binding(compiler, var_id, &binding); spirv_compiler_emit_register_debug_name(builder, var_id, reg); + if (write_only) + vkd3d_spirv_build_op_decorate(builder, var_id, SpvDecorationNonReadable, NULL, 0); + symbol.id = var_id; symbol.descriptor_array = NULL; symbol.info.descriptor_array.storage_class = storage_class; @@ -6190,7 +6221,7 @@ static void spirv_compiler_emit_cbv_declaration(struct spirv_compiler *compiler, vkd3d_spirv_build_op_name(builder, struct_id, "cb%u_struct", size); var_id = spirv_compiler_build_descriptor_variable(compiler, storage_class, struct_id, - ®, range, VKD3D_SHADER_RESOURCE_BUFFER, false, &var_info); + ®, range, VKD3D_SHADER_RESOURCE_BUFFER, false, false, &var_info); vkd3d_symbol_make_register(®_symbol, ®); vkd3d_symbol_set_register_info(®_symbol, var_id, storage_class, @@ -6247,7 +6278,7 @@ static void spirv_compiler_emit_sampler_declaration(struct spirv_compiler *compi type_id = vkd3d_spirv_get_op_type_sampler(builder); var_id = spirv_compiler_build_descriptor_variable(compiler, storage_class, type_id, ®, - range, VKD3D_SHADER_RESOURCE_NONE, false, &var_info); + range, VKD3D_SHADER_RESOURCE_NONE, false, false, &var_info); vkd3d_symbol_make_register(®_symbol, ®); vkd3d_symbol_set_register_info(®_symbol, var_id, storage_class, @@ -6294,26 +6325,6 @@ static SpvImageFormat image_format_for_image_read(enum vkd3d_shader_component_ty } } -static const struct vkd3d_shader_descriptor_info1 *spirv_compiler_get_descriptor_info( - struct spirv_compiler *compiler, enum vkd3d_shader_descriptor_type type, - const struct vkd3d_shader_register_range *range) -{ - const struct vkd3d_shader_scan_descriptor_info1 *descriptor_info = compiler->scan_descriptor_info; - unsigned int register_last = (range->last == ~0u) ? range->first : range->last; - const struct vkd3d_shader_descriptor_info1 *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 - && (d->count == ~0u || d->count > register_last - d->register_index)) - return d; - } - - return NULL; -} - static uint32_t spirv_compiler_get_image_type_id(struct spirv_compiler *compiler, const struct vkd3d_shader_register *reg, const struct vkd3d_shader_register_range *range, const struct vkd3d_spirv_resource_type *resource_type_info, enum vkd3d_shader_component_type data_type, @@ -6492,7 +6503,7 @@ static void spirv_compiler_emit_resource_declaration(struct spirv_compiler *comp } var_id = spirv_compiler_build_descriptor_variable(compiler, storage_class, type_id, ®, - range, resource_type, false, &var_info); + range, resource_type, is_uav, false, &var_info); if (is_uav) { @@ -6500,9 +6511,6 @@ static void spirv_compiler_emit_resource_declaration(struct spirv_compiler *comp d = spirv_compiler_get_descriptor_info(compiler, VKD3D_SHADER_DESCRIPTOR_TYPE_UAV, range); - if (!(d->flags & VKD3D_SHADER_DESCRIPTOR_INFO_FLAG_UAV_READ)) - vkd3d_spirv_build_op_decorate(builder, var_id, SpvDecorationNonReadable, NULL, 0); - /* ROVs are implicitly globally coherent. */ if (d->uav_flags & (VKD3DSUF_GLOBALLY_COHERENT | VKD3DSUF_RASTERISER_ORDERED_VIEW)) vkd3d_spirv_build_op_decorate(builder, var_id, SpvDecorationCoherent, NULL, 0); @@ -6549,7 +6557,7 @@ static void spirv_compiler_emit_resource_declaration(struct spirv_compiler *comp } counter_var_id = spirv_compiler_build_descriptor_variable(compiler, storage_class, - type_id, ®, range, resource_type, true, &counter_var_info); + type_id, ®, range, resource_type, false, true, &counter_var_info); } }