[Mesa-dev] [PATCH v2 7/8] nir: add loop unroll support for wrapper loops

[Timothy Arceri]

On 18/08/18 07:14, Jason Ekstrand wrote:
> On Mon, Jul 23, 2018 at 3:02 AM 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 | 77 ++++++++++++++++++++++++++
>       1 file changed, 77 insertions(+)
> 
>     diff --git a/src/compiler/nir/nir_opt_loop_unroll.c
>     b/src/compiler/nir/nir_opt_loop_unroll.c
>     index e0e0b754716..9c33267cb72 100644
>     --- a/src/compiler/nir/nir_opt_loop_unroll.c
>     +++ b/src/compiler/nir/nir_opt_loop_unroll.c
>     @@ -465,6 +465,65 @@ complex_unroll(nir_loop *loop,
>     nir_loop_terminator *unlimit_term,
>          _mesa_hash_table_destroy(remap_table, NULL);
>       }
> 
>     +/* Unrolls the classic wrapper loops e.g
>     + *
>     + *    do {
>     + *        // ...
>     + *    } while (false)
>     + */
>     +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;
>     +
>     +      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)) {
>     +
>     +      /* 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;
>     +   }
>     +
>     +   return progress;
>     +}
>     +
>       static bool
>       is_loop_small_enough_to_unroll(nir_shader *shader, nir_loop_info *li)
>       {
>     @@ -516,6 +575,24 @@ process_loops(nir_shader *sh, nir_cf_node
>     *cf_node, bool *has_nested_loop_out)
>           */
>          if (!progress) {
> 
>     +      /* Check for 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) {
> 
> 
> What does this do with "do { } while (true)"?  Would those also hit this 
> path?  If they do, unrolling them is probably ok since it's a GPU hang 
> if we don't but I thought it was worth asking.

No this should leave them as is. There is a check in wrapper_unroll() to 
make sure the loop has a break in the last block.


> 
>     +
>     +         progress = wrapper_unroll(loop);
>     +
>     +         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
>