On 1 March 2013 02:02, Aras Pranckevicius <a...@unity3d.com> wrote: > Hi, > > 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. If we resolve the implicit write issue, there should be no harm in marking variables as const if there's a dereference of them before the first assignment, because a dereference of a variable before the first assignment is undefined. I'm also not certain I like the idea of disabling the optimization in cases where the assignment is in a dynamic branch or a loop. It misses out, for instance, on the opportunity to optimize this code: uniform int u; void main() { float c; for (int i = 0; i < u; i++) { c = 0.5; ... } /* It should be fine to constant-fold c in the code below, since * it is guaranteed to be == 0.5 or undefined, depending on * the value of u. */ gl_FragColor = vec4(c); } > > > -- > Aras Pranckevičius > work: http://unity3d.com > home: http://aras-p.info > > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev