From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30281 invoked by alias); 24 Apr 2015 03:15:00 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 30264 invoked by uid 89); 24 Apr 2015 03:14:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 24 Apr 2015 03:14:58 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 50808AC7A8; Fri, 24 Apr 2015 03:14:57 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-113.phx2.redhat.com [10.3.113.113]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3O3EuNO020214; Thu, 23 Apr 2015 23:14:56 -0400 Message-ID: <5539B530.4030803@redhat.com> Date: Fri, 24 Apr 2015 03:15:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: "Thomas Preud'homme" , Steven Bosscher , gcc-patches@gcc.gnu.org, "'Richard Biener'" Subject: Re: [PATCH, GCC, stage1] Fallback to copy-prop if constant-prop not possible References: <000501d049d3$079385a0$16ba90e0$@arm.com> <552BBAF9.2010504@redhat.com> <000001d07821$6fb82f60$4f288e20$@arm.com> <5539B177.8020705@redhat.com> <000501d07e3c$30f53c20$92dfb460$@arm.com> In-Reply-To: <000501d07e3c$30f53c20$92dfb460$@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg01481.txt.bz2 On 04/23/2015 09:10 PM, Thomas Preud'homme wrote: >> From: Jeff Law [mailto:law@redhat.com] >> Sent: Friday, April 24, 2015 10:59 AM >> > > Hi Jeff, > >>> + >>> +static bool >>> +cprop_reg_p (const_rtx x) >>> +{ >>> + return REG_P (x) && !HARD_REGISTER_P (x); >>> +} >> How about instead this move to a more visible location (perhaps a macro >> in regs.h or an inline function). Then as a followup, change the >> various places that have this sequence to use that common definition >> that exist outside of cprop.c. > > According to Steven this was proposed in the past but was refused (see > end of [1]). > > [1] https://gcc.gnu.org/ml/gcc-patches/2015-03/msg01066.html Missed that message. Given we've already gone round and round on it, let go with the patch as-is and deal with hard_register_p and pseudo_register_p independently. No idea who objected to that, seems like a no-brainer to me. > >> >>> @@ -1191,7 +1192,7 @@ do_local_cprop (rtx x, rtx_insn *insn) >>> /* Rule out USE instructions and ASM statements as we don't want >> to >>> change the hard registers mentioned. */ >>> if (REG_P (x) >>> - && (REGNO (x) >= FIRST_PSEUDO_REGISTER >>> + && (cprop_reg_p (x) >>> || (GET_CODE (PATTERN (insn)) != USE >>> && asm_noperands (PATTERN (insn)) < 0))) >> Isn't the REG_P test now redundant? > > I made the same mistake when reviewing that change and indeed it's not. > Note the opening parenthesis before cprop_reg_p that contains a bitwise > OR expression. So in the case where cprop_reg_p is false, REG_P still > needs to be true. > > We could keep a check on FIRST_PSEUDO_REGISTER but the intent (checking > that the register is suitable for propagation) is clearer now, as pointed out by > Steven to me. Ah. Nevermind then. So revised review is "ok for the trunk" :-) jeff