On Fri, Oct 12, 2018, at 5:04 AM, Tapani Pälli wrote:
> 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 <tapani.pa...@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106833
> ---
> 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 2f4c8860547..905d4b3cbed 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -2709,7 +2709,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
> @@ -3072,6 +3073,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 (i == MESA_SHADER_VERTEX) {
Can we please add a GLES check to the if above so we can skip this for desktop
GL.
Its a little unfortunate we need this patch but splitting
assign_attribute_or_color_locations() in two would get messy so with the above
change.
Reviewed-by: Timothy Arceri <tarc...@itsqueeze.com>
> + 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.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
