<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sat, Jul 14, 2018 at 4:26 PM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl">bas@basnieuwenhuizen.nl</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Reinserting code directly before a jump means the block gets split<br>
and merged, removing the original block and replacing it in the<br>
process.<br>
<br>
Hence keeping a pointer to the continue block over a reinsert<br>
causes issues.<br>
<br>
This code changes nir_opt_if to simply look for the new continue<br>
block.<br>
<br>
CC: 18.1 <<a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.org</a>><br>
---<br>
 src/compiler/nir/nir_opt_if.c | 33 +++++++++++++++++++++++++++------<br>
 1 file changed, 27 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c<br>
index a52de120ad6..658ff654169 100644<br>
--- a/src/compiler/nir/nir_opt_if.c<br>
+++ b/src/compiler/nir/nir_opt_if.c<br>
@@ -26,6 +26,28 @@<br>
 #include "nir_control_flow.h"<br>
 #include "nir_loop_analyze.h"<br>
<br>
+/**<br>
+ * Gets the single block that jumps back to the loop header. Already assumes<br>
+ * there is exactly one such block.<br>
+ */<br>
+static nir_block* find_continue_block(nir_loop *loop)<br></blockquote><div><br></div><div>The return type goes on its own line.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+{<br>
+   nir_block *header_block = nir_loop_first_block(loop);<br>
+   nir_block *prev_block =<br>
+      nir_cf_node_as_block(nir_cf_node_prev(&loop->cf_node));<br>
+<br>
+   assert(header_block->predecessors->entries == 2);<br>
+<br>
+   nir_block *continue_block = NULL;<br>
+   struct set_entry *pred_entry;<br>
+   set_foreach(header_block->predecessors, pred_entry) {<br>
+      if (pred_entry->key != prev_block)<br>
+         continue_block = (void *)pred_entry->key;<br></blockquote><div><br></div><div>Just return right here.  No need to keep looping or store continue_block off in a variable.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+   }<br>
+<br>
+   return continue_block;<br></blockquote><div><br></div><div>Then this becomes unreachable.</div><div><br></div><div>With those nits fixed,</div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+}<br>
+<br>
 /**<br>
  * This optimization detects if statements at the tops of loops where the<br>
  * condition is a phi node of two constants and moves half of the if to above<br>
@@ -97,12 +119,7 @@ opt_peel_loop_initial_if(nir_loop *loop)<br>
    if (header_block->predecessors->entries != 2)<br>
       return false;<br>
<br>
-   nir_block *continue_block = NULL;<br>
-   struct set_entry *pred_entry;<br>
-   set_foreach(header_block->predecessors, pred_entry) {<br>
-      if (pred_entry->key != prev_block)<br>
-         continue_block = (void *)pred_entry->key;<br>
-   }<br>
+   nir_block *continue_block = find_continue_block(loop);<br>
<br>
    nir_cf_node *if_node = nir_cf_node_next(&header_block->cf_node);<br>
    if (!if_node || if_node->type != nir_cf_node_if)<br>
@@ -193,6 +210,10 @@ opt_peel_loop_initial_if(nir_loop *loop)<br>
    nir_cf_reinsert(&tmp, nir_before_cf_node(&loop->cf_node));<br>
<br>
    nir_cf_reinsert(&header, nir_after_block_before_jump(continue_block));<br>
+<br>
+   /* Get continue block again as the previous reinsert might have removed the block. */<br>
+   continue_block = find_continue_block(loop);<br>
+<br>
    nir_cf_extract(&tmp, nir_before_cf_list(continue_list),<br>
                         nir_after_cf_list(continue_list));<br>
    nir_cf_reinsert(&tmp, nir_after_block_before_jump(continue_block));<br>
-- <br>
2.18.0<br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div></div>