glsl/linker: fix location aliasing checks for interface variables

The existing code was checking the whole interface variable rather
than its members, which is not what we want: we want to check
aliasing for each member in the interface variable.

Surprisingly, there are piglit tests that verify this and were
passing due to a bug in the existing code: when we were computing
the last component used by an interface variable we would use
the 'vector' path and multiply by vector_elements, which is 0 for
interface variables. This made the loop that checks for aliasing
be a no-op and not add the interface variable to the list of outputs
so then we would fail to link when we did not see a matching output
for the same input in the next stage. Since the tests expect a
linker error to happen, they would pass, but not for the right
reason.

Unfortunately, the current implementation uses ir_variable instances
to keep track of explicit locations. Since we don't have
ir_variables instances for individual interface members, we need
to have a custom struct with the data we need. This struct has
the ir_variable (which for interface members is the whole
interface variable), plus the data that we need to validate for
each aliased location, for now only the base type, which for
interface members we will take from the appropriate field inside
the interface variable.

Later patches will expand this custom struct so we can also check
other requirements for location aliasing, specifically that
we have matching interpolation and auxiliary storage, that once
again, we will take from the appropriate field members for the
interface variables.

v2:
 - Use MAX_VARYING instead of MAX_VARYINGS_INCL_PATCH (Illia)

Fixes:
KHR-GL45.enhanced_layouts.varying_block_automatic_member_locations

Fixes (these were passing before but for incorrect reasons):
tests/spec/arb_enhanced_layouts/linker/block-member-locations/named-block-member-location-overlap.shader_test
tests/spec/arb_enhanced_layouts/linker/block-member-locations/named-block-member-mixed-order-overlap.shader_test

Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>

diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index d394488ab91a05c5fd6f774e687451410ffe4e22..1db851b7e9d4780ef7263cfcca208433cf7eef1b 100644 (file)
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -403,8 +403,13 @@ compute_variable_location_slot(ir_variable *var, gl_shader_stage stage)
    return var->data.location - location_start;
 }
 
+struct explicit_location_info {
+   ir_variable *var;
+   unsigned base_type;
+};
+
 static bool
-check_location_aliasing(ir_variable *explicit_locations[][4],
+check_location_aliasing(struct explicit_location_info explicit_locations[][4],
                         ir_variable *var,
                         unsigned location,
                         unsigned component,
@@ -427,7 +432,7 @@ check_location_aliasing(ir_variable *explicit_locations[][4],
    while (location < location_limit) {
       unsigned i = component;
       while (i < last_comp) {
-         if (explicit_locations[location][i] != NULL) {
+         if (explicit_locations[location][i].var != NULL) {
             linker_error(prog,
                          "%s shader has multiple outputs explicitly "
                          "assigned to location %d and component %d\n",
@@ -439,9 +444,9 @@ check_location_aliasing(ir_variable *explicit_locations[][4],
          /* Make sure all component at this location have the same type.
           */
          for (unsigned j = 0; j < 4; j++) {
-            if (explicit_locations[location][j] &&
-                (explicit_locations[location][j]->type->without_array()
-                 ->base_type != type->without_array()->base_type)) {
+            if (explicit_locations[location][j].var &&
+                explicit_locations[location][j].base_type !=
+                  type->without_array()->base_type) {
                linker_error(prog,
                             "Varyings sharing the same location must "
                             "have the same underlying numerical type. "
@@ -450,7 +455,9 @@ check_location_aliasing(ir_variable *explicit_locations[][4],
             }
          }
 
-         explicit_locations[location][i] = var;
+         explicit_locations[location][i].var = var;
+         explicit_locations[location][i].base_type =
+            type->without_array()->base_type;
          i++;
 
          /* We need to do some special handling for doubles as dvec3 and
@@ -482,8 +489,7 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx,
                                  gl_linked_shader *consumer)
 {
    glsl_symbol_table parameters;
-   ir_variable *explicit_locations[MAX_VARYINGS_INCL_PATCH][4] =
-      { {NULL, NULL} };
+   struct explicit_location_info explicit_locations[MAX_VARYING][4] = { 0 };
 
    /* Find all shader outputs in the "producer" stage.
     */
@@ -514,9 +520,24 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx,
             return;
          }
 
-         if (!check_location_aliasing(explicit_locations, var, idx,
-                                      var->data.location_frac, slot_limit,
-                                      type, prog, producer->Stage)) {
+         if (type->without_array()->is_interface()) {
+            for (unsigned i = 0; i < type->without_array()->length; i++) {
+               const glsl_type *field_type = type->fields.structure[i].type;
+               unsigned field_location = type->fields.structure[i].location -
+                  (type->fields.structure[i].patch ? VARYING_SLOT_PATCH0 :
+                                                     VARYING_SLOT_VAR0);
+               if (!check_location_aliasing(explicit_locations, var,
+                                            field_location,
+                                            0, field_location + 1,
+                                            field_type, prog,
+                                            producer->Stage)) {
+                  return;
+               }
+            }
+         } else if (!check_location_aliasing(explicit_locations, var,
+                                             idx, var->data.location_frac,
+                                             slot_limit, type, prog,
+                                             producer->Stage)) {
             return;
          }
       }
@@ -581,7 +602,7 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx,
                   return;
                }
 
-               output = explicit_locations[idx][input->data.location_frac];
+               output = explicit_locations[idx][input->data.location_frac].var;
 
                if (output == NULL ||
                    input->data.location != output->data.location) {