From 396c606845dbc66427bfb097144966906d1eee30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tapani=20P=C3=A4lli?= Date: Wed, 6 Jun 2018 14:19:16 +0300 Subject: [PATCH] glsl/linker: validate attribute aliasing before optimizations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch does a 'dry run' of assign_attribute_or_color_locations before optimizations to catch cases where we have aliasing of unused attributes which is forbidden by the GLSL ES 3.0 specification. Signed-off-by: Tapani Pälli Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106833 --- src/compiler/glsl/linker.cpp | 48 ++++++++++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index e4bf634abe..54116e7a7f 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -2606,7 +2606,8 @@ static bool assign_attribute_or_color_locations(void *mem_ctx, gl_shader_program *prog, struct gl_constants *constants, - unsigned target_index) + unsigned target_index, + bool do_assignment) { /* Maximum number of generic locations. This corresponds to either the * maximum number of draw buffers or the maximum number of generic @@ -2679,6 +2680,8 @@ assign_attribute_or_color_locations(void *mem_ctx, unsigned num_attr = 0; + int assigned_loc = -1; + foreach_in_list(ir_instruction, node, sh->ir) { ir_variable *const var = node->as_variable(); @@ -2702,8 +2705,11 @@ assign_attribute_or_color_locations(void *mem_ctx, if (prog->AttributeBindings->get(binding, var->name)) { assert(binding >= VERT_ATTRIB_GENERIC0); - var->data.location = binding; - var->data.is_unmatched_generic_inout = 0; + assigned_loc = binding; + if (do_assignment) { + var->data.location = binding; + var->data.is_unmatched_generic_inout = 0; + } } } else if (target_index == MESA_SHADER_FRAGMENT) { unsigned binding; @@ -2715,10 +2721,13 @@ assign_attribute_or_color_locations(void *mem_ctx, /* Check if there's a binding for the variable name */ if (prog->FragDataBindings->get(binding, name)) { assert(binding >= FRAG_RESULT_DATA0); - var->data.location = binding; - var->data.is_unmatched_generic_inout = 0; + assigned_loc = binding; + if (do_assignment) { + var->data.location = binding; + var->data.is_unmatched_generic_inout = 0; + } - if (prog->FragDataIndexBindings->get(index, name)) { + if (do_assignment & prog->FragDataIndexBindings->get(index, name)) { var->data.index = index; } break; @@ -2764,8 +2773,8 @@ assign_attribute_or_color_locations(void *mem_ctx, * that it doesn't collide with other assigned locations. Otherwise, * add it to the list of variables that need linker-assigned locations. */ - if (var->data.location != -1) { - if (var->data.location >= generic_base && var->data.index < 1) { + if (assigned_loc != -1) { + if (assigned_loc >= generic_base && var->data.index < 1) { /* From page 61 of the OpenGL 4.0 spec: * * "LinkProgram will fail if the attribute bindings assigned @@ -2837,7 +2846,7 @@ assign_attribute_or_color_locations(void *mem_ctx, /* Mask representing the contiguous slots that will be used by * this attribute. */ - const unsigned attr = var->data.location - generic_base; + const unsigned attr = assigned_loc - generic_base; const unsigned use_mask = (1 << slots) - 1; const char *const string = (target_index == MESA_SHADER_VERTEX) ? "vertex shader input" : "fragment shader output"; @@ -2969,6 +2978,9 @@ assign_attribute_or_color_locations(void *mem_ctx, num_attr++; } + if (!do_assignment) + return true; + if (target_index == MESA_SHADER_VERTEX) { unsigned total_attribs_size = _mesa_bitcount(used_locations & SAFE_MASK_FROM_INDEX(max_index)) + @@ -3020,8 +3032,10 @@ assign_attribute_or_color_locations(void *mem_ctx, return false; } - to_assign[i].var->data.location = generic_base + location; - to_assign[i].var->data.is_unmatched_generic_inout = 0; + if (do_assignment) { + to_assign[i].var->data.location = generic_base + location; + to_assign[i].var->data.is_unmatched_generic_inout = 0; + } used_locations |= (use_mask << location); if (to_assign[i].var->type->without_array()->is_dual_slot()) @@ -4690,12 +4704,12 @@ link_varyings_and_uniforms(unsigned first, unsigned last, } if (!assign_attribute_or_color_locations(mem_ctx, prog, &ctx->Const, - MESA_SHADER_VERTEX)) { + MESA_SHADER_VERTEX, true)) { return false; } if (!assign_attribute_or_color_locations(mem_ctx, prog, &ctx->Const, - MESA_SHADER_FRAGMENT)) { + MESA_SHADER_FRAGMENT, true)) { return false; } @@ -5063,6 +5077,14 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) lower_tess_level(prog->_LinkedShaders[i]); } + /* Verify attribute aliasing. */ + if (i == MESA_SHADER_VERTEX) { + if (!assign_attribute_or_color_locations(mem_ctx, prog, &ctx->Const, + MESA_SHADER_VERTEX, false)) { + goto done; + } + } + /* Call opts before lowering const arrays to uniforms so we can const * propagate any elements accessed directly. */ -- 2.14.4