From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21382 invoked by alias); 19 Nov 2014 09:59:18 -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 21371 invoked by uid 89); 19 Nov 2014 09:59:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: foss-mx-na.foss.arm.com Received: from foss-mx-na.foss.arm.com (HELO foss-mx-na.foss.arm.com) (217.140.108.86) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Nov 2014 09:59:16 +0000 Received: from foss-smtp-na-1.foss.arm.com (unknown [10.80.61.8]) by foss-mx-na.foss.arm.com (Postfix) with ESMTP id CB7B4220; Wed, 19 Nov 2014 03:59:13 -0600 (CST) Received: from collaborate-mta1.arm.com (highbank-bc01-b06.austin.arm.com [10.112.81.134]) by foss-smtp-na-1.foss.arm.com (Postfix) with ESMTP id AA0AD5FAD7; Wed, 19 Nov 2014 03:59:10 -0600 (CST) Received: from [10.1.209.40] (e105545-lin.cambridge.arm.com [10.1.209.40]) by collaborate-mta1.arm.com (Postfix) with ESMTPS id E4AC213F6FA; Wed, 19 Nov 2014 03:59:09 -0600 (CST) Message-ID: <546C69ED.7050803@arm.com> Date: Wed, 19 Nov 2014 10:26:00 -0000 From: Ramana Radhakrishnan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: "Yangfei (Felix)" , "gcc-patches@gcc.gnu.org" CC: Chenshanyao Subject: Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian References: <545A039D.104@arm.com> <546B053E.7090503@arm.com> <546B34A5.2030602@arm.com> <546B3FE8.30907@arm.com> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-11/txt/msg02406.txt.bz2 On 19/11/14 09:29, Yangfei (Felix) wrote: >>> Sorry for missing the point. It seems to me that 't2' here will conflict with >> condition of the pattern *movhi_insn_arch4: >>> "TARGET_ARM >>> && arm_arch4 >>> && (register_operand (operands[0], HImode) >>> || register_operand (operands[1], HImode))" >>> >>> #define TARGET_ARM (! TARGET_THUMB) >>> /* 32-bit Thumb-2 code. */ >>> #define TARGET_THUMB2 (TARGET_THUMB && >> arm_arch_thumb2) >>> >> >> Bah, Indeed ! - I misremembered the t2 there, my mistake. >> >> Yes you are right there, but what I'd like you to do is to use that mechanism >> rather than putting all this logic in the predicate. >> >> So, I'd prefer you to add a v6t2 to the values for the "arch" attribute, don't forget >> to update the comments above. >> >> and in arch_enabled you need to enforce this with >> >> (and (eq_attr "arch" "v6t2") >> (match_test "TARGET_32BIT && arm_arch6 && arm_arch_thumb2")) >> (const_string "yes") >> >> And in the pattern use v6t2 ... >> >> arm_arch_thumb2 implies that this is at the architecture level of v6t2. >> Therefore TARGET_ARM && arm_arch_thumb2 implies ARM state. > > > Hi Ramana, > Thank you for your suggestions. I rebased the patch on the latest trunk and updated it accordingly. > As this patch will not work for architectures older than armv6t2, I also prefer Thomas's patch to fix for them. > I am currently performing test for this patch. Assuming no issues pops up, OK for the trunk? > And is it necessary to backport this patch to the 4.8 & 4.9 branches? This is OK for trunk if no regressions - please let it bake for a few days on trunk to let auto-testers catch up. This is OK for 4.8 / 4.9 after that and please test your backport. regards Ramana > > > Index: gcc/ChangeLog > =================================================================== > --- gcc/ChangeLog (revision 217717) > +++ gcc/ChangeLog (working copy) > @@ -1,3 +1,11 @@ > +2014-11-19 Felix Yang > + Shanyao Chen > + > + PR target/59593 > + * config/arm/arm.md (define_attr "arch"): Add v6t2. > + (define_attr "arch_enabled"): Add test for the above. > + (*movhi_insn_arch4): Add new alternative. > + > 2014-11-18 Felix Yang > > * config/aarch64/aarch64.c (doloop_end): New pattern. > Index: gcc/config/arm/arm.md > =================================================================== > --- gcc/config/arm/arm.md (revision 217717) > +++ gcc/config/arm/arm.md (working copy) > @@ -125,9 +125,10 @@ > ; This can be "a" for ARM, "t" for either of the Thumbs, "32" for > ; TARGET_32BIT, "t1" or "t2" to specify a specific Thumb mode. "v6" > ; for ARM or Thumb-2 with arm_arch6, and nov6 for ARM without > -; arm_arch6. This attribute is used to compute attribute "enabled", > -; use type "any" to enable an alternative in all cases. > -(define_attr "arch" "any,a,t,32,t1,t2,v6,nov6,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3" > +; arm_arch6. "v6t2" for Thumb-2 with arm_arch6. This attribute is > +; used to compute attribute "enabled", use type "any" to enable an > +; alternative in all cases. > +(define_attr "arch" "any,a,t,32,t1,t2,v6,nov6,v6t2,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3" > (const_string "any")) > > (define_attr "arch_enabled" "no,yes" > @@ -162,6 +163,10 @@ > (match_test "TARGET_32BIT && !arm_arch6")) > (const_string "yes") > > + (and (eq_attr "arch" "v6t2") > + (match_test "TARGET_32BIT && arm_arch6 && arm_arch_thumb2")) > + (const_string "yes") > + > (and (eq_attr "arch" "avoid_neon_for_64bits") > (match_test "TARGET_NEON") > (not (match_test "TARGET_PREFER_NEON_64BITS"))) > @@ -6288,8 +6293,8 @@ > > ;; Pattern to recognize insn generated default case above > (define_insn "*movhi_insn_arch4" > - [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,m,r") > - (match_operand:HI 1 "general_operand" "rIk,K,r,mi"))] > + [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,r,m,r") > + (match_operand:HI 1 "general_operand" "rIk,K,n,r,mi"))] > "TARGET_ARM > && arm_arch4 > && (register_operand (operands[0], HImode) > @@ -6297,16 +6302,19 @@ > "@ > mov%?\\t%0, %1\\t%@ movhi > mvn%?\\t%0, #%B1\\t%@ movhi > + movw%?\\t%0, %1\\t%@ movhi > str%(h%)\\t%1, %0\\t%@ movhi > ldr%(h%)\\t%0, %1\\t%@ movhi" > [(set_attr "predicable" "yes") > - (set_attr "pool_range" "*,*,*,256") > - (set_attr "neg_pool_range" "*,*,*,244") > + (set_attr "pool_range" "*,*,*,*,256") > + (set_attr "neg_pool_range" "*,*,*,*,244") > + (set_attr "arch" "*,*,v6t2,*,*") > (set_attr_alternative "type" > [(if_then_else (match_operand 1 "const_int_operand" "") > (const_string "mov_imm" ) > (const_string "mov_reg")) > (const_string "mvn_imm") > + (const_string "mov_imm") > (const_string "store1") > (const_string "load1")])] > ) >