From 69ff204422b6eb474bd165177492f0c71017b5ed Mon Sep 17 00:00:00 2001 From: Samuel Pitoiset Date: Mon, 12 May 2025 08:45:45 +0200 Subject: [PATCH] radv: remove the optimization for equal immutable samplers This optimization used to optimize the allocated space for descriptors when immutable samplers are equal. Though, this was basically broken : - descriptor copies were broken for combiner image sampler (or sampler) with equal immutable samplers because 96 bytes were copied instead of 64 bytes (cf. the linked ticket). This could be fixed but it's not worth it. - the value returned by vkGetDescriptorLayoutSupport() was broken, it should have been 96 with no immutable samplers (or when they aren't equal) This optimization was also not applied for descriptor buffers which is the default for vkd3d-proton and Zink. DXVK doesn't use db but it doesn't use immutable samplers, so basically only native vulkan games would be concerned. Note that immutable samplers would still be inlined in shaders if no indirect access which should be 99.9% of the usecase. Cc: mesa-stable Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/11165 Signed-off-by: Samuel Pitoiset Part-of: --- .../nir/radv_nir_apply_pipeline_layout.c | 16 +++---- src/amd/vulkan/radv_descriptor_set.c | 47 ++----------------- src/amd/vulkan/radv_descriptor_set.h | 3 +- 3 files changed, 13 insertions(+), 53 deletions(-) diff --git a/src/amd/vulkan/nir/radv_nir_apply_pipeline_layout.c b/src/amd/vulkan/nir/radv_nir_apply_pipeline_layout.c index 7133952664e..77dd6de682b 100644 --- a/src/amd/vulkan/nir/radv_nir_apply_pipeline_layout.c +++ b/src/amd/vulkan/nir/radv_nir_apply_pipeline_layout.c @@ -208,16 +208,14 @@ get_sampler_desc(nir_builder *b, apply_layout_state *state, nir_deref_instr *der * index or if all samplers in the array are the same. Note that indexing is forbidden with * embedded samplers. */ - if (desc_type == AC_DESC_SAMPLER && binding->immutable_samplers_offset && - (!indirect || binding->immutable_samplers_equal)) { + if (desc_type == AC_DESC_SAMPLER && binding->immutable_samplers_offset && !indirect) { unsigned constant_index = 0; - if (!binding->immutable_samplers_equal) { - while (deref->deref_type != nir_deref_type_var) { - assert(deref->deref_type == nir_deref_type_array); - unsigned array_size = MAX2(glsl_get_aoa_size(deref->type), 1); - constant_index += nir_src_as_uint(deref->arr.index) * array_size; - deref = nir_deref_instr_parent(deref); - } + + while (deref->deref_type != nir_deref_type_var) { + assert(deref->deref_type == nir_deref_type_array); + unsigned array_size = MAX2(glsl_get_aoa_size(deref->type), 1); + constant_index += nir_src_as_uint(deref->arr.index) * array_size; + deref = nir_deref_instr_parent(deref); } uint32_t dword0_mask = diff --git a/src/amd/vulkan/radv_descriptor_set.c b/src/amd/vulkan/radv_descriptor_set.c index e70f10f7ed8..afbf47e80ee 100644 --- a/src/amd/vulkan/radv_descriptor_set.c +++ b/src/amd/vulkan/radv_descriptor_set.c @@ -46,19 +46,6 @@ radv_descriptor_type_buffer_count(VkDescriptorType type) } } -static bool -has_equal_immutable_samplers(const VkSampler *samplers, uint32_t count) -{ - if (!samplers) - return false; - for (uint32_t i = 1; i < count; ++i) { - if (memcmp(radv_sampler_from_handle(samplers[0])->state, radv_sampler_from_handle(samplers[i])->state, 16)) { - return false; - } - } - return true; -} - static uint32_t radv_descriptor_alignment(VkDescriptorType type) { @@ -317,26 +304,9 @@ radv_CreateDescriptorSetLayout(VkDevice _device, const VkDescriptorSetLayoutCrea set_layout->binding[b].immutable_samplers_offset = samplers_offset; set_layout->has_immutable_samplers = true; - /* Do not optimize space for descriptor buffers and embedded samplers, otherwise the set - * layout size/offset are incorrect. - */ - if (!(pCreateInfo->flags & (VK_DESCRIPTOR_SET_LAYOUT_CREATE_DESCRIPTOR_BUFFER_BIT_EXT | - VK_DESCRIPTOR_SET_LAYOUT_CREATE_EMBEDDED_IMMUTABLE_SAMPLERS_BIT_EXT))) { - set_layout->binding[b].immutable_samplers_equal = - has_equal_immutable_samplers(binding->pImmutableSamplers, binding->descriptorCount); - } - for (uint32_t i = 0; i < binding->descriptorCount; i++) memcpy(samplers + 4 * i, &radv_sampler_from_handle(binding->pImmutableSamplers[i])->state, 16); - /* Don't reserve space for the samplers if they're not accessed. */ - if (set_layout->binding[b].immutable_samplers_equal) { - if (binding->descriptorType == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER && - max_sampled_image_descriptors <= 2) - set_layout->binding[b].size -= 32; - else if (binding->descriptorType == VK_DESCRIPTOR_TYPE_SAMPLER) - set_layout->binding[b].size -= 16; - } samplers += 4 * binding->descriptorCount; samplers_offset += 4 * sizeof(uint32_t) * binding->descriptorCount; @@ -437,16 +407,10 @@ radv_GetDescriptorSetLayoutSupport(VkDevice device, const VkDescriptorSetLayoutC descriptor_size = 64; break; case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: - if (!has_equal_immutable_samplers(binding->pImmutableSamplers, descriptor_count)) { - descriptor_size = 64; - } else { - descriptor_size = 96; - } + descriptor_size = 96; break; case VK_DESCRIPTOR_TYPE_SAMPLER: - if (!has_equal_immutable_samplers(binding->pImmutableSamplers, descriptor_count)) { - descriptor_size = 16; - } + descriptor_size = 16; break; case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK: descriptor_size = descriptor_count; @@ -712,7 +676,7 @@ radv_descriptor_set_create(struct radv_device *device, struct radv_descriptor_po if (layout->has_immutable_samplers) { for (unsigned i = 0; i < layout->binding_count; ++i) { - if (!layout->binding[i].immutable_samplers_offset || layout->binding[i].immutable_samplers_equal) + if (!layout->binding[i].immutable_samplers_offset) continue; unsigned offset = layout->binding[i].offset / 4; @@ -1269,8 +1233,7 @@ radv_update_descriptor_sets_impl(struct radv_device *device, struct radv_cmd_buf * allocated, so if we are writing push descriptors we have to copy the * immutable samplers into them now. */ - const bool copy_immutable_samplers = - cmd_buffer && binding_layout->immutable_samplers_offset && !binding_layout->immutable_samplers_equal; + const bool copy_immutable_samplers = cmd_buffer && binding_layout->immutable_samplers_offset; const uint32_t *samplers = radv_immutable_samplers(set->header.layout, binding_layout); const VkWriteDescriptorSetAccelerationStructureKHR *accel_structs = NULL; @@ -1496,7 +1459,7 @@ radv_CreateDescriptorUpdateTemplate(VkDevice _device, const VkDescriptorUpdateTe case VK_DESCRIPTOR_TYPE_SAMPLER: /* Immutable samplers are copied into push descriptors when they are pushed */ if (pCreateInfo->templateType == VK_DESCRIPTOR_UPDATE_TEMPLATE_TYPE_PUSH_DESCRIPTORS_KHR && - binding_layout->immutable_samplers_offset && !binding_layout->immutable_samplers_equal) { + binding_layout->immutable_samplers_offset) { immutable_samplers = radv_immutable_samplers(set_layout, binding_layout) + entry->dstArrayElement * 4; } break; diff --git a/src/amd/vulkan/radv_descriptor_set.h b/src/amd/vulkan/radv_descriptor_set.h index 96602d132c3..3495c42f93d 100644 --- a/src/amd/vulkan/radv_descriptor_set.h +++ b/src/amd/vulkan/radv_descriptor_set.h @@ -33,7 +33,6 @@ struct radv_descriptor_set_binding_layout { /* Offset in the radv_descriptor_set_layout of the immutable samplers, or 0 * if there are no immutable samplers. */ uint32_t immutable_samplers_offset; - bool immutable_samplers_equal; }; struct radv_descriptor_set_layout { @@ -200,7 +199,7 @@ radv_immutable_samplers(const struct radv_descriptor_set_layout *set, static inline unsigned radv_combined_image_descriptor_sampler_offset(const struct radv_descriptor_set_binding_layout *binding) { - return binding->size - ((!binding->immutable_samplers_equal) ? 16 : 0); + return binding->size - 16; } static inline const struct vk_ycbcr_conversion_state *