| Message ID | 55B76623.7000409@arm.com |
|---|---|
| State | New |
| Headers | show |
Hi Alan, On 28/07/15 12:23, Alan Lawrence wrote: > This makes the existing float16 vector intrinsics available only when we have an > __fp16 type (i.e. when one of the ARM_FP16_FORMAT_... macros is defined). > > Thus, we also rearrange the float16x[48]_t types to use the same type as __fp16 > for the element type (ACLE says that __fp16 should be an alias). > > To keep the existing gcc.target/arm/neon/vcvt{f16_f32,f32_f16} tests working, as > these do not specify an -mfp16-format, I've modified > check_effective_target_arm_neon_fp16_ok to add in -mfp16-format=ieee *if > necessary* (hence still allowing an explicit -mfp16-format=alternative). A > documentation fix for this follows in the last patch. > > gcc/ChangeLog: > > * config/arm/arm-builtins.c (arm_init_simd_builtin_types): Move > initialization of HFmode scalar type (float16_t) to... > (arm_init_fp16_builtins): ...here, combining with previous __fp16. I'd say: "... Here. Combine with __fp16 initialization code" > (arm_init_builtins): Call arm_init_fp16_builtins earlier and always. > > * config/arm/arm_neon.h (vcvt_f16_f32, vcvt_f32_f16): Condition on > having an -mfp16-format. > > gcc/testsuite/ChangeLog: > > * lib/target-supports.exp > (check_effective_target_arm_neon_fp16_ok_nocache): Add flag variants > with -mfp16-format=ieee. @@ -1752,12 +1749,11 @@ arm_init_builtins (void) if (TARGET_REALLY_IWMMXT) arm_init_iwmmxt_builtins (); + arm_init_fp16_builtins (); + if (TARGET_NEON) arm_init_neon_builtins (); - if (arm_fp16_format) - arm_init_fp16_builtins (); - if (TARGET_CRC32) arm_init_crc32_builtins (); Can you please add a comment above arm_init_fp16_builtins (); saying that it needs to be called before arm_init_neon_builtins so that arm_simd_floatHF_type_node gets initialised properly? (Or words to that effect). Ok with the comment. Thanks, Kyrill
Thanks, pushed with comment and ChangeLog fix as r227033. --Alan Kyrill Tkachov wrote: > Hi Alan, > > On 28/07/15 12:23, Alan Lawrence wrote: >> This makes the existing float16 vector intrinsics available only when we have an >> __fp16 type (i.e. when one of the ARM_FP16_FORMAT_... macros is defined). >> >> Thus, we also rearrange the float16x[48]_t types to use the same type as __fp16 >> for the element type (ACLE says that __fp16 should be an alias). >> >> To keep the existing gcc.target/arm/neon/vcvt{f16_f32,f32_f16} tests working, as >> these do not specify an -mfp16-format, I've modified >> check_effective_target_arm_neon_fp16_ok to add in -mfp16-format=ieee *if >> necessary* (hence still allowing an explicit -mfp16-format=alternative). A >> documentation fix for this follows in the last patch. >> >> gcc/ChangeLog: >> >> * config/arm/arm-builtins.c (arm_init_simd_builtin_types): Move >> initialization of HFmode scalar type (float16_t) to... >> (arm_init_fp16_builtins): ...here, combining with previous __fp16. > > I'd say: "... Here. Combine with __fp16 initialization code" > >> (arm_init_builtins): Call arm_init_fp16_builtins earlier and always. >> >> * config/arm/arm_neon.h (vcvt_f16_f32, vcvt_f32_f16): Condition on >> having an -mfp16-format. >> >> gcc/testsuite/ChangeLog: >> >> * lib/target-supports.exp >> (check_effective_target_arm_neon_fp16_ok_nocache): Add flag variants >> with -mfp16-format=ieee. > > @@ -1752,12 +1749,11 @@ arm_init_builtins (void) > if (TARGET_REALLY_IWMMXT) > arm_init_iwmmxt_builtins (); > > + arm_init_fp16_builtins (); > + > if (TARGET_NEON) > arm_init_neon_builtins (); > > - if (arm_fp16_format) > - arm_init_fp16_builtins (); > - > if (TARGET_CRC32) > arm_init_crc32_builtins (); > > > Can you please add a comment above arm_init_fp16_builtins (); > saying that it needs to be called before arm_init_neon_builtins > so that arm_simd_floatHF_type_node gets initialised properly? > (Or words to that effect). > > Ok with the comment. > > Thanks, > Kyrill >
diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c index 89b1b0cd2c51d83abb02d555f3881d0270557ccd..8d4833428382305dc3595cee2e172289c9a874cf 100644 --- a/gcc/config/arm/arm-builtins.c +++ b/gcc/config/arm/arm-builtins.c @@ -771,13 +771,6 @@ arm_init_simd_builtin_types (void) int nelts = sizeof (arm_simd_types) / sizeof (arm_simd_types[0]); tree tdecl; - /* Initialize the HFmode scalar type. */ - arm_simd_floatHF_type_node = make_node (REAL_TYPE); - TYPE_PRECISION (arm_simd_floatHF_type_node) = GET_MODE_PRECISION (HFmode); - layout_type (arm_simd_floatHF_type_node); - (*lang_hooks.types.register_builtin_type) (arm_simd_floatHF_type_node, - "__builtin_neon_hf"); - /* Poly types are a world of their own. In order to maintain legacy ABI, they get initialized using the old interface, and don't get an entry in our mangling table, consequently, they get default @@ -825,6 +818,8 @@ arm_init_simd_builtin_types (void) mangling. */ /* Continue with standard types. */ + /* The __builtin_simd{64,128}_float16 types are kept private unless + we have a scalar __fp16 type. */ arm_simd_types[Float16x4_t].eltype = arm_simd_floatHF_type_node; arm_simd_types[Float32x2_t].eltype = float_type_node; arm_simd_types[Float32x4_t].eltype = float_type_node; @@ -1704,10 +1699,12 @@ arm_init_iwmmxt_builtins (void) static void arm_init_fp16_builtins (void) { - tree fp16_type = make_node (REAL_TYPE); - TYPE_PRECISION (fp16_type) = 16; - layout_type (fp16_type); - (*lang_hooks.types.register_builtin_type) (fp16_type, "__fp16"); + arm_simd_floatHF_type_node = make_node (REAL_TYPE); + TYPE_PRECISION (arm_simd_floatHF_type_node) = GET_MODE_PRECISION (HFmode); + layout_type (arm_simd_floatHF_type_node); + if (arm_fp16_format) + (*lang_hooks.types.register_builtin_type) (arm_simd_floatHF_type_node, + "__fp16"); } static void @@ -1752,12 +1749,11 @@ arm_init_builtins (void) if (TARGET_REALLY_IWMMXT) arm_init_iwmmxt_builtins (); + arm_init_fp16_builtins (); + if (TARGET_NEON) arm_init_neon_builtins (); - if (arm_fp16_format) - arm_init_fp16_builtins (); - if (TARGET_CRC32) arm_init_crc32_builtins (); diff --git a/gcc/config/arm/arm_neon.h b/gcc/config/arm/arm_neon.h index c923e294cda2f8cb88e4b1ccca6fd4f13a3ed98d..2b30be61a46a0c906478c599a005c27cd467dfa6 100644 --- a/gcc/config/arm/arm_neon.h +++ b/gcc/config/arm/arm_neon.h @@ -41,7 +41,9 @@ typedef __simd64_int8_t int8x8_t; typedef __simd64_int16_t int16x4_t; typedef __simd64_int32_t int32x2_t; typedef __builtin_neon_di int64x1_t; +#if defined (__ARM_FP16_FORMAT_IEEE) || defined (__ARM_FP16_FORMAT_ALTERNATIVE) typedef __simd64_float16_t float16x4_t; +#endif typedef __simd64_float32_t float32x2_t; typedef __simd64_poly8_t poly8x8_t; typedef __simd64_poly16_t poly16x4_t; @@ -6220,21 +6222,25 @@ vcvtq_u32_f32 (float32x4_t __a) } #if ((__ARM_FP & 0x2) != 0) +#if defined (__ARM_FP16_FORMAT_IEEE) || defined (__ARM_FP16_FORMAT_ALTERNATIVE) __extension__ static __inline float16x4_t __attribute__ ((__always_inline__)) vcvt_f16_f32 (float32x4_t __a) { return (float16x4_t)__builtin_neon_vcvtv4hfv4sf (__a); } - #endif +#endif + #if ((__ARM_FP & 0x2) != 0) +#if defined (__ARM_FP16_FORMAT_IEEE) || defined (__ARM_FP16_FORMAT_ALTERNATIVE) __extension__ static __inline float32x4_t __attribute__ ((__always_inline__)) vcvt_f32_f16 (float16x4_t __a) { return (float32x4_t)__builtin_neon_vcvtv4sfv4hf (__a); } - #endif +#endif + __extension__ static __inline int32x2_t __attribute__ ((__always_inline__)) vcvt_n_s32_f32 (float32x2_t __a, const int __b) { diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index f0c209fc7022fda77ee2a04b496fca8a6b1dcce0..46fadef02bc5ae07a5b9e4fc525de9cf1968b0a3 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -2682,7 +2682,11 @@ proc check_effective_target_arm_neon_fp16_ok_nocache { } { set et_arm_neon_fp16_flags "" if { [check_effective_target_arm32] } { foreach flags {"" "-mfloat-abi=softfp" "-mfpu=neon-fp16" - "-mfpu=neon-fp16 -mfloat-abi=softfp"} { + "-mfpu=neon-fp16 -mfloat-abi=softfp" + "-mfp16-format=ieee" + "-mfloat-abi=softfp -mfp16-format=ieee" + "-mfpu=neon-fp16 -mfp16-format=ieee" + "-mfpu=neon-fp16 -mfloat-abi=softfp -mfp16-format=ieee"} { if { [check_no_compiler_messages_nocache arm_neon_fp_16_ok object { #include "arm_neon.h" float16x4_t
This makes the existing float16 vector intrinsics available only when we have an __fp16 type (i.e. when one of the ARM_FP16_FORMAT_... macros is defined). Thus, we also rearrange the float16x[48]_t types to use the same type as __fp16 for the element type (ACLE says that __fp16 should be an alias). To keep the existing gcc.target/arm/neon/vcvt{f16_f32,f32_f16} tests working, as these do not specify an -mfp16-format, I've modified check_effective_target_arm_neon_fp16_ok to add in -mfp16-format=ieee *if necessary* (hence still allowing an explicit -mfp16-format=alternative). A documentation fix for this follows in the last patch. gcc/ChangeLog: * config/arm/arm-builtins.c (arm_init_simd_builtin_types): Move initialization of HFmode scalar type (float16_t) to... (arm_init_fp16_builtins): ...here, combining with previous __fp16. (arm_init_builtins): Call arm_init_fp16_builtins earlier and always. * config/arm/arm_neon.h (vcvt_f16_f32, vcvt_f32_f16): Condition on having an -mfp16-format. gcc/testsuite/ChangeLog: * lib/target-supports.exp (check_effective_target_arm_neon_fp16_ok_nocache): Add flag variants with -mfp16-format=ieee.