glsl: fix slot_end calculations and simplify reserved_slots check

The previous code was confused about whether slot_end was inclusive or
exclusive. Make it so that it is inclusive consistently, and use it for
setting the new location. This also avoids discrepancies in how
num_components is calculated vs the more manual approach taken for the
former reserved_slots check.

Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>

diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index e622b3e..43204a7 100644 (file)
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -1546,14 +1546,16 @@ varying_matches::assign_locations(struct gl_shader_program *prog,
 
       previous_var_xfb_only = var->data.is_xfb_only;
 
-      unsigned num_elements =  type->count_attribute_slots(is_vertex_input);
-      unsigned slot_end;
-      if (this->disable_varying_packing &&
-          !is_varying_packing_safe(type, var))
-         slot_end = 4;
-      else
-         slot_end = type->without_array()->vector_elements;
-      slot_end += *location - 1;
+      /* The number of components taken up by this variable. For vertex shader
+       * inputs, we use the number of slots * 4, as they have different
+       * counting rules.
+       */
+      unsigned num_components = is_vertex_input ?
+         type->count_attribute_slots(is_vertex_input) * 4 :
+         this->matches[i].num_components;
+
+      /* The last slot for this variable, inclusive. */
+      unsigned slot_end = *location + num_components - 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
@@ -1561,29 +1563,21 @@ varying_matches::assign_locations(struct gl_shader_program *prog,
        * 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 ((slot_end < MAX_VARYING * 4u) &&
-                ((reserved_slots & (UINT64_C(1) << *location / 4u) ||
-                 (reserved_slots & (UINT64_C(1) << slot_end / 4u))))) {
+      while (slot_end < MAX_VARYING * 4u) {
+         const unsigned slots = (slot_end / 4u) - (*location / 4u) + 1;
+         const uint64_t slot_mask = ((1ull << slots) - 1) << (*location / 4u);
 
+         assert(slots > 0);
+         if (reserved_slots & slot_mask) {
             *location = ALIGN(*location + 1, 4);
-            slot_end = *location;
-
-            /* reset the counter and try again */
-            j = 0;
+            slot_end = *location + num_components - 1;
+            continue;
          }
 
-         /* Increase the slot to make sure there is enough room for next
-          * array element.
-          */
-         if (this->disable_varying_packing &&
-             !is_varying_packing_safe(type, var))
-            slot_end += 4;
-         else
-            slot_end += type->without_array()->vector_elements;
+         break;
       }
 
-      if (!var->data.patch && *location >= MAX_VARYING * 4u) {
+      if (!var->data.patch && slot_end >= MAX_VARYING * 4u) {
          linker_error(prog, "insufficient contiguous locations available for "
                       "%s it is possible an array or struct could not be "
                       "packed between varyings with explicit locations. Try "
@@ -1593,7 +1587,7 @@ varying_matches::assign_locations(struct gl_shader_program *prog,
 
       this->matches[i].generic_location = *location;
 
-      *location += this->matches[i].num_components;
+      *location = slot_end + 1;
    }
 
    return (generic_location + 3) / 4;