On 09/10/17 10:56, Tamar Christina wrote: > Hi Richard, > > Here is a respin with the requested changes. > > Ok for trunk? > > Thanks, > Tamar > > gcc/ > 2017-10-09 Tamar Christina <tamar.christ...@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 (armv8.2-a): Enabled +dotprod. > (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 >
OK. R. > The 10/06/2017 17:23, Richard Earnshaw (lists) wrote: >> 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.christ...@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.tkac...@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; ni...@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.christ...@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. > > > 7949-diff.patch > > > diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c > index > 55472434c3a6e90c5693bbaabd3265f7d968787f..295f03bf8ee02be7c89ed2967d283be206e9f25a > 100644 > --- a/gcc/config/arm/arm-c.c > +++ b/gcc/config/arm/arm-c.c > @@ -73,6 +73,7 @@ arm_cpu_builtins (struct cpp_reader* pfile) > def_or_undef_macro (pfile, "__ARM_FEATURE_QRDMX", TARGET_NEON_RDMA); > > def_or_undef_macro (pfile, "__ARM_FEATURE_CRC32", TARGET_CRC32); > + def_or_undef_macro (pfile, "__ARM_FEATURE_DOTPROD", TARGET_DOTPROD); > def_or_undef_macro (pfile, "__ARM_32BIT_STATE", TARGET_32BIT); > > cpp_undef (pfile, "__ARM_FEATURE_CMSE"); > diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in > index > 07de4c9375ba7a0df0d8bd00385e54a4042e5264..2da2a7d8e2b63d3d93ca73172b10ff23c1d8d8f9 > 100644 > --- a/gcc/config/arm/arm-cpus.in > +++ b/gcc/config/arm/arm-cpus.in > @@ -156,6 +156,8 @@ define feature crypto > # FP16 data processing (half-precision float). > define feature fp16 > > +# Dot Product instructions extension to ARMv8.2-a. > +define feature dotprod > > # ISA Quirks (errata?). Don't forget to add this to the fgroup > # ALL_QUIRKS below. > @@ -173,6 +175,17 @@ define feature quirk_cm3_ldrd > define feature smallmul > > # Feature groups. Conventionally all (or mostly) upper case. > +# 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. > + > > # List of all cryptographic extensions to stripout if crypto is > # disabled. Currently, that's trivial, but we define it anyway for > @@ -182,11 +195,12 @@ define fgroup ALL_CRYPTO crypto > # List of all SIMD bits to strip out if SIMD is disabled. This does > # strip off 32 D-registers, but does not remove support for > # double-precision FP. > -define fgroup ALL_SIMD fp_d32 neon ALL_CRYPTO > +define fgroup ALL_SIMD_INTERNAL fp_d32 neon ALL_CRYPTO > +define fgroup ALL_SIMD ALL_SIMD_INTERNAL dotprod > > # List of all FPU bits to strip out if -mfpu is used to override the > # default. fp16 is deliberately missing from this list. > -define fgroup ALL_FPU_INTERNAL vfpv2 vfpv3 vfpv4 fpv5 fp16conv fp_dbl > ALL_SIMD > +define fgroup ALL_FPU_INTERNAL vfpv2 vfpv3 vfpv4 fpv5 fp16conv fp_dbl > ALL_SIMD_INTERNAL > > # Similarly, but including fp16 and other extensions that aren't part of > # -mfpu support. > @@ -239,6 +253,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 > > # List of all quirk bits to strip out when comparing CPU features with > # architectures. > @@ -561,6 +576,7 @@ begin arch armv8.2-a > option crypto add FP_ARMv8 CRYPTO > option nocrypto remove ALL_CRYPTO > option nofp remove ALL_FP > + option dotprod add FP_ARMv8 DOTPROD > end arch armv8.2-a > > begin arch armv8-m.base > @@ -1473,7 +1489,7 @@ 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 nofp remove ALL_FP > @@ -1484,7 +1500,7 @@ begin cpu cortex-a75 > cname cortexa75 > tune for cortex-a57 > tune flags LDSCHED > - architecture armv8.2-a+fp16 > + architecture armv8.2-a+fp16+dotprod > fpu neon-fp-armv8 > option crypto add FP_ARMv8 CRYPTO > costs cortex_a73 > @@ -1496,7 +1512,7 @@ begin cpu cortex-a75.cortex-a55 > cname cortexa75cortexa55 > 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 > costs cortex_a73 > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > index > a3ca800f7a5cb876368480b97d0641f5d02af5d0..7e1eeb5254c2ce32ced2abdb43d1733ee1a45cd5 > 100644 > --- a/gcc/config/arm/arm.h > +++ b/gcc/config/arm/arm.h > @@ -210,6 +210,11 @@ extern tree arm_fp16_type_node; > /* FPU supports ARMv8.1 Adv.SIMD extensions. */ > #define TARGET_NEON_RDMA (TARGET_NEON && arm_arch8_1) > > +/* Supports for Dot Product AdvSIMD extensions. */ > +#define TARGET_DOTPROD (TARGET_NEON \ > + && bitmap_bit_p (arm_active_target.isa, \ > + isa_bit_dotprod)) > + > /* FPU supports the floating point FP16 instructions for ARMv8.2 and later. > */ > #define TARGET_VFP_FP16INST \ > (TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_VFP5 && arm_fp16_inst) > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index > 1943908bd840472bbab0a557d3e337c02b2ae26d..fb869d6441394d5caecbae9acebc19432ec788f2 > 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -971,6 +971,9 @@ int arm_condexec_masklen = 0; > /* Nonzero if chip supports the ARMv8 CRC instructions. */ > int arm_arch_crc = 0; > > +/* Nonzero if chip supports the AdvSIMD Dot Product instructions. */ > +int arm_arch_dotprod = 0; > + > /* Nonzero if chip supports the ARMv8-M security extensions. */ > int arm_arch_cmse = 0; > > @@ -3607,6 +3610,8 @@ arm_option_reconfigure_globals (void) > arm_arch_cmse = bitmap_bit_p (arm_active_target.isa, isa_bit_cmse); > arm_fp16_inst = bitmap_bit_p (arm_active_target.isa, isa_bit_fp16); > arm_arch_lpae = bitmap_bit_p (arm_active_target.isa, isa_bit_lpae); > + arm_arch_dotprod = bitmap_bit_p (arm_active_target.isa, isa_bit_dotprod); > + > if (arm_fp16_inst) > { > if (arm_fp16_format == ARM_FP16_FORMAT_ALTERNATIVE) > diff --git a/gcc/config/arm/t-multilib b/gcc/config/arm/t-multilib > index > ec4b76dbc8fc56093c2b27c95e0947558496fe5a..47f3673160a766c5b1b8972f7670355f2c357b56 > 100644 > --- a/gcc/config/arm/t-multilib > +++ b/gcc/config/arm/t-multilib > @@ -68,7 +68,7 @@ v7ve_vfpv4_simd_variants := +simd > v8_a_nosimd_variants := +crc > v8_a_simd_variants := $(call all_feat_combs, simd crypto) > v8_1_a_simd_variants := $(call all_feat_combs, simd crypto) > -v8_2_a_simd_variants := $(call all_feat_combs, simd fp16 crypto) > +v8_2_a_simd_variants := $(call all_feat_combs, simd fp16 crypto dotprod) > > > ifneq (,$(HAS_APROFILE)) > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index > f862b7f8c99e7e9cca200fbe5b7d969748fed3f9..a69b0d59cdfecf82b97c8fad0de35fe7ac939e19 > 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -15597,6 +15597,9 @@ 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. > + > @item +nocrypto > Disable the cryptographic extension. > >