[Mesa-dev] [PATCH] glsl: fix loop analysis of loop terminators

[Dieter Nützel]

Tested-by: Dieter Nützel <Dieter at nuetzel-hh.de>

Dieter

Am 04.09.2017 05:29, schrieb Timothy Arceri:
> This code incorrectly assumed that loop terminators will always be
> at the start of the loop. Fortunately we *seem* to avoid any bugs
> because the unrolling code loops over and correctly handles the
> terminators.
> 
> However the incorrect analysis can result in loops not being
> unrolled at all. For example the current code would unroll:
> 
>   int j = 0;
>   do {
>      if (j > 5)
>         break;
> 
>      ... do stuff ...
> 
>      j++;
>   } while (j < 4);
> 
> But would fail to unroll the following as no iteration limit was
> calculated because it failed to find the terminator:
> 
>   int j = 0;
>   do {
>      ... do stuff ...
> 
>      j++;
>   } while (j < 4);
> 
> Also we would fail to unroll the following as we ended up
> calculating the iteration limit as 6 rather than 4. The unroll
> code then assumed we had 3 terminators rather the 2 as it
> wasn't able to determine that "if (j > 5)" was redundant.
> 
>   int j = 0;
>   do {
>      if (j > 5)
>         break;
> 
>      ... do stuff ...
> 
>      if (bool(i))
>         break;
> 
>      j++;
>   } while (j < 4);
> ---
>  src/compiler/glsl/loop_analysis.cpp | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/src/compiler/glsl/loop_analysis.cpp
> b/src/compiler/glsl/loop_analysis.cpp
> index b9bae43536..253a405dfb 100644
> --- a/src/compiler/glsl/loop_analysis.cpp
> +++ b/src/compiler/glsl/loop_analysis.cpp
> @@ -290,22 +290,20 @@ loop_analysis::visit_leave(ir_loop *ir)
>     foreach_in_list(ir_instruction, node, &ir->body_instructions) {
>        /* Skip over declarations at the start of a loop.
>         */
>        if (node->as_variable())
>  	 continue;
> 
>        ir_if *if_stmt = ((ir_instruction *) node)->as_if();
> 
>        if ((if_stmt != NULL) && is_loop_terminator(if_stmt))
>  	 ls->insert(if_stmt);
> -      else
> -	 break;
>     }
> 
> 
>     foreach_in_list_safe(loop_variable, lv, &ls->variables) {
>        /* Move variables that are already marked as being loop constant 
> to
>         * a separate list.  These trivially don't need to be tested.
>         */
>        if (lv->is_loop_constant()) {
>  	 lv->remove();
>  	 ls->constants.push_tail(lv);