<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Aug 27, 2018 at 4:10 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">In GLSL IR we cheat with switch statements and simply convert them into loops with a single iteration. This allowed us to make use of the existing jump instruction handling provided by the loop handing code, it also allows dead code to be cleaned up once we have wrapped the code in a loop. However using loops in this way created previously unrollable loops which limits further optimisations. Here we provide a way to unroll loops that end in a break and have multiple other exits. All shader-db changes are from the dolphin uber shaders. There is a small amount of HURT shaders but in general the improvements far exceed the HURT. shader-db results IVB: total instructions in shared programs: 10018187 -> 10016468 (-0.02%) instructions in affected programs: 104080 -> 102361 (-1.65%) helped: 36 HURT: 15 total cycles in shared programs: 220065064 -> 154529655 (-29.78%) cycles in affected programs: 126063017 -> 60527608 (-51.99%) helped: 51 HURT: 0 total loops in shared programs: 2515 -> 2308 (-8.23%) loops in affected programs: 903 -> 696 (-22.92%) helped: 51 HURT: 0 total spills in shared programs: 4370 -> 4124 (-5.63%) spills in affected programs: 1397 -> 1151 (-17.61%) helped: 9 HURT: 12 total fills in shared programs: 4581 -> 4419 (-3.54%) fills in affected programs: 2201 -> 2039 (-7.36%) helped: 9 HURT: 15 --- src/compiler/nir/nir_opt_loop_unroll.c | 113 +++++++++++++++++-------- 1 file changed, 76 insertions(+), 37 deletions(-) diff --git a/src/compiler/nir/nir_opt_loop_unroll.c b/src/compiler/nir/nir_opt_loop_unroll.c index 9c33267cb72..fa60523aac7 100644 --- a/src/compiler/nir/nir_opt_loop_unroll.c +++ b/src/compiler/nir/nir_opt_loop_unroll.c @@ -67,7 +67,6 @@ loop_prepare_for_unroll(nir_loop *loop) /* Remove continue if its the last instruction in the loop */ nir_instr *last_instr = nir_block_last_instr(nir_loop_last_block(loop)); if (last_instr && last_instr->type == nir_instr_type_jump) { - assert(nir_instr_as_jump(last_instr)->type == nir_jump_continue); nir_instr_remove(last_instr); } } @@ -474,54 +473,91 @@ complex_unroll(nir_loop *loop, nir_loop_terminator *unlimit_term, static bool wrapper_unroll(nir_loop *loop) { - bool progress = false; - - nir_block *blk_after_loop = - nir_cursor_current_block(nir_after_cf_node(&loop->cf_node)); - - /* There may still be some single src phis following the loop that - * have not yet been cleaned up by another pass. Tidy those up before - * unrolling the loop. - */ - nir_foreach_instr_safe(instr, blk_after_loop) { - if (instr->type != nir_instr_type_phi) - break; + if (!list_empty(&loop->info->loop_terminator_list)) { - nir_phi_instr *phi = nir_instr_as_phi(instr); - assert(exec_list_length(&phi->srcs) == 1); + /* Unrolling a loop with a large number of exits can result in a + * large inrease in register pressure. For now we just skip + * unrolling if we have more than 3 exits (not including the break + * at the end of the loop). + * + * TODO: Most loops that fit this pattern are simply switch + * statements that are converted to a loop to take advantage of + * exiting jump instruction handling. In this case we could make + * use of a binary seach pattern like we do in + * nir_lower_indirect_derefs(), this should allow us to unroll the + * loops in an optimal way and should also avoid some of the + * register pressure that comes from simply nesting the + * terminators one after the other. + */ + if (list_length(&loop->info->loop_terminator_list) > 3) + return false; + + loop_prepare_for_unroll(loop); + + nir_cursor cursor = nir_after_block(nir_loop_last_block(loop)); </blockquote><div> </div><div>Maybe call this loop_end; that's less generic than "cursor". </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + list_for_each_entry(nir_loop_terminator, terminator, + &loop->info->loop_terminator_list, + loop_terminator_link) { </blockquote><div> </div><div>I presume the terminator list is guaranteed to be sorted top to bottom?</div><div> </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + /* Remove break from the terminator */ + nir_instr *break_instr = + nir_block_last_instr(terminator->break_block); + nir_instr_remove(break_instr); + + /* Pluck out the loop body. */ + nir_cf_list loop_body; + nir_cf_extract(&loop_body, + nir_after_cf_node(&terminator->nif->cf_node), + cursor); + + /* Reinsert loop body into continue from block */ + nir_cf_reinsert(&loop_body, + nir_after_block(terminator->continue_from_block)); + + cursor = terminator->continue_from_then ? + nir_after_block(nir_if_last_then_block(terminator->nif)) : + nir_after_block(nir_if_last_else_block(terminator->nif)); + } + } else { + nir_block *blk_after_loop = + nir_cursor_current_block(nir_after_cf_node(&loop->cf_node)); - nir_phi_src *phi_src = exec_node_data(nir_phi_src, - exec_list_get_head(&phi->srcs), - node); + /* There may still be some single src phis following the loop that + * have not yet been cleaned up by another pass. Tidy those up + * before unrolling the loop. + */ + nir_foreach_instr_safe(instr, blk_after_loop) { + if (instr->type != nir_instr_type_phi) + break; - nir_ssa_def_rewrite_uses(&phi->dest.ssa, phi_src->src); - nir_instr_remove(instr); + nir_phi_instr *phi = nir_instr_as_phi(instr); + assert(exec_list_length(&phi->srcs) == 1); - progress = true; - } + nir_phi_src *phi_src = + exec_node_data(nir_phi_src, exec_list_get_head(&phi->srcs), node); - nir_block *last_loop_blk = nir_loop_last_block(loop); - if (nir_block_ends_in_break(last_loop_blk)) { + nir_ssa_def_rewrite_uses(&phi->dest.ssa, phi_src->src); + nir_instr_remove(instr); + } /* Remove break at end of the loop */ + nir_block *last_loop_blk = nir_loop_last_block(loop); nir_instr *break_instr = nir_block_last_instr(last_loop_blk); nir_instr_remove(break_instr); + } - /* Pluck out the loop body. */ - nir_cf_list loop_body; - nir_cf_extract(&loop_body, nir_before_block(nir_loop_first_block(loop)), - nir_after_block(nir_loop_last_block(loop))); - - /* Reinsert loop body after the loop */ - nir_cf_reinsert(&loop_body, nir_after_cf_node(&loop->cf_node)); + /* Pluck out the loop body. */ + nir_cf_list loop_body; + nir_cf_extract(&loop_body, nir_before_block(nir_loop_first_block(loop)), + nir_after_block(nir_loop_last_block(loop))); - /* The loop has been unrolled so remove it. */ - nir_cf_node_remove(&loop->cf_node); + /* Reinsert loop body after the loop */ + nir_cf_reinsert(&loop_body, nir_after_cf_node(&loop->cf_node)); - progress = true; - } + /* The loop has been unrolled so remove it. */ + nir_cf_node_remove(&loop->cf_node); - return progress; + return true; } static bool @@ -585,9 +621,12 @@ process_loops(nir_shader *sh, nir_cf_node *cf_node, bool *has_nested_loop_out) * statements in a loop like this. */ if (loop->info->limiting_terminator == NULL && - list_empty(&loop->info->loop_terminator_list) && !loop->info->complex_loop) { + nir_block *last_loop_blk = nir_loop_last_block(loop); + if (!nir_block_ends_in_break(last_loop_blk)) + goto exit; + progress = wrapper_unroll(loop); goto exit; -- 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></div>