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

[Paul Berry]

On 1 March 2013 02:02, Aras Pranckevicius <aras at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130301/a223741b/attachment.html>