[Mesa-dev] [RFC 2/2] nir: When splitting blocks, always put the new block after the old

[Jason Ekstrand]

This makes block splitting happen a bit more deterministically.  In
particular, if using nir_builder to build a shader, it means that you can
always save off nir_cursor_current_block and then add another CF node after
that without fear that the nir_block pointer you just saved will get
replaced out from under you.
---
 src/compiler/nir/nir.h              |  2 +
 src/compiler/nir/nir_control_flow.c | 97 +++++++++++++------------------------
 2 files changed, 35 insertions(+), 64 deletions(-)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 6f059720..9ab8440 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -1511,6 +1511,8 @@ nir_block_last_instr(nir_block *block)
    foreach_list_typed_reverse(nir_instr, instr, node, &(block)->instr_list)
 #define nir_foreach_instr_safe(instr, block) \
    foreach_list_typed_safe(nir_instr, instr, node, &(block)->instr_list)
+#define nir_foreach_instr_safe_after(instr, block, after) \
+   foreach_list_typed_safe_after(nir_instr, instr, node, after)
 #define nir_foreach_instr_reverse_safe(instr, block) \
    foreach_list_typed_reverse_safe(nir_instr, instr, node, &(block)->instr_list)
 
diff --git a/src/compiler/nir/nir_control_flow.c b/src/compiler/nir/nir_control_flow.c
index 1ff7a53..7210197 100644
--- a/src/compiler/nir/nir_control_flow.c
+++ b/src/compiler/nir/nir_control_flow.c
@@ -178,60 +178,6 @@ link_block_to_non_block(nir_block *block, nir_cf_node *node)
 
 }
 
-/**
- * Replace a block's successor with a different one.
- */
-static void
-replace_successor(nir_block *block, nir_block *old_succ, nir_block *new_succ)
-{
-   if (block->successors[0] == old_succ) {
-      block->successors[0] = new_succ;
-   } else {
-      assert(block->successors[1] == old_succ);
-      block->successors[1] = new_succ;
-   }
-
-   block_remove_pred(old_succ, block);
-   block_add_pred(new_succ, block);
-}
-
-/**
- * Takes a basic block and inserts a new empty basic block before it, making its
- * predecessors point to the new block. This essentially splits the block into
- * an empty header and a body so that another non-block CF node can be inserted
- * between the two. Note that this does *not* link the two basic blocks, so
- * some kind of cleanup *must* be performed after this call.
- */
-
-static nir_block *
-split_block_beginning(nir_block *block)
-{
-   nir_block *new_block = nir_block_create(ralloc_parent(block));
-   new_block->cf_node.parent = block->cf_node.parent;
-   exec_node_insert_node_before(&block->cf_node.node, &new_block->cf_node.node);
-
-   struct set_entry *entry;
-   set_foreach(block->predecessors, entry) {
-      nir_block *pred = (nir_block *) entry->key;
-      replace_successor(pred, block, new_block);
-   }
-
-   /* Any phi nodes must stay part of the new block, or else their
-    * sourcse will be messed up. This will reverse the order of the phi's, but
-    * order shouldn't matter.
-    */
-   nir_foreach_instr_safe(instr, block) {
-      if (instr->type != nir_instr_type_phi)
-         break;
-
-      exec_node_remove(&instr->node);
-      instr->block = new_block;
-      exec_list_push_head(&new_block->instr_list, &instr->node);
-   }
-
-   return new_block;
-}
-
 static void
 rewrite_phi_preds(nir_block *block, nir_block *old_pred, nir_block *new_pred)
 {
@@ -359,6 +305,28 @@ block_add_normal_succs(nir_block *block)
 }
 
 static nir_block *
+split_block_beginning(nir_block *block)
+{
+   nir_block *new_block = nir_block_create(ralloc_parent(block));
+   new_block->cf_node.parent = block->cf_node.parent;
+   exec_node_insert_after(&block->cf_node.node, &new_block->cf_node.node);
+
+   /* Move everything except the phis to the new block */
+   nir_foreach_instr_safe(cur_instr, block) {
+      if (cur_instr->type == nir_instr_type_phi)
+         continue;
+
+      exec_node_remove(&cur_instr->node);
+      cur_instr->block = new_block;
+      exec_list_push_tail(&new_block->instr_list, &cur_instr->node);
+   }
+
+   move_successors(block, new_block);
+
+   return new_block;
+}
+
+static nir_block *
 split_block_end(nir_block *block)
 {
    nir_block *new_block = nir_block_create(ralloc_parent(block));
@@ -381,12 +349,13 @@ static nir_block *
 split_block_before_instr(nir_instr *instr)
 {
    assert(instr->type != nir_instr_type_phi);
-   nir_block *new_block = split_block_beginning(instr->block);
+   nir_block *block = instr->block;
 
-   nir_foreach_instr_safe(cur_instr, instr->block) {
-      if (cur_instr == instr)
-         break;
+   nir_block *new_block = nir_block_create(ralloc_parent(block));
+   new_block->cf_node.parent = block->cf_node.parent;
+   exec_node_insert_after(&block->cf_node.node, &new_block->cf_node.node);
 
+   nir_foreach_instr_safe_after(cur_instr, instr->block, instr) {
       exec_node_remove(&cur_instr->node);
       cur_instr->block = new_block;
       exec_list_push_tail(&new_block->instr_list, &cur_instr->node);
@@ -409,8 +378,8 @@ split_block_cursor(nir_cursor cursor,
    nir_block *before, *after;
    switch (cursor.option) {
    case nir_cursor_before_block:
-      after = cursor.block;
-      before = split_block_beginning(cursor.block);
+      before = cursor.block;
+      after = split_block_beginning(cursor.block);
       break;
 
    case nir_cursor_after_block:
@@ -419,8 +388,8 @@ split_block_cursor(nir_cursor cursor,
       break;
 
    case nir_cursor_before_instr:
-      after = cursor.instr->block;
-      before = split_block_before_instr(cursor.instr);
+      before = cursor.instr->block;
+      after = split_block_before_instr(cursor.instr);
       break;
 
    case nir_cursor_after_instr:
@@ -431,8 +400,8 @@ split_block_cursor(nir_cursor cursor,
          before = cursor.instr->block;
          after = split_block_end(cursor.instr->block);
       } else {
-         after = cursor.instr->block;
-         before = split_block_before_instr(nir_instr_next(cursor.instr));
+         before = cursor.instr->block;
+         after = split_block_before_instr(nir_instr_next(cursor.instr));
       }
       break;
 
-- 
2.5.0.400.gff86faf