------- Comment #5 from abel at gcc dot gnu dot org 2010-06-30 08:44 ------- The below patch implements keeping dominance info up to date in the scheduler. After discussions with Alexander, I think that it will be better to leave the copy_rtx calls as is, so that if we'll see this problem once again, we fail immediately instead of silently miscompiling code. The patch fixes both this testcase and PR 44233's testcase.
(Also, Zdenek kindly provided a patch for the dominator tree infrastructure that eases the development by failing immediately when the dominator data structures become inconsistent, and I've made a cleanup patch in the scheduler. I will also submit both of those patches.) gcc/haifa-sched.c | 2 + gcc/sel-sched-ir.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c index 8d7149f..6bf526e 100644 --- a/gcc/haifa-sched.c +++ b/gcc/haifa-sched.c @@ -4452,6 +4452,8 @@ sched_create_recovery_edges (basic_block first_bb, basic_block rec, edge_flags = 0; make_single_succ_edge (rec, second_bb, edge_flags); + if (dom_info_available_p (CDI_DOMINATORS)) + set_immediate_dominator (CDI_DOMINATORS, rec, first_bb); } /* This function creates recovery code for INSN. If MUTATE_P is nonzero, diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c index 3146a26..f2be2df 100644 --- a/gcc/sel-sched-ir.c +++ b/gcc/sel-sched-ir.c @@ -3544,6 +3544,7 @@ static bool maybe_tidy_empty_bb (basic_block bb, bool recompute_toporder_p) { basic_block succ_bb, pred_bb; + VEC (basic_block, heap) *dom_bbs; edge e; edge_iterator ei; bool rescan_p; @@ -3579,6 +3580,7 @@ maybe_tidy_empty_bb (basic_block bb, bool recompute_toporder_p) succ_bb = single_succ (bb); rescan_p = true; pred_bb = NULL; + dom_bbs = NULL; /* Redirect all non-fallthru edges to the next bb. */ while (rescan_p) @@ -3591,6 +3593,12 @@ maybe_tidy_empty_bb (basic_block bb, bool recompute_toporder_p) if (!(e->flags & EDGE_FALLTHRU)) { + /* We will update dominators here only when we'll get + an unreachable block when redirecting, otherwise + sel_redirect_edge_and_branch will take care of it. */ + if (e->dest != bb + && single_pred_p (e->dest)) + VEC_safe_push (basic_block, heap, dom_bbs, e->dest); recompute_toporder_p |= sel_redirect_edge_and_branch (e, succ_bb); rescan_p = true; break; @@ -3610,11 +3618,20 @@ maybe_tidy_empty_bb (basic_block bb, bool recompute_toporder_p) remove_empty_bb (bb, true); } + + if (!VEC_empty (basic_block, dom_bbs)) + { + VEC_safe_push (basic_block, heap, dom_bbs, succ_bb); + iterate_fix_dominators (CDI_DOMINATORS, dom_bbs, false); + VEC_free (basic_block, heap, dom_bbs); + } + if (recompute_toporder_p) sel_recompute_toporder (); #ifdef ENABLE_CHECKING verify_backedges (); + verify_dominators (CDI_DOMINATORS); #endif return true; @@ -5026,7 +5043,12 @@ sel_remove_bb (basic_block bb, bool remove_from_cfg_p) bitmap_clear_bit (blocks_to_reschedule, idx); if (remove_from_cfg_p) - delete_and_free_basic_block (bb); + { + basic_block succ = single_succ (bb); + delete_and_free_basic_block (bb); + set_immediate_dominator (CDI_DOMINATORS, succ, + recompute_dominator (CDI_DOMINATORS, succ)); + } rgn_setup_region (CONTAINING_RGN (idx)); } @@ -5361,12 +5383,15 @@ sel_merge_blocks (basic_block a, basic_block b) void sel_redirect_edge_and_branch_force (edge e, basic_block to) { - basic_block jump_bb, src; + basic_block jump_bb, src, orig_dest = e->dest; int prev_max_uid; rtx jump; - gcc_assert (!sel_bb_empty_p (e->src)); - + /* This function is now used only for bookkeeping code creation, where + we'll never get the single pred of orig_dest block and thus will not + hit unreachable blocks when updating dominator info. */ + gcc_assert (!sel_bb_empty_p (e->src) + && !single_pred_p (orig_dest)); src = e->src; prev_max_uid = get_max_uid (); jump_bb = redirect_edge_and_branch_force (e, to); @@ -5383,6 +5408,10 @@ sel_redirect_edge_and_branch_force (edge e, basic_block to) jump = find_new_jump (src, jump_bb, prev_max_uid); if (jump) sel_init_new_insn (jump, INSN_INIT_TODO_LUID | INSN_INIT_TODO_SIMPLEJUMP); + set_immediate_dominator (CDI_DOMINATORS, orig_dest, + recompute_dominator (CDI_DOMINATORS, orig_dest)); + set_immediate_dominator (CDI_DOMINATORS, to, + recompute_dominator (CDI_DOMINATORS, to)); } /* A wrapper for redirect_edge_and_branch. Return TRUE if blocks connected by @@ -5391,11 +5420,12 @@ bool sel_redirect_edge_and_branch (edge e, basic_block to) { bool latch_edge_p; - basic_block src; + basic_block src, orig_dest = e->dest; int prev_max_uid; rtx jump; edge redirected; bool recompute_toporder_p = false; + bool maybe_unreachable = single_pred_p (orig_dest); latch_edge_p = (pipelining_p && current_loop_nest @@ -5426,6 +5456,15 @@ sel_redirect_edge_and_branch (edge e, basic_block to) if (jump) sel_init_new_insn (jump, INSN_INIT_TODO_LUID | INSN_INIT_TODO_SIMPLEJUMP); + /* Only update dominator info when we don't have unreachable blocks. + Otherwise we'll update in maybe_tidy_empty_bb. */ + if (!maybe_unreachable) + { + set_immediate_dominator (CDI_DOMINATORS, orig_dest, + recompute_dominator (CDI_DOMINATORS, orig_dest)); + set_immediate_dominator (CDI_DOMINATORS, to, + recompute_dominator (CDI_DOMINATORS, to)); + } return recompute_toporder_p; } @@ -6155,6 +6194,10 @@ sel_remove_loop_preheader (void) if (BB_END (prev_bb) == bb_note (prev_bb)) free_data_sets (prev_bb); } + + set_immediate_dominator (CDI_DOMINATORS, next_bb, + recompute_dominator (CDI_DOMINATORS, + next_bb)); } } VEC_free (basic_block, heap, preheader_blocks); -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43603