<div dir="ltr"><div>Thanks for the explanation and fixing the boolean mess</div><div> </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div></div> <div class="gmail_quote"><div dir="ltr">On Mon, Jul 23, 2018 at 3:02 AM Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com">tarceri@itsqueeze.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div>