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

[Ian Romanick]

On 10/19/2016 04:52 PM, Jason Ekstrand wrote:
> On Wed, Oct 19, 2016 at 3:44 PM, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
> 
>     On 10/19/2016 02:31 PM, Kenneth Graunke wrote:
>     > On Wednesday, October 19, 2016 1:40:39 PM PDT 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
>     <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 agree.  Propagating invariant/precise to ir_variables used in
>     > expressions is pretty bogus.  We should really track this with an
>     > ir_expression::exact flag similar to NIR's "exact" flag on ALU
>     > expressions.  I don't recall why it was done this way.
>     >
>     > I've hit problems with invariant on uniforms before.  I tried not
>     > propagating it to uniforms back in the day and Curro convinced me
>     > it was wrong and would break things.
>     >
>     > This is a hack - it fixes some cases, but won't fix them all.
>     > Not propagating to uniforms will fix some cases, but I think it
>     > breaks others.  I don't think we have tests for those cases.
> 
>     Right... I think the problem case would be expression trees that involve
>     only uniforms wouldn't necessarily get the invariant treatment.  If one
>     shader had
> 
>         uniform mat4 u0;
>         uniform mat4 u1;
>         invariant out vec4 o;
>         in vec4 i;
> 
>         void main()
>         {
>             o = u0 * u1 * i;
>         }
> 
>     and the other shader had
> 
>         uniform mat4 u0;
>         uniform mat4 u1;
>         invariant out vec4 o0;
>         out vec4 o1;
>         in vec4 i;
> 
>         void main()
>         {
>             o0 = u0 * u1 * i;
>             o1 = u0 * i;
>         }
> 
>     The 'u0 * u1' part of the invariant expression might get optimized
>     differently in each shader.
> 
> I don't think so.  As soon as o0 gets flagged as invariant, piles of
> stuff gets shut off including otherwise "safe" things such as CSE and
> tree grafting.

Ugh.  Then I think we'll need to have Curro tell us what he thought the
problem was.  I'm not good at these kinds of guessing games.