anv: do not rely on sampler objects for pipeline compilation

Descriptor set layout lifetime can be shorter than what the
implementation requires. One example is :
  * create descriptor set layout
  * create graphics pipeline library
  * destroy descriptor set layout
  * link optimize library in a final pipeline

The last step might need the descriptor set layout information again.

We've so far worked around this by taking a reference on the
descriptor set layout in the pipelines. But we forgot that descriptor
set layouts have pointers to samplers (for immutable & embedded
samplers).

We could take a reference to samplers but that sucks for various
reasons :
  - it consumes dynamic state heap space
  - it could cause issues with capture-replay placement

So instead we copy the information from the samplers that might be
needed in cases like link optimization. This includes :
  - ycbcr conversion state (used for NIR lowering)
  - embedded sampler data (to recreate the sampler)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: mesa-stable
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/35955>

src/intel/vulkan/anv_descriptor_set.c

@@ -668,7 +668,7 @@ VkResult anv_CreateDescriptorSetLayout(
    VK_MULTIALLOC_DECL(&ma, struct anv_descriptor_set_layout, set_layout, 1);
    VK_MULTIALLOC_DECL(&ma, struct anv_descriptor_set_binding_layout,
                            bindings, num_bindings);
-   VK_MULTIALLOC_DECL(&ma, struct anv_sampler *, samplers,
+   VK_MULTIALLOC_DECL(&ma, struct anv_descriptor_set_layout_sampler, samplers,
                            immutable_sampler_count);
 
    if (!vk_object_multizalloc(&device->vk, &ma, NULL,
@@ -689,7 +689,7 @@ VkResult anv_CreateDescriptorSetLayout(
       set_layout->binding[b].data = 0;
       set_layout->binding[b].max_plane_count = 0;
       set_layout->binding[b].array_size = 0;
-      set_layout->binding[b].immutable_samplers = NULL;
+      set_layout->binding[b].samplers = NULL;
    }
 
    /* Initialize all samplers to 0 */
@@ -711,7 +711,7 @@ VkResult anv_CreateDescriptorSetLayout(
        * immutable_samplers pointer.  This provides us with a quick-and-dirty
        * way to sort the bindings by binding number.
        */
-      set_layout->binding[b].immutable_samplers = (void *)(uintptr_t)(j + 1);
+      set_layout->binding[b].samplers = (void *)(uintptr_t)(j + 1);
    }
 
    const VkDescriptorSetLayoutBindingFlagsCreateInfo *binding_flags_info =
@@ -722,16 +722,20 @@ VkResult anv_CreateDescriptorSetLayout(
       vk_find_struct_const(pCreateInfo->pNext,
                            MUTABLE_DESCRIPTOR_TYPE_CREATE_INFO_EXT);
 
+   const bool has_embedded_samplers =
+      pCreateInfo->flags &
+      VK_DESCRIPTOR_SET_LAYOUT_CREATE_EMBEDDED_IMMUTABLE_SAMPLERS_BIT_EXT;
+
    for (uint32_t b = 0; b < num_bindings; b++) {
       /* We stashed the pCreateInfo->pBindings[] index (plus one) in the
        * immutable_samplers pointer.  Check for NULL (empty binding) and then
        * reset it and compute the index.
        */
-      if (set_layout->binding[b].immutable_samplers == NULL)
+      if (set_layout->binding[b].samplers == NULL)
          continue;
       const uint32_t info_idx =
-         (uintptr_t)(void *)set_layout->binding[b].immutable_samplers - 1;
-      set_layout->binding[b].immutable_samplers = NULL;
+         (uintptr_t)(void *)set_layout->binding[b].samplers - 1;
+      set_layout->binding[b].samplers = NULL;
 
       const VkDescriptorSetLayoutBinding *binding =
          &pCreateInfo->pBindings[info_idx];
@@ -790,14 +794,27 @@ VkResult anv_CreateDescriptorSetLayout(
       case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
       case VK_DESCRIPTOR_TYPE_MUTABLE_EXT:
          if (binding->pImmutableSamplers) {
-            set_layout->binding[b].immutable_samplers = samplers;
+            set_layout->binding[b].samplers = samplers;
             samplers += binding->descriptorCount;
 
             for (uint32_t i = 0; i < binding->descriptorCount; i++) {
                ANV_FROM_HANDLE(anv_sampler, sampler,
                                binding->pImmutableSamplers[i]);
 
-               set_layout->binding[b].immutable_samplers[i] = sampler;
+               set_layout->binding[b].samplers[i] =
+                  (struct anv_descriptor_set_layout_sampler) {
+                  .immutable_sampler = sampler,
+               };
+               if (has_embedded_samplers) {
+                  set_layout->binding[b].samplers[i].embedded_key =
+                     sampler->embedded_key;
+               }
+               if (sampler->vk.ycbcr_conversion) {
+                  set_layout->binding[b].samplers[i].has_ycbcr_conversion = true;
+                  set_layout->binding[b].samplers[i].ycbcr_conversion_state =
+                     sampler->vk.ycbcr_conversion->state;
+               }
+
                if (set_layout->binding[b].max_plane_count < sampler->n_planes)
                   set_layout->binding[b].max_plane_count = sampler->n_planes;
             }
@@ -886,8 +903,7 @@ VkResult anv_CreateDescriptorSetLayout(
    set_layout->descriptor_buffer_surface_size = descriptor_buffer_surface_size;
    set_layout->descriptor_buffer_sampler_size = descriptor_buffer_sampler_size;
 
-   if (pCreateInfo->flags &
-       VK_DESCRIPTOR_SET_LAYOUT_CREATE_EMBEDDED_IMMUTABLE_SAMPLERS_BIT_EXT) {
+   if (has_embedded_samplers) {
       assert(set_layout->descriptor_buffer_surface_size == 0);
       assert(set_layout->descriptor_buffer_sampler_size == 0);
       set_layout->embedded_sampler_count = sampler_count;
@@ -1031,26 +1047,6 @@ anv_descriptor_set_layout_print(const struct anv_descriptor_set_layout *layout)
 
 #define SHA1_UPDATE_VALUE(ctx, x) _mesa_sha1_update(ctx, &(x), sizeof(x));
 
-static void
-sha1_update_immutable_sampler(struct mesa_sha1 *ctx,
-                              bool embedded_sampler,
-                              const struct anv_sampler *sampler)
-{
-   /* Hash the conversion if any as this affect shader compilation due to NIR
-    * lowering.
-    */
-   if (sampler->vk.ycbcr_conversion)
-      SHA1_UPDATE_VALUE(ctx, sampler->vk.ycbcr_conversion->state);
-
-   /* For embedded samplers, we need to hash the sampler parameters as the
-    * sampler handle is baked into the shader and this ultimately is part of
-    * the shader hash key. We can only consider 2 shaders identical if all
-    * their embedded samplers parameters are identical.
-    */
-   if (embedded_sampler)
-      SHA1_UPDATE_VALUE(ctx, sampler->embedded_key);
-}
-
 static void
 sha1_update_descriptor_set_binding_layout(struct mesa_sha1 *ctx,
                                           bool embedded_samplers,
@@ -1066,10 +1062,21 @@ sha1_update_descriptor_set_binding_layout(struct mesa_sha1 *ctx,
    SHA1_UPDATE_VALUE(ctx, layout->descriptor_surface_offset);
    SHA1_UPDATE_VALUE(ctx, layout->descriptor_sampler_offset);
 
-   if (layout->immutable_samplers) {
+   if (layout->samplers) {
       for (uint16_t i = 0; i < layout->array_size; i++) {
-         sha1_update_immutable_sampler(ctx, embedded_samplers,
-                                       layout->immutable_samplers[i]);
+         /* For embedded samplers, we need to hash the sampler parameters as
+          * the sampler handle is baked into the shader and this ultimately is
+          * part of the shader hash key. We can only consider 2 shaders
+          * identical if all their embedded samplers parameters are identical.
+          */
+         if (embedded_samplers)
+            SHA1_UPDATE_VALUE(ctx, layout->samplers[i].embedded_key);
+
+         /* Hash the conversion if any as this affect shader compilation due
+          * to NIR lowering.
+          */
+         if (layout->samplers[i].has_ycbcr_conversion)
+            SHA1_UPDATE_VALUE(ctx, layout->samplers[i].ycbcr_conversion_state);
       }
    }
 }
@@ -1087,7 +1094,7 @@ sha1_update_descriptor_set_layout(struct mesa_sha1 *ctx,
    SHA1_UPDATE_VALUE(ctx, layout->descriptor_buffer_surface_size);
    SHA1_UPDATE_VALUE(ctx, layout->descriptor_buffer_sampler_size);
 
-   bool embedded_samplers =
+   const bool embedded_samplers =
       layout->flags & VK_DESCRIPTOR_SET_LAYOUT_CREATE_EMBEDDED_IMMUTABLE_SAMPLERS_BIT_EXT;
 
    for (uint16_t i = 0; i < layout->binding_count; i++) {
@@ -1817,7 +1824,7 @@ anv_descriptor_set_create(struct anv_device *device,
 
    /* Go through and fill out immutable samplers if we have any */
    for (uint32_t b = 0; b < layout->binding_count; b++) {
-      if (layout->binding[b].immutable_samplers) {
+      if (layout->binding[b].samplers) {
          for (uint32_t i = 0; i < layout->binding[b].array_size; i++) {
             /* The type will get changed to COMBINED_IMAGE_SAMPLER in
              * UpdateDescriptorSets if needed.  However, if the descriptor
@@ -2187,15 +2194,15 @@ anv_descriptor_set_write_image_view(struct anv_device *device,
 
    switch (type) {
    case VK_DESCRIPTOR_TYPE_SAMPLER:
-      sampler = bind_layout->immutable_samplers ?
-                bind_layout->immutable_samplers[element] :
+      sampler = bind_layout->samplers ?
+                bind_layout->samplers[element].immutable_sampler :
                 anv_sampler_from_handle(info->sampler);
       break;
 
    case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
       image_view = anv_image_view_from_handle(info->imageView);
-      sampler = bind_layout->immutable_samplers ?
-                bind_layout->immutable_samplers[element] :
+      sampler = bind_layout->samplers ?
+                bind_layout->samplers[element].immutable_sampler :
                 anv_sampler_from_handle(info->sampler);
       break;
 

src/intel/vulkan/anv_nir_apply_pipeline_layout.c

@@ -2287,7 +2287,8 @@ add_embedded_sampler_entry(struct apply_pipeline_layout_state *state,
       state->layout->set[set].layout;
    const struct anv_descriptor_set_binding_layout *bind_layout =
       &set_layout->binding[binding];
-   const struct anv_sampler *sampler = bind_layout->immutable_samplers[0];
+   const struct anv_descriptor_set_layout_sampler *sampler =
+      &bind_layout->samplers[0];
 
    *sampler_bind = (struct anv_pipeline_embedded_sampler_binding) {
       .set = set,
@@ -2488,12 +2489,16 @@ build_packed_binding_table(struct apply_pipeline_layout_state *state,
          } else {
             state->set[set].binding[b].surface_offset = map->surface_count;
             if (binding->dynamic_offset_index < 0) {
-               struct anv_sampler **samplers = binding->immutable_samplers;
-               uint8_t max_planes = bti_multiplier(state, set, b);
+               const uint8_t max_planes = bti_multiplier(state, set, b);
                for (unsigned i = 0; i < binding->array_size; i++) {
-                  uint8_t planes = samplers ? samplers[i]->n_planes : 1;
+                  const uint8_t max_sampler_planes =
+                     (binding->samplers &&
+                      binding->samplers[i].has_ycbcr_conversion) ?
+                     vk_format_get_plane_count(
+                        binding->samplers[i].ycbcr_conversion_state.format) :
+                     1;
                   for (uint8_t p = 0; p < max_planes; p++) {
-                     if (p < planes) {
+                     if (p < max_sampler_planes) {
                         add_bti_entry(map, set, b, i, p, binding);
                      } else {
                         add_null_bti_entry(map);

src/intel/vulkan/anv_pipeline.c

@@ -813,16 +813,16 @@ lookup_ycbcr_conversion(const void *_sets_layout, uint32_t set,
    const struct anv_descriptor_set_binding_layout *bind_layout =
       &sets_layout->set[set].layout->binding[binding];
 
-   if (bind_layout->immutable_samplers == NULL)
+   if (bind_layout->samplers == NULL)
       return NULL;
 
    array_index = MIN2(array_index, bind_layout->array_size - 1);
 
-   const struct anv_sampler *sampler =
-      bind_layout->immutable_samplers[array_index];
+   const struct anv_descriptor_set_layout_sampler *sampler =
+      &bind_layout->samplers[array_index];
 
-   return sampler && sampler->vk.ycbcr_conversion ?
-          &sampler->vk.ycbcr_conversion->state : NULL;
+   return sampler->has_ycbcr_conversion ?
+          &sampler->ycbcr_conversion_state : NULL;
 }
 
 static void

src/intel/vulkan/anv_private.h

@@ -2918,6 +2918,36 @@ enum anv_descriptor_data {
    ANV_DESCRIPTOR_SURFACE_SAMPLER         = BITFIELD_BIT(9),
 };
 
+struct anv_embedded_sampler_key {
+   /** No need to track binding elements for embedded samplers as :
+    *
+    *    VUID-VkDescriptorSetLayoutBinding-flags-08006:
+    *
+    *       "If VkDescriptorSetLayoutCreateInfo:flags contains
+    *        VK_DESCRIPTOR_SET_LAYOUT_CREATE_EMBEDDED_IMMUTABLE_SAMPLERS_BIT_EXT,
+    *        descriptorCount must: less than or equal to 1"
+    *
+    * The following struct can be safely hash as it doesn't include in
+    * address/offset.
+    */
+   uint32_t sampler[4];
+   uint32_t color[4];
+};
+
+struct anv_descriptor_set_layout_sampler {
+   /* Immutable sampler used to populate descriptor sets on allocation */
+   struct anv_sampler *immutable_sampler;
+
+   /* Hashing key for embedded samplers */
+   struct anv_embedded_sampler_key embedded_key;
+
+   /* Whether ycbcr_conversion_state hold any data */
+   bool has_ycbcr_conversion;
+
+   /* YCbCr conversion state (only valid if has_ycbcr_conversion is true) */
+   struct vk_ycbcr_conversion_state ycbcr_conversion_state;
+};
+
 struct anv_descriptor_set_binding_layout {
    /* The type of the descriptors in this binding */
    VkDescriptorType type;
@@ -2969,8 +2999,8 @@ struct anv_descriptor_set_binding_layout {
     */
    uint16_t descriptor_sampler_stride;
 
-   /* Immutable samplers (or NULL if no immutable samplers) */
-   struct anv_sampler **immutable_samplers;
+   /* Sampler data (or NULL if no embedded/immutable samplers) */
+   struct anv_descriptor_set_layout_sampler *samplers;
 };
 
 enum anv_descriptor_set_layout_type {
@@ -3360,22 +3390,6 @@ struct anv_pipeline_binding {
    };
 };
 
-struct anv_embedded_sampler_key {
-   /** No need to track binding elements for embedded samplers as :
-    *
-    *    VUID-VkDescriptorSetLayoutBinding-flags-08006:
-    *
-    *       "If VkDescriptorSetLayoutCreateInfo:flags contains
-    *        VK_DESCRIPTOR_SET_LAYOUT_CREATE_EMBEDDED_IMMUTABLE_SAMPLERS_BIT_EXT,
-    *        descriptorCount must: less than or equal to 1"
-    *
-    * The following struct can be safely hash as it doesn't include in
-    * address/offset.
-    */
-   uint32_t sampler[4];
-   uint32_t color[4];
-};
-
 struct anv_pipeline_embedded_sampler_binding {
    /** The descriptor set this sampler belongs to */
    uint8_t set;