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 should also better deal with arrays of incomplete size, since the slot check will assume it gets packed.
Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu> --- No regressions in Intel's jenkins runs, which I believe hit both piglit and CTS. This is somewhat subtle code, but I think I covered what it was trying to do. src/compiler/glsl/link_varyings.cpp | 50 ++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index e622b3e..49843cc 100644 --- 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))))) { - - *location = ALIGN(*location + 1, 4); - slot_end = *location; - - /* reset the counter and try again */ - j = 0; + 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(slot_end + 1, 4); + 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; -- 2.7.3 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev