[Mesa-dev] [PATCH 07/10] nir: add loop unroll support for wrapper loops

[Timothy Arceri]

On 22/07/18 19:50, Jason Ekstrand wrote:
> On Tue, Jul 10, 2018 at 11:49 PM Timothy Arceri <tarceri at itsqueeze.com 
> <mailto:tarceri at itsqueeze.com>> wrote:
> 
>     This adds support for unrolling the classic
> 
>          do {
>              // ...
>          } while (false)
> 
>     that is used to wrap multi-line macros. GLSL IR also wraps switch
>     statements in a loop like this.
> 
>     shader-db results IVB:
> 
>     total loops in shared programs: 2515 -> 2512 (-0.12%)
>     loops in affected programs: 33 -> 30 (-9.09%)
>     helped: 3
>     HURT: 0
>     ---
>       src/compiler/nir/nir_opt_loop_unroll.c | 63 ++++++++++++++++++++++++++
>       1 file changed, 63 insertions(+)
> 
>     diff --git a/src/compiler/nir/nir_opt_loop_unroll.c
>     b/src/compiler/nir/nir_opt_loop_unroll.c
>     index 38901470f96..f715fc8ebda 100644
>     --- a/src/compiler/nir/nir_opt_loop_unroll.c
>     +++ b/src/compiler/nir/nir_opt_loop_unroll.c
>     @@ -516,6 +516,69 @@ process_loops(nir_shader *sh, nir_cf_node
>     *cf_node, bool *has_nested_loop)
>           */
>          if (!progress) {
> 
>     +      /* Check for the the classic
>     +       *
>     +       *    do {
>     +       *        // ...
>     +       *    } while (false)
>     +       *
>     +       * that is used to wrap multi-line macros. GLSL IR also wraps
>     switch
>     +       * statements in a loop like this.
>     +       */
>     +      if (loop->info->limiting_terminator == NULL &&
>     +          list_empty(&loop->info->loop_terminator_list) &&
>     +          !loop->info->complex_loop) {
>     +
> 
> 
> Mind putting all the code below in a "wrapper_unroll" or 
> "trivial_unroll" helper?

Sure.

> 
>     +         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;
>     +
>     +            nir_phi_instr *phi = nir_instr_as_phi(instr);
>     +            assert(exec_list_length(&phi->srcs) == 1);
>     +
>     +            nir_phi_src *phi_src =
>     +               exec_node_data(nir_phi_src,
>     +                              exec_list_get_head(&phi->srcs),
>     +                              node);
>     +
>     +            nir_ssa_def_rewrite_uses(&phi->dest.ssa, phi_src->src);
>     +            nir_instr_remove(instr);
>     +
>     +            progress = true;
>     +         }
>     +
>     +         nir_block *last_loop_blk = nir_loop_last_block(loop);
>     +         if (nir_block_ends_in_break(last_loop_blk)) {
> 
> 
> This confuses me.  Above, you assert that all phis have exactly one 
> source and here you have an if (nir_block_ends_in_break(last_block)); 
> which one is it?  Either our loop terminator check above guarantees that 
> we're in the "do { } while (true)" case where we can assume a single 
> exit at the end of the loop or it doesn't.  Which one is it?

Yeah I think this logic slipped in too early and belongs in the 
following patch only.


> 
>     +
>     +            /* Remove break at end of the 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));
>     +
>     +            /* The loop has been unrolled so remove it. */
>     +            nir_cf_node_remove(&loop->cf_node);
>     +
>     +            progress = true;
>     +         }
>     +
>     +         goto exit;
>     +      }
>     +
>             if (*has_nested_loop || loop->info->limiting_terminator == NULL)
>                goto exit;
> 
>     -- 
>     2.17.1
> 
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>