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

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

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. 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.