From patchwork Tue Nov 30 06:41:13 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [PR,debug/45136] revamp handling of BB boundary debug insns in sched Date: Mon, 29 Nov 2010 20:41:13 -0000 From: Alexandre Oliva X-Patchwork-Id: 73565 Message-Id: To: gcc-patches@gcc.gnu.org On Sep 4, 2010, Alexandre Oliva wrote: > We used to mark and skip debug insns at beginning and end of basic > blocks during sched initialization, so as to leave them alone along with > any notes that might appear between them and non-debug insns, because in > a compilation without debug insns, the notes would remain in place for > being outside the blocks. > This worked pretty well, except when multi-block sched happened to pull > an insn from a later block, causing a debug insns (and notes between > them and other insns) to become boundary insns. We wouldn't > re-recognize them as boundary insns or notes, so we'd end up moving the > notes about, whereas the non-debug compilation would leave them in > place, causing -fcompare-debug differences. > This patch does away with the notion of boundary debug insns and notes. > Instead, whenever we're skip boundary notes and we find debug insns, we > search further for notes and move them past the debug insns, so that > they're left alone by the scheduler, as they would in a non-debug > compilation. > There are two catches: > - if notes ever become meaningful among debug insns, this will kind of > change their meaning. I suppose we can cross that bridge when we get to > it. > - if there's a long chain of debug insns at the beginning or end of a > block, we'll iterate over all of them looking for notes every time we > call get_ebb_head_tail for such blocks. I'm not sure this could become > a performance issue, and I don't quite see how to avoid checking it > every time, considering that the status of a debug insn may change from > one call to another, without changes to a previously checked boundary > debug insn, e.g.: > (debug_insn 51 ... > (note 52 ... > (insn 53 ... > (debug_insn 55 ... > (note 54 ... > end of block > say the patch moved note 54 past debug_insn 55, so debug_insn 55 is > regarded as the scheduling boundary. Then, sched moves insn 53 to an > earlier block. Now, we have to move note 52 past debug_insn 55, but if > we optimized the search at the end of the block to stop at debug_insn > 55, we'd miss the change of status of note 52 and debug_insn 51. > Thoughts? > This patch was regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu. > Ok to install? This no longer bootstrapped recently. The symptom was moving a NOTE_INSN_FUNCTION_BEGIN out of block 2, so that the debug dumps would no longer match the debug dumps, in which the note was still at the end of block 2. I couldn't quite figure out why in some cases notes are moved out of blocks, and in other cases they aren't, so I ended up just punting at it, removing block numbers from all notes before the final dump. Better suggestions are welcome. This was again regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu. Ok to install? for gcc/ChangeLog from Alexandre Oliva PR debug/45136 PR debug/45130 * haifa-sched.c (get_ebb_head_tail): Move notes across boundary debug insns. (no_real_insns_p, schedule_block, set_priorities): Drop special treatment of boundary debug insns. * sched-deps.c (sd_init_insn, sd_finish_insn): Don't mark debug insns. * sched-ebb.c (schedule_ebbs): Don't skip debug insns. * sched-int.h (DEBUG_INSN_SCHED_P): Remove. (BOUNDARY_DEBUG_INSN_P): Likewise. (SCHEDULE_DEBUG_INSN_P): Likewise. * sched-rgn.c (init_ready_list): Drop special treatment of boundary debug insns. * final.c (rest_of_clean-state): Clear notes' BB. Index: gcc/haifa-sched.c =================================================================== --- gcc/haifa-sched.c.orig 2010-11-26 05:37:55.143932457 -0200 +++ gcc/haifa-sched.c 2010-11-26 18:10:21.300655351 -0200 @@ -1901,8 +1901,30 @@ get_ebb_head_tail (basic_block beg, basi beg_head = NEXT_INSN (beg_head); while (beg_head != beg_tail) - if (NOTE_P (beg_head) || BOUNDARY_DEBUG_INSN_P (beg_head)) + if (NOTE_P (beg_head)) beg_head = NEXT_INSN (beg_head); + else if (DEBUG_INSN_P (beg_head)) + { + rtx note, next; + + for (note = NEXT_INSN (beg_head); + note != beg_tail; + note = next) + { + next = NEXT_INSN (note); + if (NOTE_P (note)) + { + if (sched_verbose >= 9) + fprintf (sched_dump, "reorder %i\n", INSN_UID (note)); + + reorder_insns_nobb (note, note, PREV_INSN (beg_head)); + } + else if (!DEBUG_INSN_P (note)) + break; + } + + break; + } else break; @@ -1914,8 +1936,33 @@ get_ebb_head_tail (basic_block beg, basi end_head = NEXT_INSN (end_head); while (end_head != end_tail) - if (NOTE_P (end_tail) || BOUNDARY_DEBUG_INSN_P (end_tail)) + if (NOTE_P (end_tail)) end_tail = PREV_INSN (end_tail); + else if (DEBUG_INSN_P (end_tail)) + { + rtx note, prev; + + for (note = PREV_INSN (end_tail); + note != end_head; + note = prev) + { + prev = PREV_INSN (note); + if (NOTE_P (note)) + { + if (sched_verbose >= 9) + fprintf (sched_dump, "reorder %i\n", INSN_UID (note)); + + reorder_insns_nobb (note, note, end_tail); + + if (end_tail == BB_END (end)) + df_insn_change_bb (note, NULL); + } + else if (!DEBUG_INSN_P (note)) + break; + } + + break; + } else break; @@ -1929,8 +1976,7 @@ no_real_insns_p (const_rtx head, const_r { while (head != NEXT_INSN (tail)) { - if (!NOTE_P (head) && !LABEL_P (head) - && !BOUNDARY_DEBUG_INSN_P (head)) + if (!NOTE_P (head) && !LABEL_P (head)) return 0; head = NEXT_INSN (head); } @@ -2813,7 +2859,7 @@ schedule_block (basic_block *target_bb) last_scheduled_insn = prev_head; gcc_assert ((NOTE_P (last_scheduled_insn) - || BOUNDARY_DEBUG_INSN_P (last_scheduled_insn)) + || DEBUG_INSN_P (last_scheduled_insn)) && BLOCK_FOR_INSN (last_scheduled_insn) == *target_bb); /* Initialize INSN_QUEUE. Q_SIZE is the total number of insns in the @@ -3320,7 +3366,7 @@ set_priorities (rtx head, rtx tail) current_sched_info->sched_max_insns_priority; rtx prev_head; - if (head == tail && (! INSN_P (head) || BOUNDARY_DEBUG_INSN_P (head))) + if (head == tail && ! INSN_P (head)) gcc_unreachable (); n_insn = 0; Index: gcc/sched-deps.c =================================================================== --- gcc/sched-deps.c.orig 2010-11-26 05:37:55.142932376 -0200 +++ gcc/sched-deps.c 2010-11-26 16:27:26.110757202 -0200 @@ -717,9 +717,6 @@ sd_init_insn (rtx insn) INSN_FORW_DEPS (insn) = create_deps_list (); INSN_RESOLVED_FORW_DEPS (insn) = create_deps_list (); - if (DEBUG_INSN_P (insn)) - DEBUG_INSN_SCHED_P (insn) = TRUE; - /* ??? It would be nice to allocate dependency caches here. */ } @@ -729,12 +726,6 @@ sd_finish_insn (rtx insn) { /* ??? It would be nice to deallocate dependency caches here. */ - if (DEBUG_INSN_P (insn)) - { - gcc_assert (DEBUG_INSN_SCHED_P (insn)); - DEBUG_INSN_SCHED_P (insn) = FALSE; - } - free_deps_list (INSN_HARD_BACK_DEPS (insn)); INSN_HARD_BACK_DEPS (insn) = NULL; Index: gcc/sched-ebb.c =================================================================== --- gcc/sched-ebb.c.orig 2010-11-26 05:37:55.142932376 -0200 +++ gcc/sched-ebb.c 2010-11-26 16:27:26.198764286 -0200 @@ -600,9 +600,9 @@ schedule_ebbs (void) a note or two. */ while (head != tail) { - if (NOTE_P (head) || BOUNDARY_DEBUG_INSN_P (head)) + if (NOTE_P (head)) head = NEXT_INSN (head); - else if (NOTE_P (tail) || BOUNDARY_DEBUG_INSN_P (tail)) + else if (NOTE_P (tail)) tail = PREV_INSN (tail); else if (LABEL_P (head)) head = NEXT_INSN (head); Index: gcc/sched-int.h =================================================================== --- gcc/sched-int.h.orig 2010-11-26 05:37:55.143932457 -0200 +++ gcc/sched-int.h 2010-11-26 16:27:26.267769840 -0200 @@ -887,23 +887,6 @@ extern VEC(haifa_deps_insn_data_def, hea #define IS_SPECULATION_BRANCHY_CHECK_P(INSN) \ (RECOVERY_BLOCK (INSN) != NULL && RECOVERY_BLOCK (INSN) != EXIT_BLOCK_PTR) -/* The unchanging bit tracks whether a debug insn is to be handled - like an insn (i.e., schedule it) or like a note (e.g., it is next - to a basic block boundary. */ -#define DEBUG_INSN_SCHED_P(insn) \ - (RTL_FLAG_CHECK1("DEBUG_INSN_SCHED_P", (insn), DEBUG_INSN)->unchanging) - -/* True if INSN is a debug insn that is next to a basic block - boundary, i.e., it is to be handled by the scheduler like a - note. */ -#define BOUNDARY_DEBUG_INSN_P(insn) \ - (DEBUG_INSN_P (insn) && !DEBUG_INSN_SCHED_P (insn)) -/* True if INSN is a debug insn that is not next to a basic block - boundary, i.e., it is to be handled by the scheduler like an - insn. */ -#define SCHEDULE_DEBUG_INSN_P(insn) \ - (DEBUG_INSN_P (insn) && DEBUG_INSN_SCHED_P (insn)) - /* Dep status (aka ds_t) of the link encapsulates information, that is needed for speculative scheduling. Namely, it is 4 integers in the range [0, MAX_DEP_WEAK] and 3 bits. Index: gcc/sched-rgn.c =================================================================== --- gcc/sched-rgn.c.orig 2010-11-26 05:37:55.143932457 -0200 +++ gcc/sched-rgn.c 2010-11-26 16:27:26.383779177 -0200 @@ -2140,7 +2140,7 @@ init_ready_list (void) src_head = head; for (insn = src_head; insn != src_next_tail; insn = NEXT_INSN (insn)) - if (INSN_P (insn) && !BOUNDARY_DEBUG_INSN_P (insn)) + if (INSN_P (insn)) try_ready (insn); } } Index: gcc/final.c =================================================================== --- gcc/final.c.orig 2010-11-26 05:37:55.142932376 -0200 +++ gcc/final.c 2010-11-26 16:27:26.729807027 -0200 @@ -4366,7 +4366,11 @@ rest_of_clean_state (void) if (LABEL_P (insn)) INSN_UID (insn) = CODE_LABEL_NUMBER (insn); else - INSN_UID (insn) = 0; + { + if (NOTE_P (insn)) + set_block_for_insn (insn, NULL); + INSN_UID (insn) = 0; + } } } @@ -4387,7 +4391,6 @@ rest_of_clean_state (void) && NOTE_KIND (insn) != NOTE_INSN_BLOCK_END && NOTE_KIND (insn) != NOTE_INSN_CFA_RESTORE_STATE))) print_rtl_single (final_output, insn); - } if (final_output)