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

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

On 05/26/20 16:37, Leif Lindholm wrote:
> On Sat, May 23, 2020 at 00:09:52 +0200, Ard Biesheuvel wrote:
>>>>> 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.
>>
>> Works for me. By the time the next stable tag comes around, early adopters
>> that are now on GCC 10.1 will likely have moved to 10.2 by that time, and so
>> we may not need the assembly patch at all.
> 
> I'm not ecstatic that we'll be releasing the first stable tag known to
> break with current toolchains.

If this breakage affects "current toolchains", then why was
<https://bugzilla.tianocore.org/show_bug.cgi?id=2723> only reported on
2020-May-19, four days into the soft feature freeze?

> This isn't just affecting random crazies pulling latest toolchains
> down, but people using their distro defaults (native or cross).

... "people using their distro defaults" to *not* build upstream edk2
until 2020-May-19, apparently.

> I don't recall if 10.1 ended up being default in F32, but it was
> definitely included. In Arch, it does appear default.
> 
> Debian/Ubuntu are unaffected in their stable releases.
> 
> I agree it's a transitional issue, but I would really prefer to have
> the intrinsics included in the release.

OK, let's delay the release then, by a few days. I agree the present
patch may qualify as a bugfix, but the other patch with the assembly
language intrinsics doesn't. If it's really that important to have in
the upcoming stable tag, then it's worth delaying the tag for. I'm fine
delaying the release for it; it wouldn't be without precedent.

Also, I think Ard's assembly language patch needs a Tested-by from Gary
at the least (reporter of TianoCore#2723). Please reach out to him in
that thread.

... More precisely, please *ping* Gary for a Tested-by in that thread,
because Ard CC'd him from the start, and even credited Gary in the
commit message.

Thanks,
Laszlo