From patchwork Fri Jul 22 23:47:34 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Henderson X-Patchwork-Id: 106402 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 6523FB6F6F for ; Sat, 23 Jul 2011 09:47:57 +1000 (EST) Received: (qmail 29456 invoked by alias); 22 Jul 2011 23:47:55 -0000 Received: (qmail 29446 invoked by uid 22791); 22 Jul 2011 23:47:53 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, SPF_HELO_PASS, TW_CF 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; Fri, 22 Jul 2011 23:47:36 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p6MNlZE2026949 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 22 Jul 2011 19:47:35 -0400 Received: from anchor.twiddle.net (vpn-227-82.phx2.redhat.com [10.3.227.82]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p6MNlZUw018633 for ; Fri, 22 Jul 2011 19:47:35 -0400 Message-ID: <4E2A0C16.30700@redhat.com> Date: Fri, 22 Jul 2011 16:47:34 -0700 From: Richard Henderson User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.18) Gecko/20110621 Fedora/3.1.11-1.fc15 Thunderbird/3.1.11 MIME-Version: 1.0 To: GCC Patches Subject: Fix mark_all_labels vs cfglayout mode X-IsSubscribed: yes 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 I re-tested my dwarf2 cfi propagation pass today, with the partitioning vs eh edge splitting fix included. Only this time I tested languages=all,ada,go instead of just =c,c++. Lo and behold I find another latent problem, this time via a Fortran test case. The dwarf2 cfg pass is set to abort for unreachable blocks, and gfortran.dg/debug/pr46756.f found such a case. The test case contains a computed goto. Except that during combine, we (incorrectly) filled in the JUMP_LABEL of that jump insn. This, by definition, means it is not computed_jump_p. Which means that when it came to pass_dwarf2, several blocks that ought to have been reachable via forced_labels weren't. During normal optimization we don't really care about JUMP_LABEL because we rely on the CFG. And I guess nothing else really cared either, since somehow the test case worked. We should probably have a check for this in rtl_verify_flow_info... The incorrect substitution in combine happened because mark_all_labels scans extended basic blocks the old skool way, by looking for labels. Except that in cfglayout mode we don't bother with unimportant bits like labels, so m_a_l got the wrong answer. If we admit that something as simplistic as constant propagation between adjacent instructions is something that's actually useful in the specific context of being in cfglayout mode while lots of other optimizers are running, then, really, there's other work that should be done when this substitution is made. Namely, clean up the dead CFG edges. However, I do not in fact agree with the preceding predicate. And a bootstrap and test run with a well-placed assertion agree: this is not something that ever happens. Which makes the resulting code much tidier. Tested on x86_64-linux. Committed. r~ * jump.c (maybe_propagate_label_ref): Split out of... (mark_all_labels): ... here. Do not attempt label_ref propagation while in cfglayout mode. diff --git a/gcc/jump.c b/gcc/jump.c index f337eb3..0d29f0f 100644 --- a/gcc/jump.c +++ b/gcc/jump.c @@ -200,6 +200,54 @@ init_label_info (rtx f) } } +/* A subroutine of mark_all_labels. Trivially propagate a simple label + load into a jump_insn that uses it. */ + +static void +maybe_propagate_label_ref (rtx jump_insn, rtx prev_nonjump_insn) +{ + rtx label_note, pc, pc_src; + + pc = pc_set (jump_insn); + pc_src = pc != NULL ? SET_SRC (pc) : NULL; + label_note = find_reg_note (prev_nonjump_insn, REG_LABEL_OPERAND, NULL); + + /* If the previous non-jump insn sets something to a label, + something that this jump insn uses, make that label the primary + target of this insn if we don't yet have any. That previous + insn must be a single_set and not refer to more than one label. + The jump insn must not refer to other labels as jump targets + and must be a plain (set (pc) ...), maybe in a parallel, and + may refer to the item being set only directly or as one of the + arms in an IF_THEN_ELSE. */ + + if (label_note != NULL && pc_src != NULL) + { + rtx label_set = single_set (prev_nonjump_insn); + rtx label_dest = label_set != NULL ? SET_DEST (label_set) : NULL; + + if (label_set != NULL + /* The source must be the direct LABEL_REF, not a + PLUS, UNSPEC, IF_THEN_ELSE etc. */ + && GET_CODE (SET_SRC (label_set)) == LABEL_REF + && (rtx_equal_p (label_dest, pc_src) + || (GET_CODE (pc_src) == IF_THEN_ELSE + && (rtx_equal_p (label_dest, XEXP (pc_src, 1)) + || rtx_equal_p (label_dest, XEXP (pc_src, 2)))))) + { + /* The CODE_LABEL referred to in the note must be the + CODE_LABEL in the LABEL_REF of the "set". We can + conveniently use it for the marker function, which + requires a LABEL_REF wrapping. */ + gcc_assert (XEXP (label_note, 0) == XEXP (SET_SRC (label_set), 0)); + + mark_jump_label_1 (label_set, jump_insn, false, true); + + gcc_assert (JUMP_LABEL (jump_insn) == XEXP (label_note, 0)); + } + } +} + /* Mark the label each jump jumps to. Combine consecutive labels, and count uses of labels. */ @@ -207,85 +255,31 @@ static void mark_all_labels (rtx f) { rtx insn; - rtx prev_nonjump_insn = NULL; - - for (insn = f; insn; insn = NEXT_INSN (insn)) - if (NONDEBUG_INSN_P (insn)) - { - mark_jump_label (PATTERN (insn), insn, 0); - - /* If the previous non-jump insn sets something to a label, - something that this jump insn uses, make that label the primary - target of this insn if we don't yet have any. That previous - insn must be a single_set and not refer to more than one label. - The jump insn must not refer to other labels as jump targets - and must be a plain (set (pc) ...), maybe in a parallel, and - may refer to the item being set only directly or as one of the - arms in an IF_THEN_ELSE. */ - if (! INSN_DELETED_P (insn) - && JUMP_P (insn) - && JUMP_LABEL (insn) == NULL) - { - rtx label_note = NULL; - rtx pc = pc_set (insn); - rtx pc_src = pc != NULL ? SET_SRC (pc) : NULL; - - if (prev_nonjump_insn != NULL) - label_note - = find_reg_note (prev_nonjump_insn, REG_LABEL_OPERAND, NULL); - - if (label_note != NULL && pc_src != NULL) - { - rtx label_set = single_set (prev_nonjump_insn); - rtx label_dest - = label_set != NULL ? SET_DEST (label_set) : NULL; - - if (label_set != NULL - /* The source must be the direct LABEL_REF, not a - PLUS, UNSPEC, IF_THEN_ELSE etc. */ - && GET_CODE (SET_SRC (label_set)) == LABEL_REF - && (rtx_equal_p (label_dest, pc_src) - || (GET_CODE (pc_src) == IF_THEN_ELSE - && (rtx_equal_p (label_dest, XEXP (pc_src, 1)) - || rtx_equal_p (label_dest, - XEXP (pc_src, 2)))))) - - { - /* The CODE_LABEL referred to in the note must be the - CODE_LABEL in the LABEL_REF of the "set". We can - conveniently use it for the marker function, which - requires a LABEL_REF wrapping. */ - gcc_assert (XEXP (label_note, 0) - == XEXP (SET_SRC (label_set), 0)); - - mark_jump_label_1 (label_set, insn, false, true); - gcc_assert (JUMP_LABEL (insn) - == XEXP (SET_SRC (label_set), 0)); - } - } - } - else if (! INSN_DELETED_P (insn)) - prev_nonjump_insn = insn; - } - else if (LABEL_P (insn)) - prev_nonjump_insn = NULL; - /* If we are in cfglayout mode, there may be non-insns between the - basic blocks. If those non-insns represent tablejump data, they - contain label references that we must record. */ if (current_ir_type () == IR_RTL_CFGLAYOUT) { basic_block bb; - rtx insn; FOR_EACH_BB (bb) { + /* In cfglayout mode, we don't bother with trivial next-insn + propagation of LABEL_REFs into JUMP_LABEL. This will be + handled by other optimizers using better algorithms. */ + FOR_BB_INSNS (bb, insn) + { + gcc_assert (! INSN_DELETED_P (insn)); + if (NONDEBUG_INSN_P (insn)) + mark_jump_label (PATTERN (insn), insn, 0); + } + + /* In cfglayout mode, there may be non-insns between the + basic blocks. If those non-insns represent tablejump data, + they contain label references that we must record. */ for (insn = bb->il.rtl->header; insn; insn = NEXT_INSN (insn)) if (INSN_P (insn)) { gcc_assert (JUMP_TABLE_DATA_P (insn)); mark_jump_label (PATTERN (insn), insn, 0); } - for (insn = bb->il.rtl->footer; insn; insn = NEXT_INSN (insn)) if (INSN_P (insn)) { @@ -294,6 +288,28 @@ mark_all_labels (rtx f) } } } + else + { + rtx prev_nonjump_insn = NULL; + for (insn = f; insn; insn = NEXT_INSN (insn)) + { + if (INSN_DELETED_P (insn)) + ; + else if (LABEL_P (insn)) + prev_nonjump_insn = NULL; + else if (NONDEBUG_INSN_P (insn)) + { + mark_jump_label (PATTERN (insn), insn, 0); + if (JUMP_P (insn)) + { + if (JUMP_LABEL (insn) == NULL && prev_nonjump_insn != NULL) + maybe_propagate_label_ref (insn, prev_nonjump_insn); + } + else + prev_nonjump_insn = insn; + } + } + } } /* Given a comparison (CODE ARG0 ARG1), inside an insn, INSN, return a code