On 7 July 2011 10:53, Kenneth Graunke <[email protected]> wrote: > Furthermore, I think can_inline probably ought to be altered. Right now > it returns true if there's only one "return". But it doesn't seem to be > checking that it's the _canonical_ return. > > Without lower_jumps, I think the following code would be incorrectly > inlined: > > uniform bool b; > float foo() { > if (b) > return 0.0; > gl_FragColor = vec4(1, 0, 0, 1); > return 1.0; > } > > IOW, opt_function_inlining has broken code, but we're not seeing any > ill-effects because we run lower_jumps first, which fixes things. We > should probably make that dependency explicitly clear. >
I just double-checked this, and it turns out that can_inline() _does_ check that the one return is the canonical return, but it does it in a roundabout way: after counting the number of returns in the function, it adds one if the function doesn't end in a return instruction (to account for the "implicit return"). So if a function contains a non-canonical return, it gets counted as having two returns, and the function won't be inlined. The bug you were worried about doesn't exist. Of course, failing to inline a function is bad too, but fortunately, in all code paths, do_lower_jumps() is called before opt_function_inlining(), so it's not an issue. I'm not sure if that was intentional or just a coincidence--the order of optimizations seems a little ad-hoc :) _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
