From patchwork Wed Jan 19 22:26:48 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 79607 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id AE7A7B70E4 for ; Thu, 20 Jan 2011 09:27:11 +1100 (EST) Received: (qmail 12002 invoked by alias); 19 Jan 2011 22:27:06 -0000 Received: (qmail 11970 invoked by uid 22791); 19 Jan 2011 22:27:00 -0000 X-SWARE-Spam-Status: No, hits=-5.5 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 19 Jan 2011 22:26:53 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id p0JMQqIu032253 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 19 Jan 2011 17:26:52 -0500 Received: from freie.oliva.athome.lsd.ic.unicamp.br (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p0JMQoRl029271 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 19 Jan 2011 17:26:51 -0500 Received: from livre.localdomain (livre-to-gw.oliva.athome.lsd.ic.unicamp.br [172.31.160.19]) by freie.oliva.athome.lsd.ic.unicamp.br (8.14.4/8.14.4) with ESMTP id p0JMQnNf016114 for ; Wed, 19 Jan 2011 20:26:49 -0200 Received: from livre.localdomain (aoliva@localhost [127.0.0.1]) by livre.localdomain (8.14.3/8.14.3/Debian-5+lenny1) with ESMTP id p0JMQnZ9024611; Wed, 19 Jan 2011 20:26:49 -0200 Received: (from aoliva@localhost) by livre.localdomain (8.14.3/8.14.3/Submit) id p0JMQmkA024609; Wed, 19 Jan 2011 20:26:48 -0200 From: Alexandre Oliva To: gcc-patches@gcc.gnu.org Subject: Re: [PR debug/45136] revamp handling of BB boundary debug insns in sched References: Date: Wed, 19 Jan 2011 20:26:48 -0200 In-Reply-To: (Alexandre Oliva's message of "Tue, 30 Nov 2010 04:41:13 -0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Ping^2? On Nov 30, 2010, Alexandre Oliva wrote: > 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)