[Mesa-dev] [PATCH] GLSL: fix too eager constant variable optimization

[Aras Pranckevicius]

>> opt_constant_variable was marking a variable as constant as long as there was
>> exactly one constant assignment to it, but did not take into account that this
>> assignment might be in a dynamic branch or a loop.
>> Was happening on a fragment shader like this:
>>
>> uniform float mode;
>> float func (float c) {
>>     if (mode == 2.0)
>>         return c; // was returning 0.1 after optimization!
>>     if (mode == 3.0)
>>     discard;
>>     if (mode == 10.0)
>>         c = 0.1;
>>     return c;
>> }
>> void main() {
>>     vec4 c = gl_FragCoord;
>>     c.x = func(c.x);
>>     gl_FragColor = c;
>> }
>>
>> Now, looking further this optimization pass should also not mark variables as
>> const if there was a dereference of them before that first assignment. I had
>> code to do this (a hashtable that would track dereferences before assignment
>> is done). But couldn't come up with a test case that would break the whole set
>> of optimizations that Mesa does (lower jumps, or inlining, ... were getting in
>> the way and hide the bug).
>
> I'm not sure I agree with this.  The real problem with the example code you showed
> above is that there's an implicit write to the variable c at the top of the function,
> so c is not in fact constant--it's written twice.  What we should really do is modify
> the optimization pass so that it's aware of the implicit write that happens in "in"
> and "inout" function args.

Attached version two of the patch which does what you suggest - any
ir_var_in, ir_var_const_in or ir_var_inout function args are being
marked as assigned to. Fixes the issue just as well as my initial
patch on several shaders that were problematic before.



--
Aras Pranckevičius
work: http://unity3d.com
home: http://aras-p.info
-------------- next part --------------
A non-text attachment was scrubbed...
Name: glsl-constant-variable-fix-v2.diff
Type: application/octet-stream
Size: 1663 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130303/15067a8e/attachment.obj>