Message ID | e9de1323-a0f3-44d2-9b1d-6d2d9a46d263@arm.com |
---|---|
State | New |
Headers | show |
Series | aarch64: Add missing ACLE macro for NEON-SVE Bridge | expand |
Richard Ball <richard.ball@arm.com> writes: > __ARM_NEON_SVE_BRIDGE was missed in the original patch and is > added by this patch. > > Ok for trunk and a backport into gcc-14? > > gcc/ChangeLog: > > * config/aarch64/aarch64-c.cc (aarch64_update_cpp_builtins): > Add missing __ARM_NEON_SVE_BRIDGE. After this patch was posted, there was some internal discussion involving LLVM & GNU devs about what this kind of macro means, now that we have FMV. The feeling was that __ARM_NEON_SVE_BRIDGE should just indicate whether the compiler provides the file, not whether AdvSIMD & SVE are enabled. I think we should therefore add this to aarch64_define_unconditional_macros instead. Sorry for the slow review. I was waiting for the outcome of that discussion before replying. Thanks, Richard > diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc > index fe1a20e4e546a68e5f7eddff3bbb0d3e831fbd9b..1121be118cf8d05e3736ad4ee75568ff7cb92bfd 100644 > --- a/gcc/config/aarch64/aarch64-c.cc > +++ b/gcc/config/aarch64/aarch64-c.cc > @@ -260,6 +260,7 @@ aarch64_update_cpp_builtins (cpp_reader *pfile) > aarch64_def_or_undef (TARGET_SME_I16I64, "__ARM_FEATURE_SME_I16I64", pfile); > aarch64_def_or_undef (TARGET_SME_F64F64, "__ARM_FEATURE_SME_F64F64", pfile); > aarch64_def_or_undef (TARGET_SME2, "__ARM_FEATURE_SME2", pfile); > + aarch64_def_or_undef (TARGET_SVE, "__ARM_NEON_SVE_BRIDGE", pfile); > > /* Not for ACLE, but required to keep "float.h" correct if we switch > target between implementations that do or do not support ARMv8.2-A
v2: Change macro definition following internal discussion. __ARM_NEON_SVE_BRIDGE was missed in the original patch and is added by this patch. Ok for trunk and a backport into gcc-14? gcc/ChangeLog: * config/aarch64/aarch64-c.cc (aarch64_define_unconditional_macros): Add missing __ARM_NEON_SVE_BRIDGE. On 6/6/24 13:20, Richard Sandiford wrote: > Richard Ball <richard.ball@arm.com> writes: >> __ARM_NEON_SVE_BRIDGE was missed in the original patch and is >> added by this patch. >> >> Ok for trunk and a backport into gcc-14? >> >> gcc/ChangeLog: >> >> * config/aarch64/aarch64-c.cc (aarch64_update_cpp_builtins): >> Add missing __ARM_NEON_SVE_BRIDGE. > > After this patch was posted, there was some internal discussion > involving LLVM & GNU devs about what this kind of macro means, now that > we have FMV. The feeling was that __ARM_NEON_SVE_BRIDGE should just > indicate whether the compiler provides the file, not whether AdvSIMD > & SVE are enabled. I think we should therefore add this to > aarch64_define_unconditional_macros instead. > > Sorry for the slow review. I was waiting for the outcome of that > discussion before replying. > > Thanks, > Richard > >> diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc >> index fe1a20e4e546a68e5f7eddff3bbb0d3e831fbd9b..1121be118cf8d05e3736ad4ee75568ff7cb92bfd 100644 >> --- a/gcc/config/aarch64/aarch64-c.cc >> +++ b/gcc/config/aarch64/aarch64-c.cc >> @@ -260,6 +260,7 @@ aarch64_update_cpp_builtins (cpp_reader *pfile) >> aarch64_def_or_undef (TARGET_SME_I16I64, "__ARM_FEATURE_SME_I16I64", pfile); >> aarch64_def_or_undef (TARGET_SME_F64F64, "__ARM_FEATURE_SME_F64F64", pfile); >> aarch64_def_or_undef (TARGET_SME2, "__ARM_FEATURE_SME2", pfile); >> + aarch64_def_or_undef (TARGET_SVE, "__ARM_NEON_SVE_BRIDGE", pfile); >> >> /* Not for ACLE, but required to keep "float.h" correct if we switch >> target between implementations that do or do not support ARMv8.2-A
Richard Ball <richard.ball@arm.com> writes: > v2: Change macro definition following internal discussion. > > __ARM_NEON_SVE_BRIDGE was missed in the original patch and is > added by this patch. > > Ok for trunk and a backport into gcc-14? Yes, thanks. Richard > gcc/ChangeLog: > > * config/aarch64/aarch64-c.cc (aarch64_define_unconditional_macros): > Add missing __ARM_NEON_SVE_BRIDGE. > > On 6/6/24 13:20, Richard Sandiford wrote: >> Richard Ball <richard.ball@arm.com> writes: >>> __ARM_NEON_SVE_BRIDGE was missed in the original patch and is >>> added by this patch. >>> >>> Ok for trunk and a backport into gcc-14? >>> >>> gcc/ChangeLog: >>> >>> * config/aarch64/aarch64-c.cc (aarch64_update_cpp_builtins): >>> Add missing __ARM_NEON_SVE_BRIDGE. >> >> After this patch was posted, there was some internal discussion >> involving LLVM & GNU devs about what this kind of macro means, now that >> we have FMV. The feeling was that __ARM_NEON_SVE_BRIDGE should just >> indicate whether the compiler provides the file, not whether AdvSIMD >> & SVE are enabled. I think we should therefore add this to >> aarch64_define_unconditional_macros instead. >> >> Sorry for the slow review. I was waiting for the outcome of that >> discussion before replying. >> >> Thanks, >> Richard >> >>> diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc >>> index fe1a20e4e546a68e5f7eddff3bbb0d3e831fbd9b..1121be118cf8d05e3736ad4ee75568ff7cb92bfd 100644 >>> --- a/gcc/config/aarch64/aarch64-c.cc >>> +++ b/gcc/config/aarch64/aarch64-c.cc >>> @@ -260,6 +260,7 @@ aarch64_update_cpp_builtins (cpp_reader *pfile) >>> aarch64_def_or_undef (TARGET_SME_I16I64, "__ARM_FEATURE_SME_I16I64", pfile); >>> aarch64_def_or_undef (TARGET_SME_F64F64, "__ARM_FEATURE_SME_F64F64", pfile); >>> aarch64_def_or_undef (TARGET_SME2, "__ARM_FEATURE_SME2", pfile); >>> + aarch64_def_or_undef (TARGET_SVE, "__ARM_NEON_SVE_BRIDGE", pfile); >>> >>> /* Not for ACLE, but required to keep "float.h" correct if we switch >>> target between implementations that do or do not support ARMv8.2-A > > diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc > index fe1a20e4e546a68e5f7eddff3bbb0d3e831fbd9b..d042e5fbd8c562df2e4538b51b960c194d2ca2c9 100644 > --- a/gcc/config/aarch64/aarch64-c.cc > +++ b/gcc/config/aarch64/aarch64-c.cc > @@ -75,6 +75,7 @@ aarch64_define_unconditional_macros (cpp_reader *pfile) > > builtin_define ("__ARM_STATE_ZA"); > builtin_define ("__ARM_STATE_ZT0"); > + builtin_define ("__ARM_NEON_SVE_BRIDGE"); > > /* Define keyword attributes like __arm_streaming as macros that expand > to the associated [[...]] attribute. Use __extension__ in the attribute
diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc index fe1a20e4e546a68e5f7eddff3bbb0d3e831fbd9b..1121be118cf8d05e3736ad4ee75568ff7cb92bfd 100644 --- a/gcc/config/aarch64/aarch64-c.cc +++ b/gcc/config/aarch64/aarch64-c.cc @@ -260,6 +260,7 @@ aarch64_update_cpp_builtins (cpp_reader *pfile) aarch64_def_or_undef (TARGET_SME_I16I64, "__ARM_FEATURE_SME_I16I64", pfile); aarch64_def_or_undef (TARGET_SME_F64F64, "__ARM_FEATURE_SME_F64F64", pfile); aarch64_def_or_undef (TARGET_SME2, "__ARM_FEATURE_SME2", pfile); + aarch64_def_or_undef (TARGET_SVE, "__ARM_NEON_SVE_BRIDGE", pfile); /* Not for ACLE, but required to keep "float.h" correct if we switch target between implementations that do or do not support ARMv8.2-A