v3dv: remove combined_idx support

Now that the v3d compiler has support for separated texture and
sampler indices, we can stop to combine them. Again, that's what
Vulkan allows after all.

As we are doing this we can't use anymore the texture format (coming
from the texture) to chose the return size (that is a sampling
parameter). We default for 32, and just go to 16 for shadow. We plan
to use SPIR-V RelaxedPrecision to use in more cases 16 bit. We would
do that on following patches.

v2 (from Iago feedback):
   * Fix typos/bad grammar on comments.
   * Move tex/sampler number assert to before the loop that fills
     tex/sampler info.

Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7545>

src/broadcom/vulkan/v3dv_pipeline.c

@@ -177,9 +177,6 @@ v3dv_destroy_pipeline(struct v3dv_pipeline *pipeline,
       pipeline->default_attribute_values = NULL;
    }
 
-   if (pipeline->combined_index_map)
-      _mesa_hash_table_destroy(pipeline->combined_index_map, NULL);
-
    if (pipeline->default_attribute_values)
       v3dv_bo_free(device, pipeline->default_attribute_values);
 
@@ -634,42 +631,6 @@ lower_vulkan_resource_index(nir_builder *b,
    nir_instr_remove(&instr->instr);
 }
 
-static struct hash_table *
-pipeline_ensure_combined_index_map(struct v3dv_pipeline *pipeline)
-{
-   if (pipeline->combined_index_map == NULL) {
-      pipeline->combined_index_map =
-         _mesa_hash_table_create(NULL, _mesa_hash_u32, _mesa_key_u32_equal);
-      pipeline->next_combined_index = 0;
-   }
-
-   assert(pipeline->combined_index_map);
-
-   return pipeline->combined_index_map;
-}
-
-static uint32_t
-get_combined_index(struct v3dv_pipeline *pipeline,
-                   uint32_t texture_index,
-                   uint32_t sampler_index)
-{
-   struct hash_table *ht = pipeline_ensure_combined_index_map(pipeline);
-   uint32_t key = v3dv_pipeline_combined_index_key_create(texture_index, sampler_index);
-   struct hash_entry *entry = _mesa_hash_table_search(ht, &key);
-
-   if (entry)
-      return (uint32_t)(uintptr_t) (entry->data);
-
-   uint32_t new_index = pipeline->next_combined_index;
-   pipeline->next_combined_index++;
-
-   pipeline->combined_index_to_key_map[new_index] = key;
-   _mesa_hash_table_insert(ht, &pipeline->combined_index_to_key_map[new_index],
-                           (void *)(uintptr_t) (new_index));
-
-   return new_index;
-}
-
 static void
 lower_tex_src_to_offset(nir_builder *b, nir_tex_instr *instr, unsigned src_idx,
                         struct v3dv_pipeline *pipeline,
@@ -775,13 +736,8 @@ lower_sampler(nir_builder *b, nir_tex_instr *instr,
    if (texture_idx < 0 && sampler_idx < 0)
       return false;
 
-   int combined_index =
+   instr->sampler_index = sampler_idx < 0 ?
-      get_combined_index(pipeline,
+      V3DV_NO_SAMPLER_IDX : instr->sampler_index;
-                         instr->texture_index,
-                         sampler_idx < 0 ? V3DV_NO_SAMPLER_IDX : instr->sampler_index);
-
-   instr->texture_index = combined_index;
-   instr->sampler_index = combined_index;
 
    return true;
 }
@@ -847,13 +803,7 @@ lower_image_deref(nir_builder *b,
                          binding_layout->array_size,
                          false /* is_shadow: Doesn't really matter in this case */);
 
-   /* We still need to get a combined_index, as we are integrating images with
+   index = nir_imm_int(b, desc_index);
-    * the rest of the texture/sampler support
-    */
-   int combined_index =
-      get_combined_index(pipeline, desc_index, V3DV_NO_SAMPLER_IDX);
-
-   index = nir_imm_int(b, combined_index);
 
    nir_rewrite_image_intrinsic(instr, index, false);
 }
