From 27f1298b9d95899c4061294e384fadfd1c0a1b44 Mon Sep 17 00:00:00 2001 From: =?utf8?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=utf8 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.x specifications. We need to run this pass before unused attributes may be removed and with attribute binding information from program, therefore we re-use existing pass in linker rather than attempt to write another one. This fixes WebGL2 test 'gl-bindAttribLocation-aliasing-inactive' and Piglit test 'gles-3.0-attribute-aliasing'. Signed-off-by: Tapani Pälli Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106833 Reviewed-by: Timothy Arceri --- src/compiler/glsl/linker.cpp | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 8b1b03322a0..17fe0a58448 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -2710,7 +2710,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 @@ -3073,6 +3074,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 = util_bitcount(used_locations & SAFE_MASK_FROM_INDEX(max_index)) + @@ -4780,12 +4784,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; } @@ -5162,6 +5166,27 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) lower_tess_level(prog->_LinkedShaders[i]); } + /* Section 13.46 (Vertex Attribute Aliasing) of the OpenGL ES 3.2 + * specification says: + * + * "In general, the behavior of GLSL ES should not depend on compiler + * optimizations which might be implementation-dependent. Name matching + * rules in most languages, including C++ from which GLSL ES is derived, + * are based on declarations rather than use. + * + * RESOLUTION: The existence of aliasing is determined by declarations + * present after preprocessing." + * + * Because of this rule, we do a 'dry-run' of attribute assignment for + * vertex shader inputs here. + */ + if (prog->IsES && 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.30.2