From patchwork Tue Sep 25 07:59:54 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bin Cheng X-Patchwork-Id: 186701 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 37C152C0082 for ; Tue, 25 Sep 2012 18:04:48 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1349165088; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: From:To:Cc:References:In-Reply-To:Subject:Date:Message-ID: MIME-Version:Content-Type:Mailing-List:Precedence:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=DJWMnXKTwBz4FJ3axh4PFDO2/WM=; b=QISkdWy+XdwP5Uq oHf/0+pHGHkBRxwdXWIQJ478S1Gzvo/psnXSlb6PvO1kInX9M2CAvx86HRSV+IH2 KjUNupAG3k14G7gtgN29LIsZLfi2z15+Sn+Q+9AeS29Ub9kz2pwnNUnQM2OcTYFl zmWx3ZnqW1zeDEFT2XUCIUE91bOQ= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:From:To:Cc:References:In-Reply-To:Subject:Date:Message-ID:MIME-Version:X-MC-Unique:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=TYPUkgcBwXHv4GNigzyTocKnx5T86BFv9EAhLyHOicT65WExT+I/AWYymW2tZc QpLGUrGIN1rxbYIDnLL097A2M8wvoUBgVX01AiyuJV+GYpuNITPtK5XckI54NaaF Qd+KA5l2bqgr2DdpsMhefC0HQDKdAV0H5oiZd/7VpZJ3Q=; Received: (qmail 10168 invoked by alias); 25 Sep 2012 08:04:36 -0000 Received: (qmail 10138 invoked by uid 22791); 25 Sep 2012 08:04:31 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, MSGID_MULTIPLE_AT, RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 25 Sep 2012 08:04:17 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Tue, 25 Sep 2012 09:04:15 +0100 Received: from Binsh02 ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.0); Tue, 25 Sep 2012 09:04:13 +0100 From: "Bin Cheng" To: "'Richard Sandiford'" Cc: "Ramana Radhakrishnan" , "Richard Earnshaw" , References: <005001cd793a$4e8956e0$eb9c04a0$@cheng@arm.com> <5045d229.8208b40a.1602.ffffde72SMTPIN_ADDED@mx.google.com> <87627t5tll.fsf@talisman.home> In-Reply-To: <87627t5tll.fsf@talisman.home> Subject: RE: [Updated]: [PATCH GCC/ARM] Fix problem that hardreg_cprop opportunities are missed on thumb1 Date: Tue, 25 Sep 2012 15:59:54 +0800 Message-ID: <009601cd9af3$c3049400$490dbc00$@cheng@arm.com> MIME-Version: 1.0 X-MC-Unique: 112092509041507801 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 > -----Original Message----- > From: Richard Sandiford [mailto:rdsandiford@googlemail.com] > Sent: Wednesday, September 05, 2012 6:09 AM > To: Bin Cheng > Cc: Ramana Radhakrishnan; 'Eric Botcazou'; gcc-patches@gcc.gnu.org > Subject: Re: Ping: [PATCH GCC/ARM] Fix problem that hardreg_cprop > opportunities are missed on thumb1 > Subtraction of zero isn't canonical rtl though. Passes after peephole2 would > be well within their rights to simplify the expression back to a move. > From that point of view, making the passes recognise (plus X 0) and (minus X 0) > as special cases would be inconsistent. > > Rather than make the Thumb 1 CC usage implicit in the rtl stream, and carry > the current state around in cfun->machine, it seems like it would be better to > get md_reorg to rewrite the instructions into a form that makes the use of > condition codes explicit. > > md_reorg also sounds like a better place in the pipeline than peephole2 to be > doing this kind of transformation, although I admit I have zero evidence to > back that up... > Hi Richard, This is the updated patch according to your suggestions. I removed the peephole2 patterns and introduced function thumb1_reorg to rewrite instructions in md_reorg pass. In addition to missed propagation, this patch also detects following case: mov r5, r0 str r0, [r4] <-------miscellaneous irrelevant instructions [cmp r0, 0] <-------saved bne .Lxxx Patch tested on arm-none-eabi/cortex-m0, no regressions introduced. Is it OK? Thanks. 2012-09-25 Bin Cheng * config/arm/arm.c (thumb1_reorg): New function. (arm_reorg): Call thumb1_reorg. (thumb1_final_prescan_insn): Record src operand in thumb1_cc_op0. * config/arm/arm.md : Remove peephole2 patterns which rewrites move into subtract of ZERO. Index: gcc/config/arm/arm.c =================================================================== --- gcc/config/arm/arm.c (revision 191088) +++ gcc/config/arm/arm.c (working copy) @@ -13263,6 +13263,62 @@ note_invalid_constants (rtx insn, HOST_WIDE_INT ad return; } +/* Rewrite move insn into subtract of 0 if the condition codes will + be useful in next conditional jump insn. */ + +static void +thumb1_reorg (void) +{ + basic_block bb; + + FOR_EACH_BB (bb) + { + rtx set, dest, src; + rtx pat, op0; + rtx prev, insn = BB_END (bb); + + while (insn != BB_HEAD (bb) && DEBUG_INSN_P (insn)) + insn = PREV_INSN (insn); + + /* Find the last cbranchsi4_insn in basic block BB. */ + if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn) + continue; + + /* Find the first non-note insn before INSN in basic block BB. */ + gcc_assert (insn != BB_HEAD (bb)); + prev = PREV_INSN (insn); + while (prev != BB_HEAD (bb) && (NOTE_P (prev) || DEBUG_INSN_P (prev))) + prev = PREV_INSN (prev); + + set = single_set (prev); + if (!set) + continue; + + dest = SET_DEST (set); + src = SET_SRC (set); + if (!low_register_operand (dest, SImode) + || !low_register_operand (src, SImode)) + continue; + + pat = PATTERN (insn); + op0 = XEXP (XEXP (SET_SRC (pat), 0), 0); + /* Rewrite move into subtract of 0 if its operand is compared with ZERO + in INSN. Don't need to check dest since cprop_hardreg pass propagates + src into INSN. */ + if (REGNO (op0) == REGNO (src)) + { + dest = copy_rtx (dest); + src = copy_rtx (src); + src = gen_rtx_MINUS (SImode, src, const0_rtx); + PATTERN (prev) = gen_rtx_SET (VOIDmode, dest, src); + INSN_CODE (prev) = -1; + /* Set test register in INSN to dest. */ + XEXP (XEXP (SET_SRC (pat), 0), 0) = copy_rtx (dest); + INSN_CODE (insn) = -1; + } + } +} + /* Convert instructions to their cc-clobbering variant if possible, since that allows us to use smaller encodings. */ @@ -13459,7 +13515,9 @@ arm_reorg (void) HOST_WIDE_INT address = 0; Mfix * fix; - if (TARGET_THUMB2) + if (TARGET_THUMB1) + thumb1_reorg (); + else if (TARGET_THUMB2) thumb2_reorg (); /* Ensure all insns that must be split have been split at this point. @@ -21736,6 +21794,12 @@ thumb1_final_prescan_insn (rtx insn) if (src1 == const0_rtx) cfun->machine->thumb1_cc_mode = CCmode; } + else if (REG_P (SET_DEST (set)) && REG_P (SET_SRC (set))) + { + /* Record the src register operand instead of dest because + cprop_hardreg pass propagates src. */ + cfun->machine->thumb1_cc_op0 = SET_SRC (set); + } } else if (conds != CONDS_NOCOND) cfun->machine->thumb1_cc_insn = NULL_RTX; Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md (revision 191088) +++ gcc/config/arm/arm.md (working copy) @@ -7102,43 +7102,6 @@ (const_int 8))))] ) -;; Two peepholes to generate subtract of 0 instead of a move if the -;; condition codes will be useful. -(define_peephole2 - [(set (match_operand:SI 0 "low_register_operand" "") - (match_operand:SI 1 "low_register_operand" "")) - (set (pc) - (if_then_else (match_operator 2 "arm_comparison_operator" - [(match_dup 1) (const_int 0)]) - (label_ref (match_operand 3 "" "")) - (pc)))] - "TARGET_THUMB1" - [(set (match_dup 0) (minus:SI (match_dup 1) (const_int 0))) - (set (pc) - (if_then_else (match_op_dup 2 [(match_dup 0) (const_int 0)]) - (label_ref (match_dup 3)) - (pc)))] - "") - -;; Sigh! This variant shouldn't be needed, but combine often fails to -;; merge cases like this because the op1 is a hard register in -;; arm_class_likely_spilled_p. -(define_peephole2 - [(set (match_operand:SI 0 "low_register_operand" "") - (match_operand:SI 1 "low_register_operand" "")) - (set (pc) - (if_then_else (match_operator 2 "arm_comparison_operator" - [(match_dup 0) (const_int 0)]) - (label_ref (match_operand 3 "" "")) - (pc)))] - "TARGET_THUMB1" - [(set (match_dup 0) (minus:SI (match_dup 1) (const_int 0))) - (set (pc) - (if_then_else (match_op_dup 2 [(match_dup 0) (const_int 0)]) - (label_ref (match_dup 3)) - (pc)))] - "") - (define_insn "*negated_cbranchsi4" [(set (pc) (if_then_else