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

[Francisco Jerez]

Ian Romanick <idr at freedesktop.org> writes:

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

I don't recall the details of the conversation Ken is referring to, but
avoiding propagation of invariance into uniform variables doesn't sound
like a satisfactory fix, because you're likely to run into the same
problem when invariance is propagated into a variable of any other kind
in one compilation unit but it's not propagated into the same variable
in another compilation unit.  What's the purpose of this
cross-validation of invariance done in the linker?  It seems bogus to me
since the same variable may get the invariant qualifier propagated into
it in some compilation unit but not others (and AFAIK that's fine
according to the spec because invariance is only guaranteed as long as
the whole computation of the variable happens within the same
compilation unit).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161020/468ca6a0/attachment.sig>