[Mesa-dev] [PATCH 6/6] glsl: fix overlapping of varying locations for arrays

[Timothy Arceri]

Previously we were not reserving the full array for explicit locations.

We also didn't take into account implicit locations clashing with
explicit locations when assigning locations for their arrays.

This patch fixes both issues.

There is no effort to make arrays of arrays work here because we should
just add a lowing pass for inputs and outputs that turns
arrays of arrays into single dimension arrays with consecutive locations
which would allow this change to work with arrays of array also.

Cc: Gregory Hainaut <gregory.hainaut at gmail.com>
---
 src/glsl/link_varyings.cpp | 68 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 60 insertions(+), 8 deletions(-)

diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 71750d1..44909c2 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -766,7 +766,8 @@ public:
                    gl_shader_stage consumer_stage);
    ~varying_matches();
    void record(ir_variable *producer_var, ir_variable *consumer_var);
-   unsigned assign_locations(uint64_t reserved_slots, bool separate_shader);
+   unsigned assign_locations(struct gl_shader_program *prog,
+                             uint64_t reserved_slots, bool separate_shader);
    void store_locations() const;
 
 private:
@@ -988,7 +989,9 @@ varying_matches::record(ir_variable *producer_var, ir_variable *consumer_var)
  * passed to varying_matches::record().
  */
 unsigned
-varying_matches::assign_locations(uint64_t reserved_slots, bool separate_shader)
+varying_matches::assign_locations(struct gl_shader_program *prog,
+                                  uint64_t reserved_slots,
+                                  bool separate_shader)
 {
    /* We disable varying sorting for separate shader programs for the
     * following reasons:
@@ -1040,9 +1043,53 @@ varying_matches::assign_locations(uint64_t reserved_slots, bool separate_shader)
           != this->matches[i].packing_class) {
          *location = ALIGN(*location, 4);
       }
-      while ((*location < MAX_VARYING * 4u) &&
-            (reserved_slots & (1u << *location / 4u))) {
-         *location = ALIGN(*location + 1, 4);
+
+      const ir_variable *var =
+         matches[i].consumer_var ? matches[i].consumer_var :
+            matches[i].producer_var;
+
+      unsigned num_elements =
+         var->type->is_array() ? var->type->length : 1;
+
+      /* This will hold the location of the last component in the varying */
+      unsigned last_component = this->disable_varying_packing ? 4 :
+         var->type->without_array()->vector_elements;
+      last_component += *location - 1;
+
+      /* FIXME: We could be smarter in the below code and loop back over
+       * trying to fill any locations that we skipped because we couldn't pack
+       * the varying between an explicit location. For now just let the user
+       * hit the linking error if we run out of room and suggest they use
+       * explicit locations.
+       */
+      for (unsigned j = 0; j < num_elements; j++) {
+         while ((last_component < MAX_VARYING * 4u) &&
+                ((reserved_slots & (1u << *location / 4u) ||
+                 (reserved_slots & (1u << last_component / 4u))))) {
+
+            *location = ALIGN(*location + 1, 4);
+            last_component = *location - 1;
+
+            /* reset the counter and try again */
+            j = 0;
+         }
+
+         /* Increase the last component location to make sure there is enough
+          * room for next array element, or if we are trying a new location
+          * we are bumping this to the last component.
+          */
+         if (this->disable_varying_packing)
+            last_component += 4;
+         else
+            last_component += var->type->without_array()->vector_elements;
+      }
+
+      if (*location >= MAX_VARYING * 4u) {
+         linker_error(prog, "insufficient contiguous locations available for "
+                      "%s it is likely an array could not be packed between "
+                      "varyings with explicit locations. Try using an "
+                      "explicit location for arrays.",
+                      var->name);
       }
 
       this->matches[i].generic_location = *location;
@@ -1430,8 +1477,13 @@ reserved_varying_slot(struct gl_shader *stage, ir_variable_mode io_mode)
          continue;
 
       var_slot = var->data.location - VARYING_SLOT_VAR0;
-      if (var_slot >= 0 && var_slot < MAX_VARYING)
-         slots |= 1u << var_slot;
+      unsigned num_elements =
+         var->type->is_array() ? var->type->length : 1;
+      for (unsigned i = 0; i < num_elements; i++) {
+         if (var_slot >= 0 && var_slot < MAX_VARYING)
+            slots |= 1u << var_slot;
+         var_slot += 1;
+      }
    }
 
    return slots;
@@ -1617,7 +1669,7 @@ assign_varying_locations(struct gl_context *ctx,
       reserved_varying_slot(producer, ir_var_shader_out) |
       reserved_varying_slot(consumer, ir_var_shader_in);
 
-   const unsigned slots_used = matches.assign_locations(reserved_slots,
+   const unsigned slots_used = matches.assign_locations(prog, reserved_slots,
                                                         prog->SeparateShader);
    matches.store_locations();
 
-- 
2.4.3