[Mesa-dev] [PATCH] glsl: Skip invariant/precision linker checks for built-in variables.

[Kenneth Graunke]

Brian found a bug with my "inline built-ins immediately" code for shaders
which use ftransform() and declare gl_Position invariant:

https://lists.freedesktop.org/archives/mesa-dev/2016-October/132452.html

Before my patch, things worked due to a specific order of operations:

1. link_intrastage_varyings imported the ftransform function into the VS
2. cross_validate_uniforms() ran and signed off that everything matched
3. do_common_optimization did both inlining and invariance propagation,
   making the VS/FS versions of gl_ModelViewProjectionMatrix have
   different invariant qualifiers...but after the check in step 2,
   so we never raised an error.

After my patch, ftransform() is inlined right away, and at compile time,
do_common_optimization propagates the invariant qualifier to the
gl_ModelViewProjectionMatrix.  When the linker eventually happens, it
detects the mismatch.

I can't see any good reason to raise a linker error based on qualifiers
we internally applied to built-in variables - it's not the application's
fault.  It's either not a problem, or it's our fault.o

We should probably rework invariance, but this should keep us limping
along for now.  It's definitely a hack.

Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/compiler/glsl/linker.cpp | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

Hi Brian,

I'm on vacation today through Friday, so I likely won't be able to
push this until next week.  If people are okay with my hack, feel free
to push it before I get back :)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 8599590..66f9e76 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1038,12 +1038,28 @@ cross_validate_globals(struct gl_shader_program *prog,
             }
          }
 
-         if (existing->data.invariant != var->data.invariant) {
-            linker_error(prog, "declarations for %s `%s' have "
-                         "mismatching invariant qualifiers\n",
-                         mode_string(var), var->name);
-            return;
+         /* Skip invariant/precise checks for built-in uniforms.
+          * If they're used in an invariant calculation, the invariance
+          * propagation pass might mark these.  But that's not an error
+          * on the programmer's part - it's our problem.  It shouldn't
+          * actually matter anyway, so ignore it.
+          */
+         if (var->get_num_state_slots() == 0) {
+            if (existing->data.invariant != var->data.invariant) {
+               linker_error(prog, "declarations for %s `%s' have "
+                            "mismatching invariant qualifiers\n",
+                            mode_string(var), var->name);
+               return;
+            }
+
+            if (prog->IsES && existing->data.precision != var->data.precision) {
+               linker_error(prog, "declarations for %s `%s` have "
+                            "mismatching precision qualifiers\n",
+                            mode_string(var), var->name);
+               return;
+            }
          }
+
          if (existing->data.centroid != var->data.centroid) {
             linker_error(prog, "declarations for %s `%s' have "
                          "mismatching centroid qualifiers\n",
@@ -1062,13 +1078,6 @@ cross_validate_globals(struct gl_shader_program *prog,
                          mode_string(var), var->name);
             return;
          }
-
-         if (prog->IsES && existing->data.precision != var->data.precision) {
-            linker_error(prog, "declarations for %s `%s` have "
-                         "mismatching precision qualifiers\n",
-                         mode_string(var), var->name);
-            return;
-         }
       } else
          variables->add_variable(var);
    }
-- 
2.10.0