[Mesa-dev] [PATCH 08/26] spirv/nir: Handle location decorations on block interface members

[Alejandro Piñeiro]

From: Neil Roberts <nroberts at igalia.com>

Previously the code was taking any location decoration on the block
and using that to calculate the member locations for all of the
members. I think this was assuming that there would only be one
location decoration for the entire block. According to the Vulkan spec
it is possible to add location decorations to individual members:

   “If the structure type is a Block but without a Location, then each
    of its members must have a Location decoration. If it is a Block
    with a Location decoration, then its members are assigned
    consecutive locations in declaration order, starting from the
    first member which is initially the Block. Any member with its own
    Location decoration is assigned that location. Each remaining
    member is assigned the location after the immediately preceding
    member in declaration order.”

This patch makes it instead keep track of which members have been
assigned an explicit location. It also has a space to store the
location for the struct as a whole. Once all the decorations have been
processed it iterates over each member to fill in the missing
locations using the rules described above.

v2: update after commit b0c643d, where spirv_to_nir stopped to do
    struct member splitting, done it later in NIR (Alejandro Piñeiro)

Signed-off-by: Neil Roberts <nroberts at igalia.com>
Signed-off-by: Alejandro Piñeiro <apinheiro at igalia.com>
---
 src/compiler/spirv/vtn_private.h   |  6 ++++
 src/compiler/spirv/vtn_variables.c | 62 ++++++++++++++++++++++++++++++++------
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h
index b5199bda633..5e6d2888576 100644
--- a/src/compiler/spirv/vtn_private.h
+++ b/src/compiler/spirv/vtn_private.h
@@ -471,6 +471,12 @@ struct vtn_variable {
 
    nir_variable *var;
 
+   /* If the variable is a struct with a location set on it then this will be
+    * stored here. This will be used to calculate locations for members that
+    * don’t have their own explicit location.
+    */
+   int base_location;
+
    int shared_location;
 
    /**
diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c
index 7de02b91efc..30fb15810ec 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -1406,13 +1406,11 @@ var_decoration_cb(struct vtn_builder *b, struct vtn_value *val, int member,
     */
    if (dec->decoration == SpvDecorationLocation) {
       unsigned location = dec->literals[0];
-      bool is_vertex_input = false;
       if (b->shader->info.stage == MESA_SHADER_FRAGMENT &&
           vtn_var->mode == vtn_variable_mode_output) {
          location += FRAG_RESULT_DATA0;
       } else if (b->shader->info.stage == MESA_SHADER_VERTEX &&
                  vtn_var->mode == vtn_variable_mode_input) {
-         is_vertex_input = true;
          location += VERT_ATTRIB_GENERIC0;
       } else if (vtn_var->mode == vtn_variable_mode_input ||
                  vtn_var->mode == vtn_variable_mode_output) {
@@ -1429,14 +1427,13 @@ var_decoration_cb(struct vtn_builder *b, struct vtn_value *val, int member,
       } else {
          /* This handles the structure member case */
          assert(vtn_var->var->members);
-         for (unsigned i = 0; i < vtn_var->var->num_members; i++) {
-            vtn_var->var->members[i].location = location;
-            const struct glsl_type *member_type =
-               glsl_get_struct_field(vtn_var->var->interface_type, i);
-            location += glsl_count_attribute_slots(member_type,
-                                                   is_vertex_input);
-         }
+
+         if (member == -1)
+            vtn_var->base_location = location;
+         else
+            vtn_var->var->members[member].location = location;
       }
+
       return;
    } else {
       if (vtn_var->var) {
@@ -1629,6 +1626,43 @@ is_per_vertex_inout(const struct vtn_variable *var, gl_shader_stage stage)
    return false;
 }
 
+static void
+add_missing_member_locations(struct vtn_variable *var,
+                             bool is_vertex_input)
+{
+   unsigned length =
+      glsl_get_length(glsl_without_array(var->type->type));
+   int location = var->base_location;
+
+   for (unsigned i = 0; i < length; i++) {
+      /* From the Vulkan spec:
+       *
+       * “If the structure type is a Block but without a Location, then each
+       *  of its members must have a Location decoration.”
+       */
+      assert(var->base_location != -1 ||
+             var->var->members[i].location != -1);
+
+      /* From the Vulkan spec:
+       *
+       * “Any member with its own Location decoration is assigned that
+       *  location. Each remaining member is assigned the location after the
+       *  immediately preceding member in declaration order.”
+       */
+      if (var->var->members[i].location != -1)
+         location = var->var->members[i].location;
+      else
+         var->var->members[i].location = location;
+
+      const struct glsl_type *member_type =
+         glsl_get_struct_field(var->var->interface_type, i);
+
+      location +=
+         glsl_count_attribute_slots(member_type, is_vertex_input);
+   }
+}
+
+
 static void
 vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
                     struct vtn_type *ptr_type, SpvStorageClass storage_class,
@@ -1669,6 +1703,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
    struct vtn_variable *var = rzalloc(b, struct vtn_variable);
    var->type = type;
    var->mode = mode;
+   var->base_location = -1;
 
    vtn_assert(val->value_type == vtn_value_type_pointer);
    val->pointer = vtn_pointer_for_variable(b, var, ptr_type);
@@ -1780,6 +1815,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
          for (unsigned i = 0; i < var->var->num_members; i++) {
             var->var->members[i].mode = nir_mode;
             var->var->members[i].patch = var->patch;
+            var->var->members[i].location = -1;
          }
       } else {
          var->var->interface_type = NULL;
@@ -1806,6 +1842,14 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
 
    vtn_foreach_decoration(b, val, var_decoration_cb, var);
 
+   if ((var->mode == vtn_variable_mode_input ||
+        var->mode == vtn_variable_mode_output) &&
+       var->var->members) {
+      bool is_vertex_input = (b->shader->info.stage == MESA_SHADER_VERTEX &&
+                              var->mode == vtn_variable_mode_input);
+      add_missing_member_locations(var, is_vertex_input);
+   }
+
    if (var->mode == vtn_variable_mode_uniform ||
        var->mode == vtn_variable_mode_ubo ||
        var->mode == vtn_variable_mode_ssbo) {
-- 
2.14.1