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.
>  
> 

Reply via email to