From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 108784 invoked by alias); 6 Oct 2017 16:24:05 -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 108758 invoked by uid 89); 6 Oct 2017 16:24:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_PASS 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; Fri, 06 Oct 2017 16:24:02 +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 759371435; Fri, 6 Oct 2017 09:23:59 -0700 (PDT) Received: from [192.168.1.19] (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 317D93F53D; Fri, 6 Oct 2017 09:23:56 -0700 (PDT) Subject: Re: [PATCH][GCC][ARM] Dot Product commandline options [Patch (1/8)] To: Tamar Christina , Kyrill Tkachov , "gcc-patches@gcc.gnu.org" Cc: nd , Ramana Radhakrishnan , "nickc@redhat.com" References: <20170901131912.GA31822@arm.com> <59B90213.3020906@foss.arm.com> From: "Richard Earnshaw (lists)" Message-ID: Date: Fri, 06 Oct 2017 16:24:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-10/txt/msg00389.txt.bz2 On 06/10/17 13:44, Tamar Christina wrote: > Hi All, > > This is a respin of the patch with the feedback processed. > > Regtested on arm-none-eabi, armeb-none-eabi, > aarch64-none-elf and aarch64_be-none-elf with no issues found. > > Ok for trunk? > > gcc/ > 2017-10-06 Tamar Christina > > * config/arm/arm.h (TARGET_DOTPROD): New. > * config/arm/arm.c (arm_arch_dotprod): New. > (arm_option_reconfigure_globals): Add arm_arch_dotprod. > * config/arm/arm-c.c (__ARM_FEATURE_DOTPROD): New. > * config/arm/arm-cpus.in (cortex-a55, cortex-75): Enabled +dotprod. > (armv8.2-a, cortex-a75.cortex-a55): Likewise. > (feature dotprod, group dotprod, ALL_SIMD_INTERNAL): New. > (ALL_FPU_INTERNAL): Use ALL_SIMD_INTERNAL. > * config/arm/t-multilib (v8_2_a_simd_variants): Add dotprod. > * doc/invoke.texi (armv8.2-a): Document dotprod > ________________________________________ > From: Kyrill Tkachov > Sent: Wednesday, September 13, 2017 11:01:55 AM > To: Tamar Christina; gcc-patches@gcc.gnu.org > Cc: nd; Ramana Radhakrishnan; Richard Earnshaw; nickc@redhat.com > Subject: Re: [PATCH][GCC][ARM] Dot Product commandline options [Patch (1/8)] > > Hi Tamar, > > On 01/09/17 14:19, Tamar Christina wrote: >> Hi All, >> >> This patch adds support for the +dotprod extension to ARM. >> Dot Product requires Adv.SIMD to work and so enables this option >> by default when enabled. >> >> It is available from ARMv8.2-a and onwards and is enabled by >> default on Cortex-A55 and Cortex-A75. >> >> Regtested and bootstrapped on arm-none-eabi and no issues. > > I'm assuming you mean arm-none-linux-gnueabihf :) > >> Ok for trunk? >> >> gcc/ >> 2017-09-01 Tamar Christina >> >> * config/arm/arm.h (TARGET_DOTPROD): New. >> * config/arm/arm.c (arm_arch_dotprod): New. >> (arm_option_reconfigure_globals): Add arm_arch_dotprod. >> * config/arm/arm-c.c (__ARM_FEATURE_DOTPROD): New. >> * config/arm/arm-cpus.in (cortex-a55, cortex-75): Enabled +dotprod. >> (armv8.2-a, cortex-a75.cortex-a55): Likewise. >> * config/arm/arm-isa.h (isa_bit_dotprod, ISA_DOTPROD): New. > > arm-isa.h is now autogenerated after r251799 so you'll need to rebase on > top of that. > That being said, that patch was temporarily reverted [1] so you'll have > to apply it manually in your > tree to rebase, or wait until it is reapplied. > > [1] https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00579.html > > The patch looks ok to me otherwise with a documentation nit below. > >> * config/arm/t-multilib (v8_2_a_simd_variants): Add dotprod. >> * doc/invoke.texi (armv8.2-a): Document dotprod >> > > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -15492,6 +15492,10 @@ The ARMv8.1 Advanced SIMD and floating-point instructions. > The cryptographic instructions. This also enables the Advanced SIMD and > floating-point instructions. > > +@item +dotprod > +Enable the Dot Product extension. This also enables Advanced SIMD instructions > +and allows auto vectorization of dot products to the Dot Product instructions. > > This should be "auto-vectorization" > > Thanks, > Kyrill > > > Hmm, can you arrange to add patches as text/plain attachments, so that when I reply the patch is included for comments, please? +# double-precision FP. Make sure bits that are not an FPU bit go instructions +# ALL_SIMD instead of ALL_SIMD_INTERNAL. Two spaces after full stop. The new sentence doesn't make sense. Instead, I think you should probably put the following: "ALL_FPU lists all the feature bits associated with the floating-point unit; these will all be removed if the floating-point unit is disabled (eg -mfloat-abi=soft). ALL_FPU_INTERNAL must ONLY contain features that form part of a named -mfpu option; it is used to map the capabilities back to a named FPU for the benefit of the assembler. ALL_SIMD_INTERNAL and ALL_SIMD are similarly defined to help with the construction of ALL_FPU and ALL_FPU_INTERNAL; they describe the SIMD extensions that are either part of a named FPU or optional extensions respectively." You might need to rejig the other sentence there as well to make it more consistent. @@ -239,6 +243,7 @@ define fgroup FP_D32 FP_DBL fp_d32 define fgroup FP_ARMv8 FPv5 FP_D32 define fgroup NEON FP_D32 neon define fgroup CRYPTO NEON crypto +define fgroup DOTPROD NEON dotprod lines above have a hard tab between the group name and the features it contains. Your entry has spaces. Please fix for consistency. @@ -1473,9 +1479,10 @@ begin cpu cortex-a55 cname cortexa55 tune for cortex-a53 tune flags LDSCHED - architecture armv8.2-a+fp16 + architecture armv8.2-a+fp16+dotprod fpu neon-fp-armv8 option crypto add FP_ARMv8 CRYPTO + option dotprod add FP_ARMv8 DOTPROD We don't have an option entry for +fp16 (all Cortex-a55 cores implement it), so we should treat dotprod similarly here. Crypto is a special case because it isn't enabled by default. Similarly for the other cores later in the patch.