From 9674c76c0e473a3edbc45f935ea88afd64024325 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Thu, 3 Sep 2015 00:33:50 -0700 Subject: [PATCH] nir/cf: Don't break outer-block successors in split_block_beginning(). Consider the following NIR: block block_0; /* succs: block_1 block_2 */ if (...) { block block_1; ... } else { block block_2; } Calling split_block_beginning() on block_1 would break block_0's successors: link_block() sets both successors of a block, so calling link_block(block_0, new_block, NULL) would throw away the second successor, leaving only /* succ: new_block */. This is invalid: the block before an if statement must have two successors. Changing the call to link_block(pred, new_block, pred->successors[0]) would correctly leave both successors in place, but because unlink_block may shift successor[1] to successor[0], it may not preserve the original order. NIR maintains a convention that successor[0] must point to the "then" block, while successor[1] points to the "else" block, so we need to take care to preserve this ordering. This patch creates a new function that swaps out one successor for another, preserving the ordering. It then uses this to fix the issue. Signed-off-by: Kenneth Graunke Reviewed-by: Connor Abbott Reviewed-by: Jason Ekstrand --- src/glsl/nir/nir_control_flow.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/glsl/nir/nir_control_flow.c b/src/glsl/nir/nir_control_flow.c index 43e4e43aede..e2a151dafac 100644 --- a/src/glsl/nir/nir_control_flow.c +++ b/src/glsl/nir/nir_control_flow.c @@ -199,6 +199,23 @@ 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 @@ -217,9 +234,7 @@ split_block_beginning(nir_block *block) struct set_entry *entry; set_foreach(block->predecessors, entry) { nir_block *pred = (nir_block *) entry->key; - - unlink_blocks(pred, block); - link_blocks(pred, new_block, NULL); + replace_successor(pred, block, new_block); } /* Any phi nodes must stay part of the new block, or else their -- 2.30.2