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. > 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
