Re: [PATCH] MdePkg/Include: AARCH64: disable outline atomics on GCC 10.2+

["Laszlo Ersek" <lersek@redhat.com>]

On 05/22/20 22:01, Laszlo Ersek wrote:
> On 05/22/20 12:12, Ard Biesheuvel wrote:
>> GCC 10 enabled a feature by default that was introduced in GCC 9,
>> which results in atomic operations to be emitted as function calls
>> to intrinsics provided by a runtime library.
>>
>> Atomics are hardly used in EDK2, which runs on a single CPU anyway,
>> and any benefit that would result from reusing library code that
>> implements these operations is defeated by the fact that every EDK2
>> module will need to have its own copy anyway.
>>
>> So let's disable this feature on GCC versions that support the
>> pragma to do so (GCC 10.2 and up)
>>
>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> ---
>> The GCC support for this pragma has already been pulled into the 10.2
>> release branch. I think we should consider adding this to the stable
>> tag, so that the issue can easily be resolved by upgrading the compiler.
>> Whether we add the intrinsics too is a separate matter, but we can
>> revisit that later.
>>
>>  MdePkg/Include/AArch64/ProcessorBind.h | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
>> index 896bf273ac7a..a3ca8f09e51c 100644
>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
>> @@ -24,6 +24,17 @@
>>  #pragma pack()
>>  #endif
>>  
>> +#if defined(__GNUC__) && !defined(__clang__)
>> +
>> +//
>> +// Disable GCC outline atomics
>> +// Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2723
>> +//
>> +#if __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 2)
>> +#pragma GCC target "no-outline-atomics"
>> +#endif
>> +#endif
>> +
>>  #if defined(_MSC_EXTENSIONS)
>>  
>>  //
>>
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> But I think it should be merged later, after GCC 10.2 is out.
> 
> (Obviously I don't "insist" that we follow this approach, I'm just OK
> with it.)

Oh and I think both this patch and the assembly language implementation
for the atomics should be delayed after the stable tag. gcc-10 is a new
toolchain; so even if we don't introduce a new toolchain tag such as
GCC10 for it, whatever we do in order to make it work, that's feature
enablement in my book.

Thanks,
Laszlo