@@ -1012,31 +962,32 @@ pipeline_populate_v3d_key(struct v3d_key *key,
                           bool robust_buffer_access)
 {
    /* The following values are default values used at pipeline create. We use
-    * there 16 bit as default return size.
+    * there 32 bit as default return size.
     */
+   struct v3dv_descriptor_map *sampler_map = &p_stage->pipeline->sampler_map;
+   struct v3dv_descriptor_map *texture_map = &p_stage->pipeline->texture_map;
 
-   /* We don't use the nir shader info.num_textures because that doesn't take
+   key->num_tex_used = texture_map->num_desc;
-    * into account input attachments, even after calling
-    * nir_lower_input_attachments. As a general rule that makes sense, but on
-    * our case we are handling them mostly as textures. We iterate through the
-    * combined_index_map that was filled with the textures sused on th sader.
-    */
-   uint32_t tex_idx = 0;
-   if (p_stage->pipeline->combined_index_map) {
-      hash_table_foreach(p_stage->pipeline->combined_index_map, entry) {
-         key->tex[tex_idx].swizzle[0] = PIPE_SWIZZLE_X;
-         key->tex[tex_idx].swizzle[1] = PIPE_SWIZZLE_Y;
-         key->tex[tex_idx].swizzle[2] = PIPE_SWIZZLE_Z;
-         key->tex[tex_idx].swizzle[3] = PIPE_SWIZZLE_W;
-
-         key->sampler[tex_idx].return_size = 16;
-         key->sampler[tex_idx].return_channels = 2;
-
-         tex_idx++;
-      }
-   }
-   key->num_tex_used = tex_idx;
    assert(key->num_tex_used <= V3D_MAX_TEXTURE_SAMPLERS);
+   for (uint32_t tex_idx = 0; tex_idx < texture_map->num_desc; tex_idx++) {
+      key->tex[tex_idx].swizzle[0] = PIPE_SWIZZLE_X;
+      key->tex[tex_idx].swizzle[1] = PIPE_SWIZZLE_Y;
+      key->tex[tex_idx].swizzle[2] = PIPE_SWIZZLE_Z;
+      key->tex[tex_idx].swizzle[3] = PIPE_SWIZZLE_W;
+   }
+
+   key->num_samplers_used = sampler_map->num_desc;
+   assert(key->num_samplers_used <= V3D_MAX_TEXTURE_SAMPLERS);
+   for (uint32_t sampler_idx = 0; sampler_idx < sampler_map->num_desc;
+        sampler_idx++) {
+      /* FIXME: use RelaxedPrecision here */
+      key->sampler[sampler_idx].return_size =
+         sampler_map->is_shadow[sampler_idx] ? 16 : 32;
+      key->sampler[sampler_idx].return_channels =
+         key->sampler[sampler_idx].return_size == 32 ? 4 : 2;
+   }
+
+
 
    /* default value. Would be override on the vs/gs populate methods when GS
     * gets supported
@@ -1723,6 +1674,15 @@ pipeline_lower_nir(struct v3dv_pipeline *pipeline,
 {
    nir_shader_gather_info(p_stage->nir, nir_shader_get_entrypoint(p_stage->nir));
 
+   /* We add this because we need a valid sampler for nir_lower_tex to do
+    * unpacking of the texture operation result, even for the case where there
+    * is no sampler state.
+    */
+   unsigned index =
+      descriptor_map_add(&pipeline->sampler_map,
+                         -1, -1, -1, 0, false);
+   assert(index == V3DV_NO_SAMPLER_IDX);
+
    /* Apply the actual pipeline layout to UBOs, SSBOs, and textures */
    NIR_PASS_V(p_stage->nir, lower_pipeline_layout_info, pipeline, layout);
 }

src/broadcom/vulkan/v3dv_private.h

@@ -1517,7 +1517,14 @@ struct v3dv_sampler {
    uint8_t sampler_state[cl_packet_length(SAMPLER_STATE)];
 };
 
