[Mesa-stable] [Mesa-dev] [PATCH] glsl: Handle empty if statement encountered during loop analysis.

[Kenneth Graunke]

On 07/24/2013 08:17 AM, Paul Berry wrote:
> The is_loop_terminator() function was asserting that the following
> kind of if statement could never occur:
>
>      if (...) { } else { }
>
> (presumably based on the assumption that such an if statement would be
> eliminated by previous optimization stages).  But that isn't the
> case--it's possible that previous optimization stages might simplify
> more complex code down to this empty if statement, in which case it
> won't be eliminated until the next time through the optimization loop.
>
> So is_loop_terminator() needs to handle it.  Fortunately it's easy to
> handle--it's not a loop terminator because it does nothing.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64330
> CC: mesa-stable at lists.freedesktop.org
> ---
>   src/glsl/loop_analysis.cpp | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/loop_analysis.cpp b/src/glsl/loop_analysis.cpp
> index 191e92d..40897bb 100644
> --- a/src/glsl/loop_analysis.cpp
> +++ b/src/glsl/loop_analysis.cpp
> @@ -503,7 +503,8 @@ is_loop_terminator(ir_if *ir)
>
>      ir_instruction *const inst =
>         (ir_instruction *) ir->then_instructions.get_head();
> -   assert(inst != NULL);
> +   if (inst == NULL)
> +      return false;
>
>      if (inst->ir_type != ir_type_loop_jump)
>         return false;
>

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

This would have been a bit more obvious if 'diff' had included a few 
more lines of context.  Immediately above this hunk, there is:

    if (!ir->else_instructions.is_empty())
       return false;

which already guarantees the else block is empty, so it really is just 
the noop "if (...) {} {}" statement.

--Ken