Re: [edk2-devel] [PATCH v2] ArmPkg/CompilerIntrinsicsLib: provide atomics intrinsics

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

On 05/22/20 15:27, Ard Biesheuvel wrote:
> On 5/22/20 12:54 PM, Leif Lindholm wrote:
>> On Thu, May 21, 2020 at 22:22:58 +0200, Laszlo Ersek wrote:
>>> On 05/21/20 16:16, Leif Lindholm wrote:
>>>
>>>> OK, then I would vote *for* merging the patch regardless. We know how
>>>> long some toolchain versions can stick around simply because they were
>>>> mentioned in some blog post somewhere that ended up high in search
>>>> rankings.
>>>>
>>>> Once gcc 10.2 is released (and we have verified the problem can be
>>>> worked around elsewhere), I guess we could add a note saying "once all
>>>> gcc 10.0 and 10.1 toolchains are considered obsolete, this file can
>>>> be deleted".
>>>
>>> I think we can expect all distros that ship gcc-10 to eventually migrate
>>> to gcc-10.2+. Until then, this patch should hopefully work. (I'm quite
>>> annoyed by having to call the patch "temporary", as it feels very
>>> technically impressive.)
>>>
>>> So I think I agree with Leif, with a small modification to the idea:
>>> rather than a *note* saying "back this out once 10.0 and 10.1 have been
>>> replaced by 10.2+ in all 'large' distros"
>>
>> That isn't actually exatly what I meant - I meant properly obsolete
>> as in "we are now reasonably certain no one is still using some silly
>> ancient cross compiler they checked into their build infrastructure
>> years ago".
>>
>>> , I would suggest filing a *BZ*
>>> for the same. And I recommend making the new BZ dependent on
>>> TianoCore#2723 (i.e. the present BZ).
>>
>> But I don't object to that approach.
>>
> 
> OK, so i will leave it up to Liming and the stewards to decide whether
> this gets incorporated ino the stable tag or not.

I'd delay it -- first, maybe have some more discussion around it,
second, we can consider this "GCC10 feature enablement".

IMO anyway.

Thanks
Laszlo


> If it is, I would like
> to fold in the fixup below
> 
> --- a/ArmPkg/Library/CompilerIntrinsicsLib/AArch64/Atomics.S
> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/AArch64/Atomics.S
> @@ -53,10 +53,10 @@
>  0:     ld\a\()xr\s     r0_\sz, [x1]
>         .ifnc           \insn, swp
>         \opc            tmp1_\sz, r0_\sz, tmp0_\sz
> +       st\l\()xr\s     w15, tmp1_\sz, [x1]
>         .else
> -       \opc            tmp1_\sz, tmp0_\sz
> +       st\l\()xr\s     w15, tmp0_\sz, [x1]
>         .endif
> -       st\l\()xr\s     w15, tmp1_\sz, [x1]
>         cbnz            w15, 0b
>         ret
>         fn_end          __aarch64_\insn\()\sz\()\model
> 
> to get rid of the redundant 'mov' for the SWP flavor of the atomics
> helpers.
> 
>