Re: [PATCH][AArch64][10/14] Implement target pragmas

[James Greenhalgh <james dot greenhalgh at arm dot com>]

On Mon, Aug 03, 2015 at 10:36:17AM +0100, Kyrill Tkachov wrote:
> And here is a rebased version to resolve a conflict after Alan's patches went in.
> 

OK with the nits below fixed.

> 2015-08-03  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>       * config.gcc (aarch64*-*-*): Specify c_target_objs and cxx_target_objs.
>       * config/aarch64/aarch64.h (REGISTER_TARGET_PRAGMAS): Define.
>       (TARGET_CPU_CPP_BUILTINS): Redefine to call aarch64_cpu_cpp_builtins.
>       * config/aarch64/aarch64.c (aarch64_override_options_internal): Remove
>       static keyword.
>       (aarch64_reset_previous_fndecl): New function.
>       (aarch64_handle_attr_isa_flags): Handle "+nothing" in the beginning of
>       the string.
>       * config/aarch64/aarch64-c.c: New file.
>       * config/aarch64/arm_acle.h: Add pragma +crc+nofp at the top.
>       Push and pop options at beginning and end.  Remove ifdef
>       __ARM_FEATURE_CRC32.
>       * config/aarch64/arm_neon.h: Remove #ifdef check on __ARM_NEON.
>       Add pragma arch=armv8-a+simd and +crypto where appropriate.
>       * config/aarch64/t-aarch64 (aarch64-c.o): New rule.
>       * config/aarch64/aarch64-protos.h (aarch64_cpu_cpp_builtins):
>       Define prototype.
>       (aarch64_register_pragmas): Likewise.
>       (aarch64_reset_previous_fndecl): Likewise.
>       (aarch64_process_target_attr): Likewise.
>       (aarch64_override_options_internal): Likewise.
> 
> 2015-08-03  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>      * gcc.target/aarch64/arm_neon-nosimd-error.c: Delete.
> 
> 

> +/* Define the macros that we always expect to have on AArch64.  */
> +
> +static void
> +aarch64_define_unconditional_macros (cpp_reader *pfile)
> +{
> +  builtin_define ("__aarch64__");
> +  builtin_define ("__ARM_64BIT_STATE");
> +
> +  builtin_define ("__ARM_ARCH_ISA_A64");
> +  builtin_define_with_int_value ("__ARM_ALIGN_MAX_PWR", 28);
> +  builtin_define_with_int_value ("__ARM_ALIGN_MAX_STACK_PWR", 16);
> +
> +  /* __ARM_ARCH_8A is not mandated by ACLE but we define it unconditionally
> +     as interoperability with the same arm macro.  */
> +  builtin_define ("__ARM_ARCH_8A");
> +
> +  builtin_define_with_int_value ("__ARM_ARCH_PROFILE", 'A');
> +  builtin_define ("__ARM_FEATURE_CLZ");
> +  builtin_define ("__ARM_FEATURE_IDIV");
> +  builtin_define ("__ARM_FEATURE_UNALIGNED");
> +  builtin_define ("__ARM_PCS_AAPCS64");
> +  builtin_define_with_int_value ("__ARM_SIZEOF_WCHAR_T", WCHAR_TYPE_SIZE / 8);
> +
> +}

Extra newline.

> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 3a5482d..6844c90 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -249,6 +249,7 @@ enum aarch64_symbol_type
>  aarch64_classify_symbolic_expression (rtx, enum aarch64_symbol_context);
>  bool aarch64_const_vec_all_same_int_p (rtx, HOST_WIDE_INT);
>  bool aarch64_constant_address_p (rtx);
> +extern void aarch64_cpu_cpp_builtins (cpp_reader *);

No need for this "extern" - and keep this in alphabetical order (first by
return type, then by name).

> +#pragma GCC push_options
> +#pragma GCC target ("+crypto")

Keep things simple to manage and understand by inspection, and make these
"+nothing+crypto".

> @@ -21067,7 +21065,8 @@ vrsrad_n_u64 (uint64_t __a, uint64_t __b, const int __c)
>    return __builtin_aarch64_ursra_ndi_uuus (__a, __b, __c);
>  }
>  
> -#ifdef __ARM_FEATURE_CRYPTO
> +#pragma GCC push_options
> +#pragma GCC target ("+crypto")
>  
>  /* vsha1  */
>  

Likewise here.

Thanks,
James