[Mesa-dev] [RFC PATCH 05/13] spirv/nir: fixing the xfb_offset for arrays of structs

[Alejandro Piñeiro]

GLSLang computes the xfb_offset for struct members. In fact, for basic
structs, the xfb nir gathering pass expect those to be filled, as one
struct variable is lowered to several nir variables, and those need to
have the xfb offset already set. See [1].

But, as one existing comments at spirv to nir already points:

  "GLSLang really likes to place decorations in the most interior
   thing it possibly can.  In particular, if you have a struct, it
   will place the patch decorations on the struct members"

And that includes xfb offset. In fact, GLSLang not expose the xfb
offset of the full struct, as it is properly assigned to the members,
and it makes a lot of the internal checks (like offset overlapping)
easier for them. I was not able to find a spec quote saying that is
wrong, as all the individual members has the proper offset.

This affects the case of variables that are array of structs, are they
are exposed as just one nir variable output.

So this commit resets the xfb_offset for the nir_variable if any of
the members has a xfb_offset assigned.

Rant: In general, the rules for xfb offset assignment on the spec are
somewhat underspecified for the new ARB_gl_spirv/vulkan world, as it
is not clear who is the responsible to do that (in opposite to the old
GLSL world, where the answer is "always/everything should solved by
the driver"). Ideally, it would be good if glslang does it, so the
vulkan/opengl driver just need to get the info. Unfourtunately, there
are cases, like arrays of structs where the driver still need to do
the assignment. So perhaps in the end it should be the opposite,
glslang (or any other frontend), just exposing the explicit info from
the user, and let the driver do the individual
assignments. Unfourtunately, with the current spec, there isn't
anything preventing the frontend to do that, so we would need to be
defensive, and cover all aspects.

[1] https://github.com/KhronosGroup/glslang/pull/154
---
 src/compiler/spirv/vtn_variables.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c
index 13e8bf1fc3c..91e351187d2 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -1718,6 +1718,26 @@ add_missing_member_locations(struct vtn_variable *var,
    }
 }
 
+static void
+vtn_fix_struct_array_xfb_offset(nir_variable *var)
+{
+   if (!glsl_type_is_array(var->type))
+      return;
+
+   const struct glsl_type *child_type = glsl_get_array_element(var->type);
+
+   if (!glsl_type_is_struct(child_type))
+      return;
+
+   if (var->data.explicit_offset)
+      return;
+
+   int offset = glsl_get_struct_field_offset(child_type, 0);
+   if (offset != -1) {
+      var->data.explicit_offset = 1;
+      var->data.offset = offset;
+   }
+}
 
 static void
 vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
@@ -1882,6 +1902,8 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
       vtn_foreach_decoration(b, vtn_value(b, interface_type->id,
                                           vtn_value_type_type),
                              var_decoration_cb, var);
+
+      vtn_fix_struct_array_xfb_offset(var->var);
       break;
    }
 
-- 
2.19.1