[cfe-dev] [libcxx] optimizing shared_ptr atomics in destructors

[David Chisnall via cfe-dev]

On 19 Jul 2016, at 16:05, Craig, Ben <ben.craig at codeaurora.org> wrote:
> 
> On 7/19/2016 3:52 AM, David Chisnall wrote:
>> On 18 Jul 2016, at 19:09, Craig, Ben via cfe-dev <cfe-dev at lists.llvm.org>
>>  wrote:
>> 
>>>         ldr     r1, [r0]  @bcraig note: it would be nice to combine this load with the ldrex
>>> 
>> I believe that you will see this transformation if you do a non-atomic increment on the loaded value and then a weak compare and swap to write the result back (with a back branch to retry if it failed).  I’m not 100% sure though, because this will leave a ldrex that isn’t paired with a store / release in the case that the loaded value is 0 and some ARM cores don’t like it when that happens.
> No luck.  With this body instead...
>     auto val = __atomic_load_n(&__shared_weak_owners_, 2 /*_AO_Acquire*/);
>     while(val != 0 && !__atomic_compare_exchange_n(&__shared_weak_owners_, &val, val-1, true /*weak*/, 4 /*Acq_Rel*/, 2 /*Acquire*/));
>     if(val == 0) __on_zero_shared_weak();
> 
> I still get ldr, ldrex, and barriers for both.  Even worse, this approach makes x86 a lot worse.  I get a lock CMPXCHG loop instead of a lock XADD.

That’s quite surprising (not the x86 part, that’s expected).  The expand atomics pass is meant to merge qualifying load into the corresponding ldrex, though it might only work for a non-atomic initial load.  I wonder if it would make sense for this pass to expand atomicrmw into ll/sc pairs as a first pass, run the other optimisations, and then collapse them again (actually, I think for ARMv<8.1 it does the expansion early already, but it’s been a while since I looked at the code).

David

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3719 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160719/d4bed9bd/attachment.bin>