[Mesa-dev] [PATCH v2 3/8] nir: allow more nested loops to be unrolled

[Jason Ekstrand]

Thanks for the explanation and fixing the boolean mess

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

On Mon, Jul 23, 2018 at 3:02 AM Timothy Arceri <tarceri at itsqueeze.com>
wrote:

> The innermost check was added to stop us from unrolling multiple
> loops in a single pass, and to stop outer loops from unrolling.
>
> When we successfully unroll a loop we need to run the analysis
> pass again before deciding if we want to go ahead an unroll a
> second loop.
>
> However the logic was flawed because it never tried to unroll any
> nested loops other than the first innermost loop it found.
> If this innermost loop is not unrolled we end up skipping all
> other nested loops.
>
> This unrolls a loop in a Deus Ex: MD shader on ultra settings and
> also unrolls a loop in a shader from the game Prey when running
> on DXVK.
> ---
>  src/compiler/nir/nir_opt_loop_unroll.c | 31 ++++++++++++++------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_loop_unroll.c
> b/src/compiler/nir/nir_opt_loop_unroll.c
> index 955dfede694..a1ad0e59814 100644
> --- a/src/compiler/nir/nir_opt_loop_unroll.c
> +++ b/src/compiler/nir/nir_opt_loop_unroll.c
> @@ -483,9 +483,10 @@ is_loop_small_enough_to_unroll(nir_shader *shader,
> nir_loop_info *li)
>  }
>
>  static bool
> -process_loops(nir_shader *sh, nir_cf_node *cf_node, bool *innermost_loop)
> +process_loops(nir_shader *sh, nir_cf_node *cf_node, bool
> *has_nested_loop_out)
>  {
>     bool progress = false;
> +   bool has_nested_loop = false;
>     nir_loop *loop;
>
>     switch (cf_node->type) {
> @@ -494,32 +495,32 @@ process_loops(nir_shader *sh, nir_cf_node *cf_node,
> bool *innermost_loop)
>     case nir_cf_node_if: {
>        nir_if *if_stmt = nir_cf_node_as_if(cf_node);
>        foreach_list_typed_safe(nir_cf_node, nested_node, node,
> &if_stmt->then_list)
> -         progress |= process_loops(sh, nested_node, innermost_loop);
> +         progress |= process_loops(sh, nested_node, has_nested_loop_out);
>        foreach_list_typed_safe(nir_cf_node, nested_node, node,
> &if_stmt->else_list)
> -         progress |= process_loops(sh, nested_node, innermost_loop);
> +         progress |= process_loops(sh, nested_node, has_nested_loop_out);
>        return progress;
>     }
>     case nir_cf_node_loop: {
>        loop = nir_cf_node_as_loop(cf_node);
>        foreach_list_typed_safe(nir_cf_node, nested_node, node, &loop->body)
> -         progress |= process_loops(sh, nested_node, innermost_loop);
> +         progress |= process_loops(sh, nested_node, &has_nested_loop);
> +
>        break;
>     }
>     default:
>        unreachable("unknown cf node type");
>     }
>
> -   if (*innermost_loop) {
> -      /* Don't attempt to unroll outer loops or a second inner loop in
> -       * this pass wait until the next pass as we have altered the cf.
> -       */
> -      *innermost_loop = false;
> +   /* Don't attempt to unroll a second inner loop in this pass, wait
> until the
> +    * next pass as we have altered the cf.
> +    */
> +   if (!progress) {
>
> -      if (loop->info->limiting_terminator == NULL)
> -         return progress;
> +      if (has_nested_loop || loop->info->limiting_terminator == NULL)
> +         goto exit;
>
>        if (!is_loop_small_enough_to_unroll(sh, loop->info))
> -         return progress;
> +         goto exit;
>
>        if (loop->info->is_trip_count_known) {
>           simple_unroll(loop);
> @@ -555,6 +556,8 @@ process_loops(nir_shader *sh, nir_cf_node *cf_node,
> bool *innermost_loop)
>        }
>     }
>
> +exit:
> +   *has_nested_loop_out = true;
>     return progress;
>  }
>
> @@ -567,9 +570,9 @@ nir_opt_loop_unroll_impl(nir_function_impl *impl,
>     nir_metadata_require(impl, nir_metadata_block_index);
>
>     foreach_list_typed_safe(nir_cf_node, node, node, &impl->body) {
> -      bool innermost_loop = true;
> +      bool has_nested_loop = false;
>        progress |= process_loops(impl->function->shader, node,
> -                                &innermost_loop);
> +                                &has_nested_loop);
>     }
>
>     if (progress)
> --
> 2.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180817/7379f09f/attachment.html>