On 7 July 2011 10:53, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On 07/07/2011 09:56 AM, Paul Berry wrote:
>> On 7 July 2011 01:16, Kenneth Graunke <kenn...@whitecape.org> wrote:
>>> What exactly is the difference between lowering returns in "main" and
>>> lowering them in other subroutines? void vs. non-void is definitely
>>> different, but I don't see why main is special. Looking at the code, it
>>> doesn't actually seem to use lower_main_return anywhere...
>>
>> There's no difference in how returns are lowered in "main" and in
>> subroutines. The difference is in whether or not we lower returns.
>> For instance, when _mesa_ir_link_shader() calls do_lower_jumps(), it
>> always asks it to lower returns in subroutines (presumably to
>> facilitate inlining), but it only asks it to lower returns in main if
>> options->EmitNoMainReturn is true.
>
> You're right of course, sorry...I somehow missed that
> lower_main_return/lower_sub_return were options to the lowering pass.
>
> I didn't realize that lower_jumps was needed for inlining (though it
> sure makes sense). opt_function_inlining.cpp has its own code for
> this---the "replace_return_with_assignment" function. Looking at that
> again, it's fairly limited...it doesn't do any guarding in the case of
> conditional returns...so it's really only useful for replacing the
> canonical jump at the end of a non-void function.
>
> Yet, we're using visit_tree with replace_return_with_assignment. I
> believe that this means we're actually walking over the whole IR tree
> for the function (including all the expression trees!) looking for
> returns. That seems wasteful---if we're using lower_jumps, we know
> exactly where it is...it's the last instruction.
>
> 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.
>
That's a good point. I'll make a note to have a look at that when I'm
finished working on lower_jumps.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
