<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Wed, Oct 19, 2016 at 3:44 PM, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">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: >>> >>> <a href="https://lists.freedesktop.org/archives/mesa-dev/2016-October/132452.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/archives/mesa-dev/2016-October/132452.html</a> >>> >>> 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. </div></div>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. </blockquote><div> </div><div>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. </div></div> </div></div>