From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 62595 invoked by alias); 30 Jul 2018 13:30:56 -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 62366 invoked by uid 89); 30 Jul 2018 13:30:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=replies, Hx-spam-relays-external:209.85.213.66, H*RU:209.85.213.66 X-HELO: mail-vk0-f66.google.com Received: from mail-vk0-f66.google.com (HELO mail-vk0-f66.google.com) (209.85.213.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Jul 2018 13:30:38 +0000 Received: by mail-vk0-f66.google.com with SMTP id s17-v6so5700162vke.10 for ; Mon, 30 Jul 2018 06:30:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=jWXDEfx8FFlnwDRTK4brjcZigMeQaBLCGQxso3UTdl0=; b=ILM0VMp71lUknPWllidHQITBZbrQwcllOrUmJUmW53jorW8CZ02HXEbhhUD396keTe T24CyWOhYsFPhZNw8Y/aqNJGYsk+gYCkxqtSg5ChViXcMAFtjWxOQLYvZkpau4SI6G0c bsDDClWM7NOYRKyGq59kfLhm2xK0gprWibHp0= MIME-Version: 1.0 References: <006edc68-c5a2-e77d-70d6-02ac77ef53e3@arm.com> <87bmbc7o67.fsf@arm.com> <030aafb9-ad3c-422f-c20d-1e407068a786@arm.com> <22844196-5cd4-d456-4194-14ced88f642e@arm.com> <20180724151229.GA11547@arm.com> <5B574821.80405@foss.arm.com> In-Reply-To: <5B574821.80405@foss.arm.com> From: Christophe Lyon Date: Mon, 30 Jul 2018 13:30:00 -0000 Message-ID: Subject: Re: [PATCH][GCC][AARCH64] Canonicalize aarch64 widening simd plus insns To: Kyrill Tkachov Cc: James Greenhalgh , Matthew.Malcomson@arm.com, Sudi Das , gcc Patches , Richard Earnshaw , Marcus Shawcroft , Richard Sandiford , nd Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-07/txt/msg01840.txt.bz2 Hi, On Tue, 24 Jul 2018 at 17:39, Kyrill Tkachov wrote: > > > On 24/07/18 16:12, James Greenhalgh wrote: > > On Thu, Jul 19, 2018 at 07:35:22AM -0500, Matthew Malcomson wrote: > > > Hi again. > > > > > > Providing an updated patch to include the formatting suggestions. > > > > Please try not to top-post replies, it makes the conversation thread > > harder to follow (reply continues below!). > > > > > On 12/07/18 11:39, Sudakshina Das wrote: > > > > Hi Matthew > > > > > > > > On 12/07/18 11:18, Richard Sandiford wrote: > > > >> Looks good to me FWIW (not a maintainer), just a minor formatting thing: > > > >> > > > >> Matthew Malcomson writes: > > > >>> diff --git a/gcc/config/aarch64/aarch64-simd.md > > > >>> b/gcc/config/aarch64/aarch64-simd.md > > > >>> index > > > >>> aac5fa146ed8dde4507a0eb4ad6a07ce78d2f0cd..67b29cbe2cad91e031ee23be656ec61a403f2cf9 > > > >>> 100644 > > > >>> --- a/gcc/config/aarch64/aarch64-simd.md > > > >>> +++ b/gcc/config/aarch64/aarch64-simd.md > > > >>> @@ -3302,38 +3302,78 @@ > > > >>> DONE; > > > >>> }) > > > >>> -(define_insn "aarch64_w" > > > >>> +(define_insn "aarch64_subw" > > > >>> [(set (match_operand: 0 "register_operand" "=w") > > > >>> - (ADDSUB: (match_operand: 1 "register_operand" > > > >>> "w") > > > >>> - (ANY_EXTEND: > > > >>> - (match_operand:VD_BHSI 2 "register_operand" "w"))))] > > > >>> + (minus: > > > >>> + (match_operand: 1 "register_operand" "w") > > > >>> + (ANY_EXTEND: > > > >>> + (match_operand:VD_BHSI 2 "register_operand" "w"))))] > > > >> > > > >> The (minus should be under the "(match_operand": > > > >> > > > >> (define_insn "aarch64_subw" > > > >> [(set (match_operand: 0 "register_operand" "=w") > > > >> (minus: (match_operand: 1 "register_operand" "w") > > > >> (ANY_EXTEND: > > > >> (match_operand:VD_BHSI 2 "register_operand" "w"))))] > > > >> > > > >> Same for the other patterns. > > > >> > > > >> Thanks, > > > >> Richard > > > >> > > > > > > > > You will need a maintainer's approval but this looks good to me. > > > > Thanks for doing this. I would only point out one other nit which you > > > > can choose to ignore: > > > > > > > > +/* Ensure > > > > + saddw2 and one saddw for the function add() > > > > + ssubw2 and one ssubw for the function subtract() > > > > + uaddw2 and one uaddw for the function uadd() > > > > + usubw2 and one usubw for the function usubtract() */ > > > > + > > > > +/* { dg-final { scan-assembler-times "\[ \t\]ssubw2\[ \t\]+" 1 } } */ > > > > +/* { dg-final { scan-assembler-times "\[ \t\]ssubw\[ \t\]+" 1 } } */ > > > > +/* { dg-final { scan-assembler-times "\[ \t\]saddw2\[ \t\]+" 1 } } */ > > > > +/* { dg-final { scan-assembler-times "\[ \t\]saddw\[ \t\]+" 1 } } */ > > > > +/* { dg-final { scan-assembler-times "\[ \t\]usubw2\[ \t\]+" 1 } } */ > > > > +/* { dg-final { scan-assembler-times "\[ \t\]usubw\[ \t\]+" 1 } } */ > > > > +/* { dg-final { scan-assembler-times "\[ \t\]uaddw2\[ \t\]+" 1 } } */ > > > > +/* { dg-final { scan-assembler-times "\[ \t\]uaddw\[ \t\]+" 1 } } */ > > > > > > > > The scan-assembly directives for the different > > > > functions can be placed right below each of them and that would > > > > make it easier to read the expected results in the test and you > > > > can get rid of the comments saying the same. > > > > Thanks for the first-line review Sudi. > > > > OK for trunk. > > > > I've committed this on behalf of Matthew as: https://gcc.gnu.org/r262949 > The new test fail with -mabi=ilp32: gcc.target/aarch64/simd/vect_su_add_sub.c scan-assembler-times [ \t]saddw2[ \t]+ 1 gcc.target/aarch64/simd/vect_su_add_sub.c scan-assembler-times [ \t]saddw[ \t]+ 1 gcc.target/aarch64/simd/vect_su_add_sub.c scan-assembler-times [ \t]ssubw2[ \t]+ 1 gcc.target/aarch64/simd/vect_su_add_sub.c scan-assembler-times [ \t]ssubw[ \t]+ 1 gcc.target/aarch64/simd/vect_su_add_sub.c scan-assembler-times [ \t]uaddw2[ \t]+ 1 gcc.target/aarch64/simd/vect_su_add_sub.c scan-assembler-times [ \t]uaddw[ \t]+ 1 gcc.target/aarch64/simd/vect_su_add_sub.c scan-assembler-times [ \t]usubw2[ \t]+ 1 gcc.target/aarch64/simd/vect_su_add_sub.c scan-assembler-times [ \t]usubw[ \t]+ 1 I'm not sure how much we care about ilp32? Christophe > Thanks, > Kyrill > > > Thanks, > > James > > >