[Mesa-dev] [PATCH 2/4] glsl/linker: fix location aliasing checks for interface variables
Iago Toral Quiroga
itoral at igalia.com
Thu Oct 19 16:31:37 UTC 2017
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.
Fixes:
KHR-GL45.enhanced_layouts.varying_block_automatic_member_locations
Fixes:
tests/spec/arb_separate_shader_objects/execution/layout-location-named-block.shader_test -auto -fbo
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
---
src/compiler/glsl/link_varyings.cpp | 45 +++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 12 deletions(-)
diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index 687bd50e52..5dc2704321 100644
--- 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,8 @@ 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_VARYINGS_INCL_PATCH][4] =
+ { 0 };
/* Find all shader outputs in the "producer" stage.
*/
@@ -514,9 +521,23 @@ 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 - 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;
}
}
@@ -574,7 +595,7 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx,
unsigned slot_limit = idx + num_elements;
while (idx < slot_limit) {
- 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) {
--
2.11.0
More information about the mesa-dev
mailing list