diff mbox series

aarch64: Add missing ACLE macro for NEON-SVE Bridge

Message ID e9de1323-a0f3-44d2-9b1d-6d2d9a46d263@arm.com
State New
Headers show
Series aarch64: Add missing ACLE macro for NEON-SVE Bridge | expand

Commit Message

Richard Ball May 31, 2024, 10:37 a.m. UTC
__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.

Comments

Richard Sandiford June 6, 2024, 12:20 p.m. UTC | #1
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 June 6, 2024, 3:20 p.m. UTC | #2
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 Sandiford June 6, 2024, 3:25 p.m. UTC | #3
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 mbox series

Patch

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