From patchwork Mon Sep 27 09:11:04 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans-Peter Nilsson X-Patchwork-Id: 65822 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 615F7B70E0 for ; Mon, 27 Sep 2010 19:11:20 +1000 (EST) Received: (qmail 28717 invoked by alias); 27 Sep 2010 09:11:18 -0000 Received: (qmail 28698 invoked by uid 22791); 27 Sep 2010 09:11:16 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL, BAYES_00, TW_RR, TW_XT, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from ra.se.axis.com (HELO ra.se.axis.com) (195.60.68.13) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 27 Sep 2010 09:11:10 +0000 Received: from localhost (localhost [127.0.0.1]) by ra.se.axis.com (Postfix) with ESMTP id 6D09A11E53; Mon, 27 Sep 2010 11:11:07 +0200 (CEST) Received: from ra.se.axis.com ([127.0.0.1]) by localhost (ra.se.axis.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id YWZr7BF5Qprm; Mon, 27 Sep 2010 11:11:06 +0200 (CEST) Received: from miranda.se.axis.com (miranda.se.axis.com [10.0.2.171]) by ra.se.axis.com (Postfix) with ESMTPS id 606AD11E50; Mon, 27 Sep 2010 11:11:06 +0200 (CEST) Received: from ignucius.se.axis.com (ignucius.se.axis.com [10.88.21.50]) by miranda.se.axis.com (8.13.4/8.13.4/Debian-3sarge3) with ESMTP id o8R9B6eh002389; Mon, 27 Sep 2010 11:11:06 +0200 Received: from ignucius.se.axis.com (localhost [127.0.0.1]) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) with ESMTP id o8R9B5F6011310; Mon, 27 Sep 2010 11:11:05 +0200 Received: (from hp@localhost) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) id o8R9B4BX011306; Mon, 27 Sep 2010 11:11:04 +0200 Date: Mon, 27 Sep 2010 11:11:04 +0200 Message-Id: <201009270911.o8R9B4BX011306@ignucius.se.axis.com> From: Hans-Peter Nilsson To: bernds@codesourcery.com CC: gcc-patches@gcc.gnu.org In-reply-to: <4C9B26F7.3080000@codesourcery.com> (message from Bernd Schmidt on Thu, 23 Sep 2010 12:07:51 +0200) Subject: Fix PR45792, cris-elf build breakage from PR44374-fix "ifcvt/crossjump patch: Fix PR 42496, 21803" 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 (I'm replying to the recent message regarding the patch.) The title was missing PR44374, I'm not sure whether there is overlap or just a cutnpasto. > Date: Thu, 23 Sep 2010 12:07:51 +0200 > From: Bernd Schmidt > Thanks. Committed with a small fix found while testing on x86_64: In > try_head_merge_bb, in the case where we try to move across multiple > blocks to optimize for a switch statement, the final destination block > must be the only predecessor of the block we're looking at. Revision 164552 broke build for cris-elf, see PR45792. Looking at the patch, I can't see how the "Try again"-codepath (see patch below) can ever work; the retry will try merging the insns starting with the NEXT_INSN after the old, merged, insns. But, the insns are merged, so the old NEXT_INSN is lost; the new NEXT_INSN is now move_before (the compare insn for the cbranch sequence), so you'll try moving the compare and jump to before the jump. Which will cause reorder_insns to get stuck in a loop, after having set PREV_INSN(cmp) = NEXT_INSN (cmp) = cmp... How did that pass testing? Is there something I (we both) miss causing that code-path to never execute for non-cc0 targets? Is the following what you had in mind? I don't like adding more #ifdef HAVE_cc0, but it's either there or in can_move_insns_across, and the latter seems like the worse choice. I'll test this native x86_64 and cross to cris-elf, but perhaps you meant something different altogether so I thought better give early notice. Maybe reconsider and lose the loopness of the do...while(!moveall) loop, as that apparently doesn't happen very often, at least not successfully. :) Perhaps a sanity-check for reorder_insns would is be in order. (With it, I couldn't spot any slowdown for the .52s user time that the test-case compiled, considering that reorder_insns_nobb is now linear in the insn range length, but I didn't check further.) Ok to commit, either or all parts, after testing? gcc/ChangeLog: PR rtl-optimization/45792 * cfgcleanup.c (try_head_merge_bb): New rtx vector nextptr. If not all insns are to be merged, for each edge, stash the NEXT_INSNs after the to-be-merged insns before doing the merge, and use them for the retry at the new insertion point. * emit-rtl.c (reorder_insns_nobb) [ENABLE_CHECKING]: Sanity-check that AFTER is not in the range FROM..TO, inclusive. brgds, H-P --- gcc/cfgcleanup.c~ Sun Sep 26 08:24:42 2010 +++ gcc/cfgcleanup.c Mon Sep 27 07:33:36 2010 @@ -1944,7 +1941,7 @@ try_head_merge_bb (basic_block bb) basic_block final_dest_bb = NULL; int max_match = INT_MAX; edge e0; - rtx *headptr, *currptr; + rtx *headptr, *currptr, *nextptr; bool changed, moveall; unsigned ix; rtx e0_last_head, cond, move_before; @@ -2077,6 +2074,7 @@ try_head_merge_bb (basic_block bb) currptr = XNEWVEC (rtx, nedges); headptr = XNEWVEC (rtx, nedges); + nextptr = XNEWVEC (rtx, nedges); for (ix = 0; ix < nedges; ix++) { @@ -2132,6 +2130,14 @@ try_head_merge_bb (basic_block bb) /* Try again, using a different insertion point. */ move_before = jump; + +#ifdef HAVE_cc0 + /* Don't try moving before a cc0 user, as that may invalidate + the cc0. */ + if (reg_mentioned_p (cc0_rtx, jump)) + break; +#endif + continue; } @@ -2155,6 +2161,12 @@ try_head_merge_bb (basic_block bb) } } + /* If we can't currently move all of the identical insns, remember + each insn after the range that we'll merge. */ + if (!moveall) + for (ix = 0; ix < nedges; ix++) + nextptr[ix] = NEXT_INSN (currptr[ix]); + reorder_insns (headptr[0], currptr[0], PREV_INSN (move_before)); df_set_bb_dirty (EDGE_SUCC (bb, 0)->dest); if (final_dest_bb != NULL) @@ -2170,14 +2182,21 @@ try_head_merge_bb (basic_block bb) if (jump == move_before) break; - /* Try again, using a different insertion point. */ + /* For the unmerged insns, try a different insertion point. */ move_before = jump; + +#ifdef HAVE_cc0 + /* Don't try moving before a cc0 user, as that may invalidate + the cc0. */ + if (reg_mentioned_p (cc0_rtx, jump)) + break; +#endif + for (ix = 0; ix < nedges; ix++) { - rtx curr = currptr[ix]; - do - curr = NEXT_INSN (curr); + rtx curr = nextptr[ix]; while (!NONDEBUG_INSN_P (curr)); + curr = NEXT_INSN (curr); currptr[ix] = headptr[ix] = curr; } } --- gcc/emit-rtl.c~ Sun Sep 26 08:24:42 2010 +++ gcc/emit-rtl.c Mon Sep 27 08:19:20 2010 @@ -3972,16 +3972,23 @@ delete_insns_since (rtx from) AFTER must not be FROM or TO or any insn in between. This function does not know about SEQUENCEs and hence should not be called after delay-slot filling has been done. */ void reorder_insns_nobb (rtx from, rtx to, rtx after) { +#ifdef ENABLE_CHECKING + rtx x; + for (x = from; x != to; x = NEXT_INSN (x)) + gcc_assert (after != x); + gcc_assert (after != to); +#endif + /* Splice this bunch out of where it is now. */ if (PREV_INSN (from)) NEXT_INSN (PREV_INSN (from)) = NEXT_INSN (to); if (NEXT_INSN (to)) PREV_INSN (NEXT_INSN (to)) = PREV_INSN (from); if (get_last_insn () == to) set_last_insn (PREV_INSN (from)); if (get_insns () == from)