[Mesa-stable] [Mesa-dev] [PATCH] glsl: fix lower jumps for returns when loop is inside an if

[Roland Scheidegger]

Am 26.03.2017 um 14:34 schrieb Timothy Arceri:
> Previously we would just escape the loop and move everything
> following the loop inside the if to the else branch of a new if
> with a return flag conditional. However everything outside the
> if the loop was nested in would still get executed.
> 
> Adding a new return to the then branch of the new if fixes this
> and we just let a follow pass clean it up if needed.
> 
> Fixes:
> tests/spec/glsl-1.10/execution/vs-nested-return-sibling-loop.shader_test
> tests/spec/glsl-1.10/execution/vs-nested-return-sibling-loop2.shader_test
> 
> Cc: "13.0 17.0" <mesa-stable at lists.freedesktop.org>
> ---
> 
>  In order to fix https://bugs.freedesktop.org/show_bug.cgi?id=100303
>  we would need to extend this fix to handle return values.
> 
>  src/compiler/glsl/lower_jumps.cpp | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/src/compiler/glsl/lower_jumps.cpp b/src/compiler/glsl/lower_jumps.cpp
> index ed1b6d1..1b893e5 100644
> --- a/src/compiler/glsl/lower_jumps.cpp
> +++ b/src/compiler/glsl/lower_jumps.cpp
> @@ -925,28 +925,35 @@ lower_continue:
>            * ANALYSIS postcondition.
>            */
>           saved_loop.may_set_return_flag = true;
>           if(saved_loop.loop)
>              /* If this loop is nested inside another one, then the if
>               * statement that we generated should break out of that
>               * loop if the return flag is set.  Caller will lower that
>               * break statement if necessary.
>               */
>              return_if->then_instructions.push_tail(new(ir) ir_loop_jump(ir_loop_jump::jump_break));
> -         else
> -            /* Otherwise, all we need to do is ensure that the
> -             * instructions that follow are only executed if the
> -             * return flag is clear.  We can do that by moving those
> -             * instructions into the else clause of the generated if
> +         else {
> +            /* Otherwise, ensure that the instructions that follow are only
> +             * executed if the return flag is clear.  We can do that by moving
> +             * those instructions into the else clause of the generated if
>               * statement.
>               */
>              move_outer_block_inside(ir, &return_if->else_instructions);
> +
> +            /* In case the loop is embeded inside an if add a new return to
embedded.

Honestly I don't quite follow all the logic in this lowering code. But I
guess it won't make things worse, so
Reviewed-by: Roland Scheidegger <sroland at vmware.com>

> +             * the return flag then branch and let a future pass tidy it up.
> +             */
> +            if (this->function.signature->return_type->is_void())
> +               return_if->then_instructions.push_tail(new(ir) ir_return(NULL));
> +         }
> +
>           ir->insert_after(return_if);
>        }
>  
>        this->loop = saved_loop;
>        --this->function.nesting_depth;
>     }
>  
>     virtual void visit(ir_function_signature *ir)
>     {
>        /* these are not strictly necessary */
>