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

[Jason Ekstrand]

On Wed, Oct 19, 2016 at 3:44 PM, Ian Romanick <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
> >>>
> >>> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161019/03985a81/attachment.html>