[llvm-dev] [atomics][AArch64] Possible bug in cmpxchg lowering

[Friedman, Eli via llvm-dev]

On 5/30/2017 3:29 PM, Geoff Berry via llvm-dev wrote:
>
> Currently the AtomicExpandPass will lower the following IR:
>
> define i1 @foo(i32* %obj, i32 %old, i32 %new) {
> entry:
>   %v0 = cmpxchg weak volatile i32* %obj, i32 %old, i32 %new _*release 
> acquire*_
>   %v1 = extractvalue { i32, i1 } %v0, 1
>   ret i1 %v1
> }
>
> to the equivalent of the following on AArch64:
>
> _*ldxr    w8, [x0]*_
>     cmp        w8, w1
>     b.ne    .LBB0_3
> // BB#1:                                // %cmpxchg.trystore
>     stlxr    w8, w2, [x0]
>     cbz    w8, .LBB0_4
> // BB#2:                                // %cmpxchg.failure
>     mov     w0, wzr
>     ret
> .LBB0_3:                                // %cmpxchg.nostore
>     clrex
>     mov     w0, wzr
>     ret
> .LBB0_4:
>     orr    w0, wzr, #0x1
>     ret
>
> GCC instead generates a ldaxr for the initial load, which seems more 
> correct to me since it is honoring the requested failure case acquire 
> ordering.  I'd like to get other opinions on this before filing a bug.
>
> I believe the code in AtomicExpand::expandAtomicCmpXchg() is 
> responsible for this discrepancy, since it only uses the failure case 
> memory order for targets that use fences (i.e. when 
> TLI->shouldInsertFencesForAtomic(CI) is true).
>

 From LangRef: "the ordering constraint on failure must be no stronger 
than that on success".

It's not really explained in LangRef, but per C++11, the way we convert 
the success semantic to a failure semantic for cmpxchg is by converted 
acq_rel to acquire and release to relaxed.  Acquire is stronger than 
relaxed, so your cmpxchg is illegal.  Not sure why the verifier isn't 
catching it.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170530/174aa738/attachment.html>