Consider the case of "while (...) { break }". Or in NIR: block block_0 (0x7ab640): ... /* succs: block_1 */ loop { block block_1: /* preds: block_0 */ break /* succs: block_2 */ } block block_2:
Calling nir_handle_remove_jump(block_1, nir_jump_break) will remove the break. Unfortunately, it would mangle the predecessors and successors. Here, block_2->predecessors->entries == 1, so we would create a fake link, setting block_1->successors[1] = block_2, and adding block_1 to block_2's predecessor set. This is illegal: a block cannot specify the same successor twice. In particular, adding the predecessor would have no effect, as it was already present in the set. We'd then call unlink_block_successors(), which would delete the fake link and remove block_1 from block_2's predecessor set. It would then delete successors[0], and attempt to remove block_1 from block_2's predecessor set a second time...except that it wouldn't be present, triggering an assertion failure. The fix appears to be simple: move the fake link creation after we recreate the block's successors. Since we're inspecting "next" later, If we've created a new infinite loop, the code following the loop will be dead, and thus 0 predecessors. So we create a new fake successor. simply unlink the block's successors and recreate them to point at the correct blocks first. Then, add the fake link. In the above example, removing the break would cause block_1 to have itself as a successor (as it becomes an infinite loop), and then Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> --- src/glsl/nir/nir_control_flow.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) New patch! diff --git a/src/glsl/nir/nir_control_flow.c b/src/glsl/nir/nir_control_flow.c index 2b23f38..a59add5 100644 --- a/src/glsl/nir/nir_control_flow.c +++ b/src/glsl/nir/nir_control_flow.c @@ -551,31 +551,29 @@ remove_phi_src(nir_block *block, nir_block *pred) static void unlink_jump(nir_block *block, nir_jump_type type, bool add_normal_successors) { + nir_block *next = block->successors[0]; + if (block->successors[0]) remove_phi_src(block->successors[0], block); if (block->successors[1]) remove_phi_src(block->successors[1], block); - if (type == nir_jump_break) { - nir_block *next = block->successors[0]; + unlink_block_successors(block); + if (add_normal_successors) + block_add_normal_succs(block); - if (next->predecessors->entries == 1) { - nir_loop *loop = - nir_cf_node_as_loop(nir_cf_node_prev(&next->cf_node)); + if (type == nir_jump_break && next->predecessors->entries == 0) { + nir_loop *loop = + nir_cf_node_as_loop(nir_cf_node_prev(&next->cf_node)); - /* insert fake link */ - nir_cf_node *last = nir_loop_last_cf_node(loop); - assert(last->type == nir_cf_node_block); - nir_block *last_block = nir_cf_node_as_block(last); + /* insert fake link */ + nir_cf_node *last = nir_loop_last_cf_node(loop); + assert(last->type == nir_cf_node_block); + nir_block *last_block = nir_cf_node_as_block(last); - last_block->successors[1] = next; - block_add_pred(next, last_block); - } + last_block->successors[1] = next; + block_add_pred(next, last_block); } - - unlink_block_successors(block); - if (add_normal_successors) - block_add_normal_succs(block); } void -- 2.5.3 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev