On Mon, Jun 27, 2016 at 03:58:00PM +0100, Jiong Wang wrote:
> On 07/06/16 09:46, Jiong Wang wrote:
> >2016-06-07  Matthew Wahab<matthew.wa...@arm.com>
> >            Jiong Wang<jiong.w...@arm.com>
> >
> >        * config/aarch64/aarch64-arches.def: Add "armv8.2-a".
> >        * config/aarch64/aarch64.h (AARCH64_FL_V8_2): New.
> >        (AARCH64_FL_F16): New.
> >        (AARCH64_FL_FOR_ARCH8_2): New.
> >        (AARCH64_ISA_8_2): New.
> >        (AARCH64_ISA_F16): New.
> >        (TARGET_FP_F16INST): New.
> >        (TARGET_SIMD_F16INST): New.
> >        * config/aarch64/aarch64-option-extensions.def: New entry
> >for "fp16".
> >        * config/aarch64/aarch64-c.c (arch64_update_cpp_builtins):
> >Conditionally define
> >        __ARM_FEATURE_FP16_SCALAR_ARITHMETIC and
> >__ARM_FEATURE_FP16_VECTOR_ARITHMETIC.
> >        * doc/invoke.texi (AArch64 Options): Document "armv8.2-a"
> >and "fp16".
> >
> 
> This is a updated version of this patch, the updates are:
> 
>   * When enabling "fp16" also enables "fp".
>   * When disabling "fp" also disables "fp16".
>   * When disabling "fp16" only disables "fp16".
> 
> OK for trunk?

Hi Jiong,

Some very minor comments below.

> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index b15c23f0..d715da7 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -135,6 +135,8 @@ extern unsigned aarch64_architecture_version;
>  /* ARMv8.1 architecture extensions.  */
>  #define AARCH64_FL_LSE             (1 << 4)  /* Has Large System Extensions. 
>  */
>  #define AARCH64_FL_V8_1            (1 << 5)  /* Has ARMv8.1 extensions.  */
> +#define AARCH64_FL_V8_2            (1 << 8)  /* Has ARMv8.2 features.  */
> +#define AARCH64_FL_F16             (1 << 9)  /* Has ARMv8.2 FP16 extensions. 
>  */

Please add a new "section" for the ARMv8.2-A architecture extensions (just
a new comment that groups them), and s/ARMv8.2/ARMv8.2-A/ throughout this
section (if you'd like to do the follow-up trivial patch for
s/ARMv8.1/ARMv8.1-A/ I'd appreciate that).

> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index aa11209..d44bbc0 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -13035,7 +13035,10 @@ more feature modifiers.  This option has the form
>  @option{-march=@var{arch}@r{@{}+@r{[}no@r{]}@var{feature}@r{@}*}}.
>  
>  The permissible values for @var{arch} are @samp{armv8-a},
> -@samp{armv8.1-a} or @var{native}.
> +@samp{armv8.1-a}, @samp{armv8.2-a} or @var{native}.
> +
> +The value @samp{armv8.2-a} implies @samp{armv8.1-a} and enables compiler
> +support for the ARMv8.2 architecture extensions.

ARMv8.2-A here too please.

>  
>  The value @samp{armv8.1-a} implies @samp{armv8-a} and enables compiler
>  support for the ARMv8.1 architecture extension.  In particular, it
> @@ -13140,6 +13143,8 @@ instructions.  This is on by default for all possible 
> values for options
>  @item lse
>  Enable Large System Extension instructions.  This is on by default for
>  @option{-march=armv8.1-a}.
> +@item fp16
> +Enable FP16 extension.

Please add a note here that enabling this extension also enables "fp"

This is OK for trunk otherwise.

Thanks,
James

> 
> 2016-06-27  Matthew Wahab  <matthew.wa...@arm.com>
>             Jiong Wang  <jiong.w...@arm.com>
> 
>         * config/aarch64/aarch64-arches.def: Add "armv8.2-a".
>         * config/aarch64/aarch64.h (AARCH64_FL_V8_2): New.
>         (AARCH64_FL_F16): New.
>         (AARCH64_FL_FOR_ARCH8_2): New.
>         (AARCH64_ISA_8_2): New.
>         (AARCH64_ISA_F16): New.
>         (TARGET_FP_F16INST): New.
>         (TARGET_SIMD_F16INST): New.
>         * config/aarch64/aarch64-option-extensions.def ("fp16"): New entry.
>         ("fp"): Disabling "fp" also disables "fp16".
>         * config/aarch64/aarch64-c.c (arch64_update_cpp_builtins): 
> Conditionally define
>         __ARM_FEATURE_FP16_SCALAR_ARITHMETIC and 
> __ARM_FEATURE_FP16_VECTOR_ARITHMETIC.
>         * doc/invoke.texi (AArch64 Options): Document "armv8.2-a" and "fp16".

Reply via email to