From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 111829 invoked by alias); 24 Jul 2018 15:39:21 -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 111811 invoked by uid 89); 24 Jul 2018 15:39:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT autolearn=ham version=3.3.2 spammy= X-HELO: foss.arm.com Received: from usa-sjc-mx-foss1.foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 24 Jul 2018 15:39:18 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id EF50480D; Tue, 24 Jul 2018 08:39:16 -0700 (PDT) Received: from [10.2.207.77] (e100706-lin.cambridge.arm.com [10.2.207.77]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 78D793F237; Tue, 24 Jul 2018 08:39:15 -0700 (PDT) Message-ID: <5B574821.80405@foss.arm.com> Date: Tue, 24 Jul 2018 15:39:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: James Greenhalgh , Matthew Malcomson CC: Sudakshina Das , "gcc-patches@gcc.gnu.org" , Richard Earnshaw , Marcus Shawcroft , Richard Sandiford , nd Subject: Re: [PATCH][GCC][AARCH64] Canonicalize aarch64 widening simd plus insns 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> In-Reply-To: <20180724151229.GA11547@arm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2018-07/txt/msg01401.txt.bz2 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 Thanks, Kyrill > Thanks, > James >