vk: descriptors: sort bindings along with flags

Vulkan spec requires binding flags to be matched with the binding with
the same index, however currently bindings are sorted with flags not
properly sorted, which leads to bindings and flags mismatch.

Resolve this by adding optional flags info to the parameters of
vk_create_sorted_bindings(), and refactoring panvk/pvr (which really
pair bindings and flags instead of only iterating flags) to use sorted
flags.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Reviewed-by: Ryan Mckeever <ryan.mckeever@collabora.com>
Reviewed-by: Simon Perretta <simon.perretta@imgtec.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38967>

src/amd/vulkan/radv_descriptor_set.c

@@ -85,7 +85,7 @@ radv_CreateDescriptorSetLayout(VkDevice _device, const VkDescriptorSetLayoutCrea
       set_layout->ycbcr_sampler_offsets_offset = 0;
 
    VkDescriptorSetLayoutBinding *bindings = NULL;
-   VkResult result = vk_create_sorted_bindings(pCreateInfo->pBindings, pCreateInfo->bindingCount, &bindings);
+   VkResult result = vk_create_sorted_bindings(pCreateInfo->pBindings, pCreateInfo->bindingCount, &bindings, NULL, NULL);
    if (result != VK_SUCCESS) {
       vk_descriptor_set_layout_unref(&device->vk, &set_layout->vk);
       return vk_error(device, result);
@@ -262,7 +262,7 @@ radv_GetDescriptorSetLayoutSupport(VkDevice _device, const VkDescriptorSetLayout
    const struct radv_physical_device *pdev = radv_device_physical(device);
 
    VkDescriptorSetLayoutBinding *bindings = NULL;
-   VkResult result = vk_create_sorted_bindings(pCreateInfo->pBindings, pCreateInfo->bindingCount, &bindings);
+   VkResult result = vk_create_sorted_bindings(pCreateInfo->pBindings, pCreateInfo->bindingCount, &bindings, NULL, NULL);
    if (result != VK_SUCCESS) {
       pSupport->supported = false;
       return;

src/broadcom/vulkan/v3dv_descriptor_set.c

@@ -714,7 +714,8 @@ v3dv_CreateDescriptorSetLayout(VkDevice _device,
 
    VkDescriptorSetLayoutBinding *bindings = NULL;
    VkResult result = vk_create_sorted_bindings(pCreateInfo->pBindings,
-                                               pCreateInfo->bindingCount, &bindings);
+                                               pCreateInfo->bindingCount, &bindings,
+                                               NULL, NULL);
    if (result != VK_SUCCESS) {
       v3dv_descriptor_set_layout_destroy(device, set_layout);
       return vk_error(device, result);
@@ -1338,7 +1339,7 @@ v3dv_GetDescriptorSetLayoutSupport(
    V3DV_FROM_HANDLE(v3dv_device, device, _device);
    VkDescriptorSetLayoutBinding *bindings = NULL;
    VkResult result = vk_create_sorted_bindings(
-      pCreateInfo->pBindings, pCreateInfo->bindingCount, &bindings);
+      pCreateInfo->pBindings, pCreateInfo->bindingCount, &bindings, NULL, NULL);
    if (result != VK_SUCCESS) {
       pSupport->supported = false;
       return;

src/freedreno/vulkan/tu_descriptor_set.cc

@@ -168,7 +168,7 @@ tu_CreateDescriptorSetLayout(
 
    VkDescriptorSetLayoutBinding *bindings = NULL;
    VkResult result = vk_create_sorted_bindings(
-      pCreateInfo->pBindings, pCreateInfo->bindingCount, &bindings);
+      pCreateInfo->pBindings, pCreateInfo->bindingCount, &bindings, NULL, NULL);
    if (result != VK_SUCCESS) {
       vk_object_free(&device->vk, pAllocator, set_layout);
       return vk_error(device, result);
@@ -316,7 +316,7 @@ tu_GetDescriptorSetLayoutSupport(
 
    VkDescriptorSetLayoutBinding *bindings = NULL;
    VkResult result = vk_create_sorted_bindings(
-      pCreateInfo->pBindings, pCreateInfo->bindingCount, &bindings);
+      pCreateInfo->pBindings, pCreateInfo->bindingCount, &bindings, NULL, NULL);
    if (result != VK_SUCCESS) {
       pSupport->supported = false;
       return;

src/gallium/frontends/lavapipe/lvp_descriptor_set.c

@@ -104,7 +104,7 @@ VKAPI_ATTR VkResult VKAPI_CALL lvp_CreateDescriptorSetLayout(
    VkDescriptorSetLayoutBinding *bindings = NULL;
    VkResult result = vk_create_sorted_bindings(pCreateInfo->pBindings,
                                                pCreateInfo->bindingCount,
-                                               &bindings);
+                                               &bindings, NULL, NULL);
    if (result != VK_SUCCESS) {
       vk_descriptor_set_layout_unref(&device->vk, &set_layout->vk);
       return vk_error(device, result);

src/imagination/vulkan/pvr_descriptor_set.c

@@ -122,6 +122,10 @@ VkResult pvr_CreateDescriptorSetLayout(
    uint32_t dynamic_buffer_count = 0;
    uint32_t descriptor_count = 0;
    VkResult result = VK_SUCCESS;
+   const VkDescriptorSetLayoutBindingFlagsCreateInfo *binding_flags_create_info =
+      vk_find_struct_const(pCreateInfo->pNext,
+                           DESCRIPTOR_SET_LAYOUT_BINDING_FLAGS_CREATE_INFO);
+   VkDescriptorBindingFlags *binding_flags = NULL;
 
    assert(pCreateInfo->sType ==
           VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO);
@@ -143,7 +147,9 @@ VkResult pvr_CreateDescriptorSetLayout(
 
    result = vk_create_sorted_bindings(pCreateInfo->pBindings,
                                       pCreateInfo->bindingCount,
-                                      &bindings);
+                                      &bindings,
+                                      binding_flags_create_info,
+                                      &binding_flags);
 
    if (result != VK_SUCCESS)
       return vk_error(device, result);
@@ -178,16 +184,6 @@ VkResult pvr_CreateDescriptorSetLayout(
    layout->immutable_sampler_count = immutable_sampler_count;
    layout->immutable_samplers = immutable_samplers;
 
-   const VkDescriptorSetLayoutBindingFlagsCreateInfo *binding_flags_create_info =
-      vk_find_struct_const(pCreateInfo->pNext,
-                           DESCRIPTOR_SET_LAYOUT_BINDING_FLAGS_CREATE_INFO);
-
-   if (binding_flags_create_info && !binding_flags_create_info->bindingCount)
-      binding_flags_create_info = NULL;
-
-   assert(!binding_flags_create_info ||
-          binding_flags_create_info->bindingCount == binding_count);
-
    unsigned dynamic_buffer_idx = 0;
    for (unsigned b = 0; b < pCreateInfo->bindingCount; ++b) {
       const VkDescriptorSetLayoutBinding *binding = &bindings[b];
@@ -214,9 +210,7 @@ VkResult pvr_CreateDescriptorSetLayout(
 
       layout_binding->type = binding->descriptorType;
 
-      layout_binding->flags = binding_flags_create_info
-                                 ? binding_flags_create_info->pBindingFlags[b]
-                                 : 0;
+      layout_binding->flags = binding_flags ? binding_flags[b] : 0;
 
       layout_binding->descriptor_count = binding->descriptorCount;
       layout_binding->stage_flags = binding->stageFlags;
@@ -240,6 +234,7 @@ VkResult pvr_CreateDescriptorSetLayout(
    assert(dynamic_buffer_count == dynamic_buffer_idx);
 
    free(bindings);
+   free(binding_flags);
 
    *pSetLayout = pvr_descriptor_set_layout_to_handle(layout);
 

src/microsoft/vulkan/dzn_descriptor_set.c

@@ -265,7 +265,7 @@ dzn_descriptor_set_layout_create(struct dzn_device *device,
    VkResult ret =
       vk_create_sorted_bindings(pCreateInfo->pBindings,
                                 pCreateInfo->bindingCount,
-                                &ordered_bindings);
+                                &ordered_bindings, NULL, NULL);
    if (ret != VK_SUCCESS) {
       vk_descriptor_set_layout_destroy(&device->vk, &set_layout->vk);
       return ret;

src/panfrost/vulkan/panvk_vX_descriptor_set_layout.c

@@ -91,6 +91,10 @@ panvk_per_arch(CreateDescriptorSetLayout)(
    VkDescriptorSetLayoutBinding *bindings = NULL;
    unsigned num_bindings = 0;
    VkResult result;
+   const VkDescriptorSetLayoutBindingFlagsCreateInfo *binding_flags_info =
+      vk_find_struct_const(pCreateInfo->pNext,
+                           DESCRIPTOR_SET_LAYOUT_BINDING_FLAGS_CREATE_INFO);
+   VkDescriptorBindingFlags *sorted_binding_flags = NULL;
 
    unsigned immutable_sampler_count = 0;
    for (uint32_t j = 0; j < pCreateInfo->bindingCount; j++) {
@@ -114,7 +118,9 @@ panvk_per_arch(CreateDescriptorSetLayout)(
 
    if (pCreateInfo->bindingCount) {
       result = vk_create_sorted_bindings(pCreateInfo->pBindings,
-                                         pCreateInfo->bindingCount, &bindings);
+                                         pCreateInfo->bindingCount, &bindings,
+                                         binding_flags_info,
+                                         &sorted_binding_flags);
       if (result != VK_SUCCESS)
          return panvk_error(device, result);
 
@@ -137,10 +143,6 @@ panvk_per_arch(CreateDescriptorSetLayout)(
    layout->bindings = binding_layouts;
    layout->binding_count = num_bindings;
 
-   const VkDescriptorSetLayoutBindingFlagsCreateInfo *binding_flags_info =
-      vk_find_struct_const(pCreateInfo->pNext,
-                           DESCRIPTOR_SET_LAYOUT_BINDING_FLAGS_CREATE_INFO);
-
    unsigned desc_idx = 0;
    unsigned dyn_buf_idx = 0;
    for (unsigned i = 0; i < pCreateInfo->bindingCount; i++) {
@@ -153,9 +155,8 @@ panvk_per_arch(CreateDescriptorSetLayout)(
 
       binding_layout->type = binding->descriptorType;
 
-      if (binding_flags_info && binding_flags_info->bindingCount > 0) {
-         assert(binding_flags_info->bindingCount == pCreateInfo->bindingCount);
-         binding_layout->flags = binding_flags_info->pBindingFlags[i];
+      if (sorted_binding_flags) {
+         binding_layout->flags = sorted_binding_flags[i];
       }
 
       if (binding_layout->type == VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK) {
@@ -233,6 +234,7 @@ panvk_per_arch(CreateDescriptorSetLayout)(
    _mesa_blake3_final(&hash_ctx, layout->vk.blake3);
 
    free(bindings);
+   free(sorted_binding_flags);
    *pSetLayout = panvk_descriptor_set_layout_to_handle(layout);
 
    return VK_SUCCESS;

src/vulkan/runtime/vk_descriptors.c

@@ -28,21 +28,34 @@
 #include "vk_common_entrypoints.h"
 #include "util/macros.h"
 
+struct binding_with_flags {
+   VkDescriptorSetLayoutBinding binding;
+   VkDescriptorBindingFlags flags;
+};
+
 static int
 binding_compare(const void* av, const void *bv)
 {
-   const VkDescriptorSetLayoutBinding *a = (const VkDescriptorSetLayoutBinding*)av;
-   const VkDescriptorSetLayoutBinding *b = (const VkDescriptorSetLayoutBinding*)bv;
+   const struct binding_with_flags *a = (const struct binding_with_flags*)av;
+   const struct binding_with_flags *b = (const struct binding_with_flags*)bv;
  
-   return (a->binding < b->binding) ? -1 : (a->binding > b->binding) ? 1 : 0;
+   return (a->binding.binding < b->binding.binding) ? -1 :
+          (a->binding.binding > b->binding.binding) ? 1 : 0;
 }
  
 VkResult
 vk_create_sorted_bindings(const VkDescriptorSetLayoutBinding *bindings, unsigned count,
-                          VkDescriptorSetLayoutBinding **sorted_bindings)
+                          VkDescriptorSetLayoutBinding **sorted_bindings,
+                          const VkDescriptorSetLayoutBindingFlagsCreateInfo *binding_flags_info,
+                          VkDescriptorBindingFlags **sorted_binding_flags)
 {
+   struct binding_with_flags *bwfs;
+   unsigned index;
+
    if (!count) {
       *sorted_bindings = NULL;
+      if (sorted_binding_flags)
+         *sorted_binding_flags = NULL;
       return VK_SUCCESS;
    }
 
@@ -50,8 +63,47 @@ vk_create_sorted_bindings(const VkDescriptorSetLayoutBinding *bindings, unsigned
    if (!*sorted_bindings)
       return VK_ERROR_OUT_OF_HOST_MEMORY;
 
-   memcpy(*sorted_bindings, bindings, count * sizeof(VkDescriptorSetLayoutBinding));
-   qsort(*sorted_bindings, count, sizeof(VkDescriptorSetLayoutBinding), binding_compare);
+   if (binding_flags_info && !binding_flags_info->bindingCount)
+      binding_flags_info = NULL;
+
+   if (binding_flags_info) {
+      assert(sorted_binding_flags);
+      assert(binding_flags_info->bindingCount == count);
+
+      *sorted_binding_flags = malloc(count * sizeof(VkDescriptorBindingFlags));
+      if (!*sorted_binding_flags) {
+         free(*sorted_bindings);
+         return VK_ERROR_OUT_OF_HOST_MEMORY;
+      }
+   }
+
+   bwfs = malloc(count * sizeof(struct binding_with_flags));
+   if (!bwfs) {
+      if (binding_flags_info)
+         free(*sorted_binding_flags);
+      free(*sorted_bindings);
+      return VK_ERROR_OUT_OF_HOST_MEMORY;
+   }
+
+   for (index = 0; index < count; index++) {
+      memcpy(&bwfs[index].binding, &bindings[index],
+             sizeof(VkDescriptorSetLayoutBinding));
+      if (binding_flags_info)
+         bwfs[index].flags = binding_flags_info->pBindingFlags[index];
+      else
+         bwfs[index].flags = 0;
+   }
+
+   qsort(bwfs, count, sizeof(struct binding_with_flags), binding_compare);
+
+   for (index = 0; index < count; index++) {
+      memcpy(&(*sorted_bindings)[index], &bwfs[index].binding,
+             sizeof(VkDescriptorSetLayoutBinding));
+      if (binding_flags_info)
+         (*sorted_binding_flags)[index] = bwfs[index].flags;
+   }
+
+   free(bwfs);
  
    return VK_SUCCESS;
 }

src/vulkan/runtime/vk_descriptors.h

@@ -33,7 +33,9 @@ extern "C" {
 
 VkResult
 vk_create_sorted_bindings(const VkDescriptorSetLayoutBinding *bindings, unsigned count,
-                          VkDescriptorSetLayoutBinding **sorted_bindings);
+                          VkDescriptorSetLayoutBinding **sorted_bindings,
+                          const VkDescriptorSetLayoutBindingFlagsCreateInfo *binding_flags_info,
+                          VkDescriptorBindingFlags **sorted_binding_flags);
 
 #ifdef __cplusplus
 }