On 10/19/2016 02:40 PM, Ian Romanick wrote:
On 10/19/2016 11:11 AM, Kenneth Graunke wrote:
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.
Why are we marking a uniform as invariant in the first place? That
sounds boats.
The shader author is marking the gl_Position VS output as invariant and
calling ftransform():
invariant gl_Position;
void main()
{
gl_Position = ftransform();
}
ftransform() expands into gl_ModelviewProjectionMatrix * gl_Vertex.
Then, afaict, the propagation pass marks gl_ModelviewProjectionMatrix as
invariant, but that disagrees with the original declaration of the
matrix and linking fails.
That's my superficial understanding of it.
Do you want me to hold off on pushing the patch? I'd really like to get
this or another fix in place.
-Brian
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 <[email protected]>
---
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);
}
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
