From 54fbc625ea132fa99718f7878c91d9f565a0bfb9 Mon Sep 17 00:00:00 2001 From: Francisco Jerez Date: Tue, 23 Jul 2019 16:17:07 -0700 Subject: [PATCH] intel/ir: Fix CFG corruption in opt_predicated_break(). Specifically the optimization of a conditional BREAK + WHILE sequence into a conditional WHILE seems pretty broken. The list of successors of "earlier_block" (where the conditional BREAK was found) is emptied and then re-created with the same edges for no apparent reason. On top of that the list of predecessors of the block immediately after the WHILE loop is emptied, but only one of the original edges will be added back, which means that potentially several blocks that still have it on their list of successors won't be on its list of predecessors anymore, causing all sorts of hilarity due to the inconsistency in the control flow graph. The solution is to remove the code that's removing valid edges from the CFG. cfg_t::remove_block() will already clean up after itself. The assert in bblock_t::combine_with() also needs to be removed since we will be merging a block with multiple children into the first one of them. Found the issue on a hardware enabling branch originally, but apparently somebody reproduced the same problem independently on master in the meantime. Fixes: d13bcdb3a9f ("i965/fs: Extend predicated break pass to predicate WHILE.") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111009 Cc: jiradet.jd@gmail.com Cc: Sergii Romantsov Cc: Matt Turner Cc: mesa-stable@lists.freedesktop.org Tested-by: Paul Chelombitko Reviewed-by: Matt Turner --- src/intel/compiler/brw_cfg.cpp | 3 --- src/intel/compiler/brw_predicated_break.cpp | 6 ------ 2 files changed, 9 deletions(-) diff --git a/src/intel/compiler/brw_cfg.cpp b/src/intel/compiler/brw_cfg.cpp index 600b428a492..6c40889088d 100644 --- a/src/intel/compiler/brw_cfg.cpp +++ b/src/intel/compiler/brw_cfg.cpp @@ -128,9 +128,6 @@ void bblock_t::combine_with(bblock_t *that) { assert(this->can_combine_with(that)); - foreach_list_typed (bblock_link, link, link, &this->children) { - assert(link->block == that); - } foreach_list_typed (bblock_link, link, link, &that->parents) { assert(link->block == this); } diff --git a/src/intel/compiler/brw_predicated_break.cpp b/src/intel/compiler/brw_predicated_break.cpp index 607715dace4..e60052f3608 100644 --- a/src/intel/compiler/brw_predicated_break.cpp +++ b/src/intel/compiler/brw_predicated_break.cpp @@ -128,14 +128,8 @@ opt_predicated_break(backend_shader *s) while_inst->predicate = jump_inst->predicate; while_inst->predicate_inverse = !jump_inst->predicate_inverse; - earlier_block->children.make_empty(); - earlier_block->add_successor(s->cfg->mem_ctx, while_block); - assert(earlier_block->can_combine_with(while_block)); earlier_block->combine_with(while_block); - - earlier_block->next()->parents.make_empty(); - earlier_block->add_successor(s->cfg->mem_ctx, earlier_block->next()); } progress = true; -- 2.30.2