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

2018-08-17 Thread Timothy Arceri 

On 18/08/18 07:14, Jason Ekstrand wrote:
On Mon, Jul 23, 2018 at 3:02 AM Timothy Arceri > 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(>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(>srcs) == 1);
+
+      nir_phi_src *phi_src = exec_node_data(nir_phi_src,
+   
exec_list_get_head(>srcs),

+                                            node);
+
+      nir_ssa_def_rewrite_uses(>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(_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(_body, nir_after_cf_node(>cf_node));
+
+      /* The loop has been unrolled so remove it. */
+      nir_cf_node_remove(>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(>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@lists.freedesktop.org 
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-08-17 Thread Jason Ekstrand 
On Mon, Jul 23, 2018 at 3:02 AM Timothy Arceri 
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(>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(>srcs) == 1);
> +
> +  nir_phi_src *phi_src = exec_node_data(nir_phi_src,
> +
> exec_list_get_head(>srcs),
> +node);
> +
> +  nir_ssa_def_rewrite_uses(>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(_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(_body, nir_after_cf_node(>cf_node));
> +
> +  /* The loop has been unrolled so remove it. */
> +  nir_cf_node_remove(>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(>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.


> +
> + 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-07-23 Thread Timothy Arceri 
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(>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(>srcs) == 1);
+
+  nir_phi_src *phi_src = exec_node_data(nir_phi_src,
+exec_list_get_head(>srcs),
+node);
+
+  nir_ssa_def_rewrite_uses(>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(_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(_body, nir_after_cf_node(>cf_node));
+
+  /* The loop has been unrolled so remove it. */
+  nir_cf_node_remove(>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(>info->loop_terminator_list) &&
+  !loop->info->complex_loop) {
+
+ 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev