[Mesa-dev] [PATCH 3/3] glsl: relax loop unrolling restrictions

[Alejandro Piñeiro]

Perhaps someone would comment on the last patch, but makes sense to me.

The three patch series is:
Reviewed-by: Alejandro Piñeiro <apinheiro at igalia.com>

On 21/06/17 12:12, Timothy Arceri wrote:
> The main reason these restriction exist is because glsl the loop
> unrolling pass is super slow with large loops.
>
> be5f27a84d0d fixed things so that expression trees were counted
> against the limit, however it left the limit as
> max_iterations * 5 which is actually fine for most shaders but
> probably over conservative.
>
> This change relaxes the limit to allow more loops to unroll in the
> Unigine Superposition benchmark.
>
> Results from Unigine Superposition @ 1920x1080 - High - Fullscreen
> On radeonsi (RX480)
>
> Before:
>
> Average: 28.20 Frames Per Second
>
> After:
>
> Average: 28.60 Frames Per Second
> ---
>
>  The increase is small but it seems to be consistent, I would be
>  interested in the results if others were interested in testing it.
>
>  src/compiler/glsl/loop_unroll.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/compiler/glsl/loop_unroll.cpp b/src/compiler/glsl/loop_unroll.cpp
> index bc377df..64ebf0f 100644
> --- a/src/compiler/glsl/loop_unroll.cpp
> +++ b/src/compiler/glsl/loop_unroll.cpp
> @@ -350,21 +350,21 @@ loop_unroll_visitor::visit_leave(ir_loop *ir)
>     /* Don't try to unroll loops that have zillions of iterations either.
>      */
>     if (iterations > max_iterations)
>        return visit_continue;
>  
>     /* Don't try to unroll nested loops and loops with a huge body.
>      */
>     loop_unroll_count count(&ir->body_instructions, ls, options);
>  
>     bool loop_too_large =
> -      count.nested_loop || count.nodes * iterations > max_iterations * 5;
> +      count.nested_loop || count.nodes * iterations > max_iterations * 10;
>  
>     if (loop_too_large && !count.unsupported_variable_indexing &&
>         !count.array_indexed_by_induction_var_with_exact_iterations)
>        return visit_continue;
>  
>     /* Note: the limiting terminator contributes 1 to ls->num_loop_jumps.
>      * We'll be removing the limiting terminator before we unroll.
>      */
>     assert(ls->num_loop_jumps > 0);
>     unsigned predicted_num_loop_jumps = ls->num_loop_jumps - 1;