-#define V3DV_NO_SAMPLER_IDX 666
+/* We keep a special value for the sampler idx that represents exactly when a
+ * sampler is not needed/provided. The main use is that even if we don't have
+ * sampler, we still need to do the output unpacking (through
+ * nir_lower_tex). The easier way to do this is to add this special "no
+ * sampler" in the sampler_map, and then use a default unpacking for that
+ * case.
+ */
+#define V3DV_NO_SAMPLER_IDX 0
 
 /*
  * Following two methods are using on the combined to/from texture/sampler
@@ -1606,21 +1613,6 @@ struct v3dv_pipeline {
    struct v3dv_descriptor_map sampler_map;
    struct v3dv_descriptor_map texture_map;
 
-   /*
-    * Vulkan has separate texture and sampler objects. Previous sampler and
-    * texture map uses a sampler and texture index respectively, that can be
-    * different. But OpenGL combine both (or in other words, they are the
-    * same). The v3d compiler and all the nir lowerings that they use were
-    * written under that assumption. In order to not update all those, we
-    * combine the indexes, and we use the following maps to get one or the
-    * other. In general the driver side uses the tex/sampler indexes to gather
-    * resources, and the compiler side uses the combined index (so the v3d key
-    * texture info will be indexed using the combined index).
-    */
-   struct hash_table *combined_index_map;
-   uint32_t combined_index_to_key_map[32];
-   uint32_t next_combined_index;
-
    /* FIXME: this bo is another candidate to data to be uploaded using a
     * resource manager, instead of a individual bo
     */

src/broadcom/vulkan/v3dv_uniforms.c

@@ -89,16 +89,11 @@ write_tmu_p0(struct v3dv_cmd_buffer *cmd_buffer,
              struct v3dv_cl_out **uniforms,
              uint32_t data)
 {
-   int unit = v3d_unit_data_get_unit(data);
+   uint32_t texture_idx = v3d_unit_data_get_unit(data);
-   uint32_t texture_idx;
    struct v3dv_job *job = cmd_buffer->state.job;
    struct v3dv_descriptor_state *descriptor_state =
       &cmd_buffer->state.descriptor_state[v3dv_pipeline_get_binding_point(pipeline)];
 
-   v3dv_pipeline_combined_index_key_unpack(pipeline->combined_index_to_key_map[unit],
-                                           &texture_idx,
-                                           NULL);
-
    /* We need to ensure that the texture bo is added to the job */
    struct v3dv_bo *texture_bo =
       v3dv_descriptor_map_get_texture_bo(descriptor_state, &pipeline->texture_map,
@@ -125,14 +120,11 @@ write_tmu_p1(struct v3dv_cmd_buffer *cmd_buffer,
              struct v3dv_cl_out **uniforms,
              uint32_t data)
 {
-   uint32_t unit = v3d_unit_data_get_unit(data);
+   uint32_t sampler_idx = v3d_unit_data_get_unit(data);
-   uint32_t sampler_idx;
    struct v3dv_job *job = cmd_buffer->state.job;
    struct v3dv_descriptor_state *descriptor_state =
       &cmd_buffer->state.descriptor_state[v3dv_pipeline_get_binding_point(pipeline)];
 
-   v3dv_pipeline_combined_index_key_unpack(pipeline->combined_index_to_key_map[unit],
-                                           NULL, &sampler_idx);
    assert(sampler_idx != V3DV_NO_SAMPLER_IDX);
 
    struct v3dv_cl_reloc sampler_state_reloc =
@@ -285,15 +277,10 @@ get_texture_size(struct v3dv_cmd_buffer *cmd_buffer,
                  enum quniform_contents contents,
                  uint32_t data)
 {
-   int unit = v3d_unit_data_get_unit(data);
+   uint32_t texture_idx = v3d_unit_data_get_unit(data);
-   uint32_t texture_idx;
    struct v3dv_descriptor_state *descriptor_state =
       &cmd_buffer->state.descriptor_state[v3dv_pipeline_get_binding_point(pipeline)];
 
-   v3dv_pipeline_combined_index_key_unpack(pipeline->combined_index_to_key_map[unit],
-                                           &texture_idx,
-                                           NULL);
-
    struct v3dv_descriptor *descriptor =
       v3dv_descriptor_map_get_descriptor(descriptor_state,
                                          &pipeline->texture_map,