[Mesa-dev] [PATCH] spirv/nir: adjust location assignment for the case of arrays of blocks

[Alejandro Piñeiro]

This is needed due how the types get rearranged after the struct
splitting.

So for example, this array of blocks:

  layout(location = 0) out block {
    vec4 v;
    vec3 v2;
  } x[2];

Would be splitted on two nir variables with the following types:
  * vec4 v[2]
  * vec3 v2[2]

So we need to take into account the length of the array to avoid
locations overlaps one with the other.
---


Hi Jason,

again, sending in advance patches, just in case you are working on the
same.

I was able to fix the location overlapping without all those crazy
ideas about lowering array of blocks into individual blocks, by just
adjusting the locations as this patch shows.

FWIW, the resulting locations are equivalent to those that we get with
GLSL IR, that results on a similar splitting.

With this change I got the following working:
   * SPIR-V simple arrays of blocks input/outputs

   * The arrays of blocks inputs/outputs + interpolator qualifiers
     test I mentioned to you last week [1] when run its SPIR-V
     equivalent.

   * SPIR-V xfb tests using arrays of blocks, where the xfb offset are
     assigned to all block members.

   * SPIR-V xfb tests using arrays of blocks, where the xfb offset is
     assigned to just one member, so just that member is captured,
     although as many times as the array length (yes! afaiu by spec
     that needs to work)

So now, the only pending thing is a cleanup and send the series to
review. Specifically, I think that this series can be put on top of
current master instead of the arb_gl_spirv. Will try that and send a
final series this week or early next week.

BR


[1] https://github.com/Igalia/piglit/blob/master/tests/spec/glsl-1.50/execution/interface-block-interpolation-array.shader_test



 src/compiler/spirv/vtn_variables.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c
index a8f2fdfa534..87386cee42f 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -1672,6 +1672,14 @@ add_missing_member_locations(struct vtn_variable *var,
       glsl_get_length(glsl_without_array(var->type->type));
    int location = var->base_location;
 
+   /* To know if it is a interface block we can't ask directly for
+    * var->type->block because on the case of arrays of blocks, block is set
+    * on the array_element.
+    */
+   bool is_array_block = var->var->interface_type != NULL &&
+      glsl_type_is_array(var->type->type);
+   int adjustment = is_array_block ? glsl_get_length(var->type->type) : 1;
+
    for (unsigned i = 0; i < length; i++) {
       /* From the Vulkan spec:
        *
@@ -1702,8 +1710,12 @@ add_missing_member_locations(struct vtn_variable *var,
       const struct glsl_type *member_type =
          glsl_get_struct_field(glsl_without_array(var->type->type), i);
 
+      /* For arrays of interface blocks we can't just add the attribute slots
+       * of a member type due how the splitting would rearrange the types, so
+       * we need to adjust for the array length in that case.
+       */
       location +=
-         glsl_count_attribute_slots(member_type, is_vertex_input);
+         glsl_count_attribute_slots(member_type, is_vertex_input) * adjustment;
    }
 }
 
-- 
2.19.1