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

[Marek Olšák]

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Mon, Sep 4, 2017 at 5:29 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
> 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);
> --
> 2.13.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev