From 67a8d7199fe4e474f7bd565161fa2f27d59969fc Mon Sep 17 00:00:00 2001 From: Alexandre Oliva Date: Wed, 20 Dec 2017 14:48:34 +0000 Subject: [PATCH] [SFN] debug markers before labels no more Make sure that gimple and RTL IRs don't have debug markers before labels. When we build the CFG, we move labels before any markers appearing before them. Then, make sure we don't mistakenly reintroduce them. This reverts some of the complexity that had been brought about by the initial SFN patches. for gcc/ChangeLog PR bootstrap/83396 * cfgexpand.c (label_rtx_for_bb): Revert SFN changes that allowed debug stmts before labels. (expand_gimple_basic_block): Likewise. * gimple-iterator.c (gimple_find_edge_insert_loc): Likewise. * gimple-iterator.h (gsi_after_labels): Likewise. * tree-cfgcleanup (remove_forwarder_block): Likewise, but rename reused variable, and simplify using gsi_move_before. * tree-ssa-tail-merge.c (find_duplicate): Likewise. * tree-cfg.c (make_edges, cleanup_dead_labels): Likewise. (gimple_can_merge_blocks_p, verify_gimple_in_cfg): Likewise. (gimple_verify_flow_info, gimple_block_label): Likewise. (make_blocks): Move debug markers after adjacent labels. * cfgrtl.c (skip_insns_after_block): Revert SFN changes that allowed debug insns outside blocks. * df-scan.c (df_insn_delete): Likewise. * lra-constraints.c (update_ebb_live_info): Likewise. * var-tracking.c (get_first_insn, vt_emit_notes): Likewise. (vt_initialize, delete_vta_debug_insns): Likewise. (reemit_marker_as_note): Drop BB parm. Adjust callers. From-SVN: r255895 --- gcc/ChangeLog | 23 +++++++++ gcc/cfgexpand.c | 13 +---- gcc/cfgrtl.c | 3 -- gcc/df-scan.c | 2 +- gcc/gimple-iterator.c | 8 +-- gcc/gimple-iterator.h | 15 ++---- gcc/lra-constraints.c | 7 --- gcc/tree-cfg.c | 101 ++++++++++++++++++++++++-------------- gcc/tree-cfgcleanup.c | 49 ++++++++---------- gcc/tree-ssa-tail-merge.c | 4 +- gcc/var-tracking.c | 65 +++--------------------- 11 files changed, 124 insertions(+), 166 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 5ccbe5d3b02..6e98187f016 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,26 @@ +2017-12-20 Alexandre Oliva + + PR bootstrap/83396 + * cfgexpand.c (label_rtx_for_bb): Revert SFN changes that + allowed debug stmts before labels. + (expand_gimple_basic_block): Likewise. + * gimple-iterator.c (gimple_find_edge_insert_loc): Likewise. + * gimple-iterator.h (gsi_after_labels): Likewise. + * tree-cfgcleanup (remove_forwarder_block): Likewise, but + rename reused variable, and simplify using gsi_move_before. + * tree-ssa-tail-merge.c (find_duplicate): Likewise. + * tree-cfg.c (make_edges, cleanup_dead_labels): Likewise. + (gimple_can_merge_blocks_p, verify_gimple_in_cfg): Likewise. + (gimple_verify_flow_info, gimple_block_label): Likewise. + (make_blocks): Move debug markers after adjacent labels. + * cfgrtl.c (skip_insns_after_block): Revert SFN changes that + allowed debug insns outside blocks. + * df-scan.c (df_insn_delete): Likewise. + * lra-constraints.c (update_ebb_live_info): Likewise. + * var-tracking.c (get_first_insn, vt_emit_notes): Likewise. + (vt_initialize, delete_vta_debug_insns): Likewise. + (reemit_marker_as_note): Drop BB parm. Adjust callers. + 2017-12-20 Richard Sandiford Alan Hayward David Sherwood diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index d509326b9e6..e616ec17127 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -2327,9 +2327,6 @@ label_rtx_for_bb (basic_block bb ATTRIBUTE_UNUSED) { glabel *lab_stmt; - if (is_gimple_debug (gsi_stmt (gsi))) - continue; - lab_stmt = dyn_cast (gsi_stmt (gsi)); if (!lab_stmt) break; @@ -5502,16 +5499,14 @@ expand_gimple_basic_block (basic_block bb, bool disable_tail_calls) } } - gsi = gsi_start_nondebug (stmts); + gsi = gsi_start (stmts); if (!gsi_end_p (gsi)) { stmt = gsi_stmt (gsi); if (gimple_code (stmt) != GIMPLE_LABEL) stmt = NULL; } - gsi = gsi_start (stmts); - gimple *label_stmt = stmt; rtx_code_label **elt = lab_rtx_for_bb->get (bb); if (stmt || elt) @@ -5522,8 +5517,7 @@ expand_gimple_basic_block (basic_block bb, bool disable_tail_calls) if (stmt) { expand_gimple_stmt (stmt); - if (gsi_stmt (gsi) == stmt) - gsi_next (&gsi); + gsi_next (&gsi); } if (elt) @@ -5549,9 +5543,6 @@ expand_gimple_basic_block (basic_block bb, bool disable_tail_calls) stmt = gsi_stmt (gsi); - if (stmt == label_stmt) - continue; - /* If this statement is a non-debug one, and we generate debug insns, then this one might be the last real use of a TERed SSA_NAME, but where there are still some debug uses further diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c index 45cccba680c..e2da7608a91 100644 --- a/gcc/cfgrtl.c +++ b/gcc/cfgrtl.c @@ -3390,9 +3390,6 @@ skip_insns_after_block (basic_block bb) last_insn = insn; continue; - case DEBUG_INSN: - continue; - case NOTE: switch (NOTE_KIND (insn)) { diff --git a/gcc/df-scan.c b/gcc/df-scan.c index d0addd9e150..7f1bad08724 100644 --- a/gcc/df-scan.c +++ b/gcc/df-scan.c @@ -946,7 +946,7 @@ df_insn_delete (rtx_insn *insn) In any case, we expect BB to be non-NULL at least up to register allocation, so disallow a non-NULL BB up to there. Not perfect but better than nothing... */ - gcc_checking_assert (bb != NULL || DEBUG_INSN_P (insn) || reload_completed); + gcc_checking_assert (bb != NULL || reload_completed); df_grow_bb_info (df_scan); df_grow_reg_info (); diff --git a/gcc/gimple-iterator.c b/gcc/gimple-iterator.c index 46d3c6ab692..258bbee7ffc 100644 --- a/gcc/gimple-iterator.c +++ b/gcc/gimple-iterator.c @@ -743,13 +743,9 @@ gimple_find_edge_insert_loc (edge e, gimple_stmt_iterator *gsi, if (gsi_end_p (*gsi)) return true; - /* Make sure we insert after any leading labels. We have to - skip debug stmts before or among them, though. We didn't - have to skip debug stmts after the last label, but it - shouldn't hurt if we do. */ + /* Make sure we insert after any leading labels. */ tmp = gsi_stmt (*gsi); - while (gimple_code (tmp) == GIMPLE_LABEL - || is_gimple_debug (tmp)) + while (gimple_code (tmp) == GIMPLE_LABEL) { gsi_next (gsi); if (gsi_end_p (*gsi)) diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h index 167edc18db5..e655ef857fa 100644 --- a/gcc/gimple-iterator.h +++ b/gcc/gimple-iterator.h @@ -213,24 +213,17 @@ gsi_stmt (gimple_stmt_iterator i) } /* Return a block statement iterator that points to the first - non-label statement in block BB. Skip debug stmts only if they - precede labels. */ + non-label statement in block BB. */ static inline gimple_stmt_iterator gsi_after_labels (basic_block bb) { gimple_stmt_iterator gsi = gsi_start_bb (bb); - for (gimple_stmt_iterator gskip = gsi; - !gsi_end_p (gskip); ) + for (; !gsi_end_p (gsi); ) { - if (is_gimple_debug (gsi_stmt (gskip))) - gsi_next (&gskip); - else if (gimple_code (gsi_stmt (gskip)) == GIMPLE_LABEL) - { - gsi_next (&gskip); - gsi = gskip; - } + if (gimple_code (gsi_stmt (gsi)) == GIMPLE_LABEL) + gsi_next (&gsi); else break; } diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c index 5162183f913..de45d2c0252 100644 --- a/gcc/lra-constraints.c +++ b/gcc/lra-constraints.c @@ -5820,13 +5820,6 @@ update_ebb_live_info (rtx_insn *head, rtx_insn *tail) if (NOTE_P (curr_insn) && NOTE_KIND (curr_insn) != NOTE_INSN_BASIC_BLOCK) continue; curr_bb = BLOCK_FOR_INSN (curr_insn); - if (!curr_bb) - { - gcc_assert (DEBUG_INSN_P (curr_insn)); - if (DEBUG_MARKER_INSN_P (curr_insn)) - continue; - curr_bb = prev_bb; - } if (curr_bb != prev_bb) { if (prev_bb != NULL) diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index 48fbe52edd8..ae5335bc336 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -636,6 +636,67 @@ make_blocks_1 (gimple_seq seq, basic_block bb) static void make_blocks (gimple_seq seq) { + /* Look for debug markers right before labels, and move the debug + stmts after the labels. Accepting labels among debug markers + adds no value, just complexity; if we wanted to annotate labels + with view numbers (so sequencing among markers would matter) or + somesuch, we're probably better off still moving the labels, but + adding other debug annotations in their original positions or + emitting nonbind or bind markers associated with the labels in + the original position of the labels. + + Moving labels would probably be simpler, but we can't do that: + moving labels assigns label ids to them, and doing so because of + debug markers makes for -fcompare-debug and possibly even codegen + differences. So, we have to move the debug stmts instead. To + that end, we scan SEQ backwards, marking the position of the + latest (earliest we find) label, and moving debug stmts that are + not separated from it by nondebug nonlabel stmts after the + label. */ + if (MAY_HAVE_DEBUG_MARKER_STMTS) + { + gimple_stmt_iterator label = gsi_none (); + + for (gimple_stmt_iterator i = gsi_last (seq); !gsi_end_p (i); gsi_prev (&i)) + { + gimple *stmt = gsi_stmt (i); + + /* If this is the first label we encounter (latest in SEQ) + before nondebug stmts, record its position. */ + if (is_a (stmt)) + { + if (gsi_end_p (label)) + label = i; + continue; + } + + /* Without a recorded label position to move debug stmts to, + there's nothing to do. */ + if (gsi_end_p (label)) + continue; + + /* Move the debug stmt at I after LABEL. */ + if (is_gimple_debug (stmt)) + { + gcc_assert (gimple_debug_nonbind_marker_p (stmt)); + /* As STMT is removed, I advances to the stmt after + STMT, so the gsi_prev in the for "increment" + expression gets us to the stmt we're to visit after + STMT. LABEL, however, would advance to the moved + stmt if we passed it to gsi_move_after, so pass it a + copy instead, so as to keep LABEL pointing to the + LABEL. */ + gimple_stmt_iterator copy = label; + gsi_move_after (&i, ©); + continue; + } + + /* There aren't any (more?) debug stmts before label, so + there isn't anything else to move after it. */ + label = gsi_none (); + } + } + make_blocks_1 (seq, ENTRY_BLOCK_PTR_FOR_FN (cfun)); } @@ -1005,11 +1066,7 @@ make_edges (void) tree target; if (!label_stmt) - { - if (is_gimple_debug (gsi_stmt (gsi))) - continue; - break; - } + break; target = gimple_label_label (label_stmt); @@ -1519,9 +1576,6 @@ cleanup_dead_labels (void) for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i)) { - if (is_gimple_debug (gsi_stmt (i))) - continue; - tree label; glabel *label_stmt = dyn_cast (gsi_stmt (i)); @@ -1682,12 +1736,6 @@ cleanup_dead_labels (void) for (i = gsi_start_bb (bb); !gsi_end_p (i); ) { - if (is_gimple_debug (gsi_stmt (i))) - { - gsi_next (&i); - continue; - } - tree label; glabel *label_stmt = dyn_cast (gsi_stmt (i)); @@ -1863,8 +1911,6 @@ gimple_can_merge_blocks_p (basic_block a, basic_block b) gsi_next (&gsi)) { tree lab; - if (is_gimple_debug (gsi_stmt (gsi))) - continue; glabel *label_stmt = dyn_cast (gsi_stmt (gsi)); if (!label_stmt) break; @@ -5431,7 +5477,6 @@ verify_gimple_in_cfg (struct function *fn, bool verify_nothrow) err |= err2; } - bool label_allowed = true; for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { gimple *stmt = gsi_stmt (gsi); @@ -5448,19 +5493,6 @@ verify_gimple_in_cfg (struct function *fn, bool verify_nothrow) err2 = true; } - /* Labels may be preceded only by debug markers, not debug bind - or source bind or any other statements. */ - if (gimple_code (stmt) == GIMPLE_LABEL) - { - if (!label_allowed) - { - error ("gimple label in the middle of a basic block"); - err2 = true; - } - } - else if (!gimple_debug_begin_stmt_p (stmt)) - label_allowed = false; - err2 |= verify_gimple_stmt (stmt); err2 |= verify_location (&blocks, gimple_location (stmt)); @@ -5584,10 +5616,6 @@ gimple_verify_flow_info (void) for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { tree label; - - if (is_gimple_debug (gsi_stmt (gsi))) - continue; - gimple *prev_stmt = stmt; stmt = gsi_stmt (gsi); @@ -5912,10 +5940,8 @@ gimple_block_label (basic_block bb) tree label; glabel *stmt; - for (i = s; !gsi_end_p (i); gsi_next (&i)) + for (i = s; !gsi_end_p (i); first = false, gsi_next (&i)) { - if (is_gimple_debug (gsi_stmt (i))) - continue; stmt = dyn_cast (gsi_stmt (i)); if (!stmt) break; @@ -5926,7 +5952,6 @@ gimple_block_label (basic_block bb) gsi_move_before (&i, &s); return label; } - first = false; } label = create_artificial_label (UNKNOWN_LOCATION); diff --git a/gcc/tree-cfgcleanup.c b/gcc/tree-cfgcleanup.c index a0e5797ec0e..bfcca03257d 100644 --- a/gcc/tree-cfgcleanup.c +++ b/gcc/tree-cfgcleanup.c @@ -444,7 +444,7 @@ remove_forwarder_block (basic_block bb) { edge succ = single_succ_edge (bb), e, s; basic_block dest = succ->dest; - gimple *label; + gimple *stmt; edge_iterator ei; gimple_stmt_iterator gsi, gsi_to; bool can_move_debug_stmts; @@ -457,9 +457,9 @@ remove_forwarder_block (basic_block bb) /* If the destination block consists of a nonlocal label or is a EH landing pad, do not merge it. */ - label = first_stmt (dest); - if (label) - if (glabel *label_stmt = dyn_cast (label)) + stmt = first_stmt (dest); + if (stmt) + if (glabel *label_stmt = dyn_cast (stmt)) if (DECL_NONLOCAL (gimple_label_label (label_stmt)) || EH_LANDING_PAD_NR (gimple_label_label (label_stmt)) != 0) return false; @@ -536,28 +536,23 @@ remove_forwarder_block (basic_block bb) defined labels and labels with an EH landing pad number to the new block, so that the redirection of the abnormal edges works, jump targets end up in a sane place and debug information for - labels is retained. - - While at that, move any debug stmts that appear before or in between - labels, but not those that can only appear after labels. */ + labels is retained. */ gsi_to = gsi_start_bb (dest); - gsi = gsi_start_bb (bb); - gimple_stmt_iterator gsie = gsi_after_labels (bb); - while (gsi_stmt (gsi) != gsi_stmt (gsie)) + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); ) { - tree decl; - label = gsi_stmt (gsi); - if (is_gimple_debug (label) - ? can_move_debug_stmts - : ((decl = gimple_label_label (as_a (label))), - EH_LANDING_PAD_NR (decl) != 0 - || DECL_NONLOCAL (decl) - || FORCED_LABEL (decl) - || !DECL_ARTIFICIAL (decl))) - { - gsi_remove (&gsi, false); - gsi_insert_before (&gsi_to, label, GSI_SAME_STMT); - } + stmt = gsi_stmt (gsi); + if (is_gimple_debug (stmt)) + break; + + /* Forwarder blocks can only contain labels and debug stmts, and + labels must come first, so if we get to this point, we know + we're looking at a label. */ + tree decl = gimple_label_label (as_a (stmt)); + if (EH_LANDING_PAD_NR (decl) != 0 + || DECL_NONLOCAL (decl) + || FORCED_LABEL (decl) + || !DECL_ARTIFICIAL (decl)) + gsi_move_before (&gsi, &gsi_to); else gsi_next (&gsi); } @@ -565,14 +560,12 @@ remove_forwarder_block (basic_block bb) /* Move debug statements if the destination has a single predecessor. */ if (can_move_debug_stmts && !gsi_end_p (gsi)) { - gcc_assert (gsi_stmt (gsi) == gsi_stmt (gsie)); - gimple_stmt_iterator gsie_to = gsi_after_labels (dest); + gsi_to = gsi_after_labels (dest); do { gimple *debug = gsi_stmt (gsi); gcc_assert (is_gimple_debug (debug)); - gsi_remove (&gsi, false); - gsi_insert_before (&gsie_to, debug, GSI_SAME_STMT); + gsi_move_before (&gsi, &gsi_to); } while (!gsi_end_p (gsi)); } diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c index 743ee4e1cf9..fc94f5d83d2 100644 --- a/gcc/tree-ssa-tail-merge.c +++ b/gcc/tree-ssa-tail-merge.c @@ -1295,14 +1295,14 @@ find_duplicate (same_succ *same_succ, basic_block bb1, basic_block bb2) tree label = gimple_label_label (as_a (gsi_stmt (gsi1))); if (DECL_NONLOCAL (label) || FORCED_LABEL (label)) return; - gsi_prev_nondebug (&gsi1); + gsi_prev (&gsi1); } while (!gsi_end_p (gsi2) && gimple_code (gsi_stmt (gsi2)) == GIMPLE_LABEL) { tree label = gimple_label_label (as_a (gsi_stmt (gsi2))); if (DECL_NONLOCAL (label) || FORCED_LABEL (label)) return; - gsi_prev_nondebug (&gsi2); + gsi_prev (&gsi2); } if (!(gsi_end_p (gsi1) && gsi_end_p (gsi2))) return; diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c index c1587593397..77281fb45ee 100644 --- a/gcc/var-tracking.c +++ b/gcc/var-tracking.c @@ -9511,24 +9511,6 @@ emit_notes_in_bb (basic_block bb, dataflow_set *set) } } -/* Return BB's head, unless BB is the block that succeeds ENTRY_BLOCK, - in which case it searches back from BB's head for the very first - insn. Use [get_first_insn (bb), BB_HEAD (bb->next_bb)[ as a range - to iterate over all insns of a function while iterating over its - BBs. */ - -static rtx_insn * -get_first_insn (basic_block bb) -{ - rtx_insn *insn = BB_HEAD (bb); - - if (bb->prev_bb == ENTRY_BLOCK_PTR_FOR_FN (cfun)) - while (rtx_insn *prev = PREV_INSN (insn)) - insn = prev; - - return insn; -} - /* Emit notes for the whole function. */ static void @@ -9557,8 +9539,7 @@ vt_emit_notes (void) { /* Emit the notes for changes of variable locations between two subsequent basic blocks. */ - emit_notes_for_differences (get_first_insn (bb), - &cur, &VTI (bb)->in); + emit_notes_for_differences (BB_HEAD (bb), &cur, &VTI (bb)->in); if (MAY_HAVE_DEBUG_BIND_INSNS) local_get_addr_cache = new hash_map; @@ -9966,7 +9947,7 @@ vt_init_cfa_base (void) /* Reemit INSN, a MARKER_DEBUG_INSN, as a note. */ static rtx_insn * -reemit_marker_as_note (rtx_insn *insn, basic_block *bb) +reemit_marker_as_note (rtx_insn *insn) { gcc_checking_assert (DEBUG_MARKER_INSN_P (insn)); @@ -9981,8 +9962,6 @@ reemit_marker_as_note (rtx_insn *insn, basic_block *bb) { note = emit_note_before (kind, insn); NOTE_MARKER_LOCATION (note) = INSN_LOCATION (insn); - if (bb) - BLOCK_FOR_INSN (note) = *bb; } delete_insn (insn); return note; @@ -10190,39 +10169,11 @@ vt_initialize (void) HOST_WIDE_INT offset = VTI (bb)->out.stack_adjust; VTI (bb)->out.stack_adjust = VTI (bb)->in.stack_adjust; - /* If we are walking the first basic block, walk any HEADER - insns that might be before it too. Unfortunately, - BB_HEADER and BB_FOOTER are not set while we run this - pass. */ rtx_insn *next; - bool outside_bb = true; - for (insn = get_first_insn (bb); insn != BB_HEAD (bb->next_bb); - insn = next) + FOR_BB_INSNS_SAFE (bb, insn, next) { - if (insn == BB_HEAD (bb)) - outside_bb = false; - else if (insn == NEXT_INSN (BB_END (bb))) - outside_bb = true; - next = NEXT_INSN (insn); if (INSN_P (insn)) { - if (outside_bb) - { - /* Ignore non-debug insns outside of basic blocks. */ - if (!DEBUG_INSN_P (insn)) - continue; - /* Debug binds shouldn't appear outside of bbs. */ - gcc_assert (!DEBUG_BIND_INSN_P (insn)); - } - basic_block save_bb = BLOCK_FOR_INSN (insn); - if (!BLOCK_FOR_INSN (insn)) - { - gcc_assert (outside_bb); - BLOCK_FOR_INSN (insn) = bb; - } - else - gcc_assert (BLOCK_FOR_INSN (insn) == bb); - if (!frame_pointer_needed) { insn_stack_adjust_offset_pre_post (insn, &pre, &post); @@ -10244,7 +10195,7 @@ vt_initialize (void) adjust_insn (bb, insn); if (DEBUG_MARKER_INSN_P (insn)) { - insn = reemit_marker_as_note (insn, &save_bb); + reemit_marker_as_note (insn); continue; } @@ -10296,7 +10247,6 @@ vt_initialize (void) } } } - BLOCK_FOR_INSN (insn) = save_bb; } } gcc_assert (offset == VTI (bb)->out.stack_adjust); @@ -10338,15 +10288,12 @@ delete_vta_debug_insns (void) FOR_EACH_BB_FN (bb, cfun) { - for (insn = get_first_insn (bb); - insn != BB_HEAD (bb->next_bb) - ? next = NEXT_INSN (insn), true : false; - insn = next) + FOR_BB_INSNS_SAFE (bb, insn, next) if (DEBUG_INSN_P (insn)) { if (DEBUG_MARKER_INSN_P (insn)) { - insn = reemit_marker_as_note (insn, NULL); + reemit_marker_as_note (insn); continue; } -- 2.30.2