This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][GCC][ARM] Dot Product commandline options [Patch (1/8)]
- From: "Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>
- To: Tamar Christina <Tamar dot Christina at arm dot com>, Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Cc: nd <nd at arm dot com>, Ramana Radhakrishnan <Ramana dot Radhakrishnan at arm dot com>, "nickc at redhat dot com" <nickc at redhat dot com>
- Date: Fri, 6 Oct 2017 17:23:53 +0100
- Subject: Re: [PATCH][GCC][ARM] Dot Product commandline options [Patch (1/8)]
- Authentication-results: sourceware.org; auth=none
- References: <20170901131912.GA31822@arm.com> <59B90213.3020906@foss.arm.com> <DB6PR0802MB2309C5B880EA1A8F86576565FF710@DB6PR0802MB2309.eurprd08.prod.outlook.com>
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 <tamar.christina@arm.com>
>
> * 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 <kyrylo.tkachov@foss.arm.com>
> 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 <tamar.christina@arm.com>
>>
>> * 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.