Attachment 140052 Details for Bug 106833 – possible fix

[patch] possible fix

0001-glsl-linker-validate-attribute-aliasing-before-optim.patch (text/plain), 6.14 KB, created by Tapani Pälli on 2018-06-06 11:23:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tapani Pälli

Created: 2018-06-06 11:23:07 UTC

Size: 6.14 KB

patch

obsolete

>From 396c606845dbc66427bfb097144966906d1eee30 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Tapani=20P=C3=A4lli?= <tapani.palli@intel.com>
>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 <tapani.palli@intel.com>
>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
>

Actions: View | Splinter Review

Attachments on bug 106833: 140052 | 141034 | 141035

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.