[Mesa-dev] [PATCH v2] glsl: disallow mixed varying types within a location

[Ilia Mirkin]

The enhanced layouts spec has all kinds of restrictions about what can
and cannot be mixed in a location. Integer/float(/presumably double)
can't occupy a single location, interpolation has to be the same, as
well as auxiliary storage (including patch!).

The implication of this is ... don't specify explicit locations/components
if you want better packing, since the auto-packer doesn't care at all
about most of these restrictions. Sad. (Auto-packer, of course, also
can't always do its magic, e.g. SSO.)

This fixes:

KHR-GL45.enhanced_layouts.varying_location_aliasing_with_mixed_types
KHR-GL45.enhanced_layouts.varying_location_aliasing_with_mixed_interpolation
KHR-GL45.enhanced_layouts.varying_location_aliasing_with_mixed_auxiliary_storage

See https://github.com/KhronosGroup/OpenGL-API/issues/13 for some more
info, where I asked about patch vs non-patch locations.

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---

v1 -> v2: move to link_varyings.cpp, per tarceri

 src/compiler/glsl/link_varyings.cpp | 90 +++++++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index 528506fd0eb..0aab083010e 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -2438,6 +2438,87 @@ check_against_input_limit(struct gl_context *ctx,
    return true;
 }
 
+static void
+check_location_mixing(struct gl_shader_program *prog,
+                      struct gl_linked_shader *shader)
+{
+   struct data_type {
+      // 0: unused, 1: float, 2: int, 3: 64-bit
+      unsigned var_type:2;
+      unsigned interpolation:2;
+      bool centroid:1;
+      bool sample:1;
+      bool patch:1;
+   } data_types[2][MAX_VARYING] = {};
+
+   foreach_in_list(ir_instruction, node, shader->ir) {
+      ir_variable *var = node->as_variable();
+      if (!var || !var->data.explicit_location)
+         continue;
+
+      if (var->data.mode != ir_var_shader_in &&
+          var->data.mode != ir_var_shader_out)
+         continue;
+
+      bool output = var->data.mode == ir_var_shader_out;
+      int var_slot;
+      if (!output && shader->Stage == MESA_SHADER_VERTEX) {
+         var_slot = var->data.location - VERT_ATTRIB_GENERIC0;
+         if (var_slot >= VERT_ATTRIB_GENERIC_MAX)
+            continue;
+      } else if (var->data.patch) {
+         var_slot = var->data.location - VARYING_SLOT_PATCH0;
+         if (var_slot >= MAX_VARYING)
+            continue;
+      } else {
+         var_slot = var->data.location - VARYING_SLOT_VAR0;
+         if (var_slot >= MAX_VARYING)
+            continue;
+      }
+
+      if (var_slot < 0)
+         continue;
+
+      const glsl_type *type = get_varying_type(var, shader->Stage);
+      const glsl_type *type_without_array = type->without_array();
+      unsigned num_elements = type->count_attribute_slots(false);
+      unsigned var_type;
+      if (glsl_base_type_is_64bit(type_without_array->base_type))
+         var_type = 3;
+      else if (glsl_base_type_is_integer(type_without_array->base_type))
+         var_type = 2;
+      else
+         var_type = 1;
+
+      const char *var_dir = output ? "out" : "in";
+
+      for (unsigned i = 0; i < num_elements; i++, var_slot++) {
+         data_type& existing = data_types[output][var_slot];
+
+         if (existing.var_type) {
+            if (existing.var_type != var_type)
+               linker_error(prog, "Mismatching %s varying types at location=%d",
+                            var_dir, var_slot);
+            if (existing.interpolation != var->data.interpolation)
+               linker_error(prog, "Mismatching %s varying interpolation at location=%d",
+                            var_dir, var_slot);
+            if (existing.centroid != var->data.centroid ||
+                existing.sample != var->data.sample ||
+                existing.patch != var->data.patch)
+               linker_error(prog, "Mismatching %s varying auxiliary storage at location=%d",
+                            var_dir, var_slot);
+         } else {
+            existing.var_type = var_type;
+            existing.interpolation = var->data.interpolation;
+            existing.centroid = var->data.centroid;
+            existing.sample = var->data.sample;
+            existing.patch = var->data.patch;
+         }
+      }
+   }
+}
+
+
 bool
 link_varyings(struct gl_shader_program *prog, unsigned first, unsigned last,
               struct gl_context *ctx, void *mem_ctx)
@@ -2447,6 +2528,15 @@ link_varyings(struct gl_shader_program *prog, unsigned first, unsigned last,
    char **varying_names = NULL;
    tfeedback_decl *tfeedback_decls = NULL;
 
+   /* Ensure that no illegal type/interp/storage mixing exists within
+    * explicitly-specified varying/attribute locations.
+    */
+   for (int i = MESA_SHADER_FRAGMENT - 1; i >= 0; i--)
+      if (prog->_LinkedShaders[i] != NULL)
+         check_location_mixing(prog, prog->_LinkedShaders[i]);
+   if (!prog->data->LinkStatus)
+      return false;
+
    /* From the ARB_enhanced_layouts spec:
     *
     *    "If the shader used to record output variables for transform feedback
-- 
2.13.5