[WoA] Use fences for sequentially consistent stores/writes

[mnadeem]

Summary

LLVM currently uses LDAR/STLR and variants for acquire/release
as well as seq_cst operations. This is fine as long as all code uses
this convention.

Normally LDAR/STLR act as one way barriers but when used in
combination they provide a sequentially consistent model. i.e.
when an LDAR appears after an STLR in program order the STLR
acts as a two way fence and the store will be observed before
the load.

The problem is that normal loads (unlike ldar), when they appear
after the STLR can be observed before STLR (if my understanding
is correct). Possibly providing weaker than expected guarantees if
they are used for ordered atomic operations.

Unfortunately in Microsoft Visual Studio STL seq_cst ld/st are
implemented using normal load/stores and explicit fences:
dmb ish + str + dmb ish
ldr + dmb ish

This patch uses fences for MSVC target whenever we write to the
memory in a sequentially consistent way so that we don't rely on
the assumptions that just using LDAR/STLR will give us sequentially
consistent ordering.

[efriedma]

The explanation makes sense.

Can we make this specifically apply to _Interlocked* functions, instead of all atomics on Windows targets? I'd prefer not to impose this performance penalty on other users of atomics if we can avoid it.

It looks like for atomic rmw ops, MSVC generates one barrier, not two; can we do the same?

[BillyONeal]

In D141748#4060636, @efriedma wrote:

The explanation makes sense.

Can we make this specifically apply to _Interlocked* functions, instead of all atomics on Windows targets? I'd prefer not to impose this performance penalty on other users of atomics if we can avoid it.

To be clear this is about the ABI of std::atomic, and we are likely to change that when we get to break that Someday. Interlocked doesn't come with plain load/store ops (because, I assume, it was assumed plain volatiles were sufficient for that)

It looks like for atomic rmw ops, MSVC generates one barrier, not two; can we do the same?

Let me poke our backend folks...

[john.brawn]

It looks like if MSVC is implementing sequentially-consistent atomic operations in the manner described then we will need to stores (but not loads) in the same way, so it looks like this patch is doing the right thing. Reasoning below:

Using the memory model simulator at http://diy.inria.fr/www/?record=aarch64, the following input has three threads purely using LDAR/STRL

AArch64 SeqCst
{
0:X1=x; 1:X1=x; 2:X1=x;
0:X3=y; 1:X3=y; 2:X3=y;
}
 P0            | P1            | P2 ;
 MOV W0, #1    | MOV W2, #1    | LDAR W2, [X3] ;
 STLR W0, [X1] | STLR W2, [X3] | LDAR W0, [X1] ;
 LDAR W2, [X3] | LDAR W0, [X1] | ;
exists(2:X2=1 /\ 2:X0=0 /\ 0:X2=0 /\ 1:X0=1)

gives the result "No", which means it's not possible for P2 to see X2=1 and X0=0 if P0 sees X2=0 and P1 sees X0=1, or in other words using LDAR/STRL gives sequentially consistent behaviour.

The following has thread P0 do STLR followed by a MSVC-style sequentially consistent load

AArch64 SeqCst
{
0:X1=x; 1:X1=x; 2:X1=x;
0:X3=y; 1:X3=y; 2:X3=y;
}
 P0            | P1            | P2 ;
 MOV W0, #1    | MOV W2, #1    | LDAR W2, [X3] ;
 STLR W0, [X1] | STLR W2, [X3] | LDAR W0, [X1] ;
 LDR W2, [X3]  | LDAR W0, [X1] | ;
 DMB ISH       |               | ;
exists(2:X2=1 /\ 2:X0=0 /\ 0:X2=0 /\ 1:X0=1)

this gives the result "OK", meaning it is possible for these threads to observe that, which means that we don't have sequentially consistent behaviour.

For the MSVC-style sequentially consistent store followed by LDAR

AArch64 SeqCst
{
0:X1=x; 1:X1=x; 2:X1=x;
0:X3=y; 1:X3=y; 2:X3=y;
}
 P0            | P1            | P2 ;
 MOV W0, #1    | MOV W2, #1    | LDAR W2, [X3] ;
 DMB ISH       |               | ;
 STR W0, [X1]  | STLR W2, [X3] | LDAR W0, [X1] ;
 DMB ISH       |               | ;
 LDAR W2, [X3] | LDAR W0, [X1] | ;
exists(2:X2=1 /\ 2:X0=0 /\ 0:X2=0 /\ 1:X0=1)

we get "No", so we still do get sequentially consistent behaviour in this case.

[mnadeem]

In D141748#4060636, @efriedma wrote:

The explanation makes sense.

Can we make this specifically apply to _Interlocked* functions, instead of all atomics on Windows targets? I'd prefer not to impose this performance penalty on other users of atomics if we can avoid it.

IIUC we must add a fence after every sequentially consistent write to memory.

It looks like for atomic rmw ops, MSVC generates one barrier, not two; can we do the same?

They generate acquire/release instructions with implicit fences + one trailing explicit fence, while this patch generates normal monotonic st/ld, so two explicit fences are needed but the fences are outside the loop.
We could do something similar to MSVC if it helps performance. It would only involve adding a trailing fence while keeping the current acquire/release instructions.

I can think of a few possible ways to do this:

1. Modify clang to add a fence seq_cst after the interlock intrinsics, I think we would also need to modify any other builtins that end up generating seq_cst writes to memory, so it might get messy.
2. Add something like TLI->shouldInsertTrailingFenceForAtomic(I) to use in AtomicExpandPass and generate a Trailing Fence while keeping the original memory ordering. With the current patch we set the ordering to Monotonic and thus also need a leading fence.
3. Add a fence during lowering. Not sure how much effort this will take.

[efriedma]

In D141748#4063352, @mnadeem wrote:

In D141748#4060636, @efriedma wrote:

The explanation makes sense.

Can we make this specifically apply to _Interlocked* functions, instead of all atomics on Windows targets? I'd prefer not to impose this performance penalty on other users of atomics if we can avoid it.

IIUC we must add a fence after every sequentially consistent write to memory.

The scenario I wanted to avoid is that someone has an existing codebase that isn't using Microsoft's STL, and is working correctly, but we pessimize the performance to fix a bug that never affected it in the first place. But I guess there are two issues with that:

• To maintain sequential consistency between all atomic ops, we'd need to ensure that nothing in the process is using the Microsoft STL <atomic>, which maybe isn't practical.
• It looks like MSVC is going to implement _Atomic the same way (see https://devblogs.microsoft.com/cppblog/c11-atomics-in-visual-studio-2022-version-17-5-preview-2/); in that case, there are no intrinsics involved, so I guess we need to precisely copy whatever MSVC does.

Maybe we can expose the optimal sequence as a command-line option for codebases that don't need ABI compatibility.

It looks like for atomic rmw ops, MSVC generates one barrier, not two; can we do the same?

They generate acquire/release instructions with implicit fences + one trailing explicit fence, while this patch generates normal monotonic st/ld, so two explicit fences are needed but the fences are outside the loop.
We could do something similar to MSVC if it helps performance.

I assume fewer fences helps performance, although I haven't tried it. Using the acquire/release instructions also means we do the right thing if there's code using the normal armv8 atomic sequences.

2. Add something like TLI->shouldInsertTrailingFenceForAtomic(I) to use in AtomicExpandPass and generate a Trailing Fence while keeping the original memory ordering. With the current patch we set the ordering to Monotonic and thus also need a leading fence.

This seems fine.

[mnadeem]

• Use fewer barriers and existing acquire/release instruction sequence.
• Add a shouldInsertTrailingFenceForAtomicStore() function.

[efriedma]

Code LGTM; maybe wait to see if Microsoft has any more feedback before we merge.

llvm/include/llvm/CodeGen/TargetLowering.h

public:
 
   /// Whether AtomicExpandPass should automatically insert fences and reduce
   /// ordering for this atomic. This should be true for most architectures with
   /// weak memory ordering. Defaults to false.
   virtual bool shouldInsertFencesForAtomic(const Instruction *I) const {
     return false;
   }
 
+  /// Whether AtomicExpandPass should automatically insert a trailing fence
+  /// without reducing the ordering for this atomic. Defaults to false.
+  virtual bool
+  shouldInsertTrailingFenceForAtomicStore(const Instruction *I) const {
+    return false;
+  }
+
   /// Perform a load-linked operation on Addr, returning a "Value *" with the
   /// corresponding pointee type. This may entail some non-trivial operations to
   /// truncate or reconstruct types that will be illegal in the backend. See
   /// ARMISelLowering for an example implementation.
   virtual Value *emitLoadLinked(IRBuilderBase &Builder, Type *ValueTy,
                                 Value *Addr, AtomicOrdering Ord) const {
     llvm_unreachable("Load linked unimplemented on this target");
   }

llvm/lib/CodeGen/AtomicExpandPass.cpp

if (TLI->shouldInsertFencesForAtomic(I)) {
         FenceOrdering = CASI->getMergedOrdering();
         CASI->setSuccessOrdering(AtomicOrdering::Monotonic);
         CASI->setFailureOrdering(AtomicOrdering::Monotonic);
       }
 
       if (FenceOrdering != AtomicOrdering::Monotonic) {
         MadeChange |= bracketInstWithFences(I, FenceOrdering);
       }
+    } else if (I->hasAtomicStore() &&
+               TLI->shouldInsertTrailingFenceForAtomicStore(I)) {
+      auto FenceOrdering = AtomicOrdering::Monotonic;
+      if (SI)
+        FenceOrdering = SI->getOrdering();
+      else if (RMWI)
+        FenceOrdering = RMWI->getOrdering();
+      else if (CASI && TLI->shouldExpandAtomicCmpXchgInIR(CASI) !=
+                           TargetLoweringBase::AtomicExpansionKind::LLSC)
+        // LLSC is handled in expandAtomicCmpXchg().
+        FenceOrdering = CASI->getSuccessOrdering();
+
+      IRBuilder Builder(I);
+      if (auto TrailingFence =
+              TLI->emitTrailingFence(Builder, I, FenceOrdering)) {
+        TrailingFence->moveAfter(I);
+        MadeChange = true;
+      }
     }
 
     if (LI)
       MadeChange |= tryExpandAtomicLoad(LI);
     else if (SI)
       MadeChange |= tryExpandAtomicStore(SI);
     else if (RMWI) {
       // There are two different ways of expanding RMW instructions:
if (HasReleasedLoadBB) {
     // Update PHI node in TryStoreBB.
     LoadedTryStore->addIncoming(SecondLoad, ReleasedLoadBB);
   } else
     Builder.CreateUnreachable();
 
   // Make sure later instructions don't get reordered with a fence if
   // necessary.
   Builder.SetInsertPoint(SuccessBB);
-  if (ShouldInsertFencesForAtomic)
+  if (ShouldInsertFencesForAtomic ||
+      TLI->shouldInsertTrailingFenceForAtomicStore(CI))
     TLI->emitTrailingFence(Builder, CI, SuccessOrder);
   Builder.CreateBr(ExitBB);
 
   Builder.SetInsertPoint(NoStoreBB);
   PHINode *LoadedNoStore =
       Builder.CreatePHI(UnreleasedLoad->getType(), 2, "loaded.nostore");
   LoadedNoStore->addIncoming(UnreleasedLoad, StartBB);
   if (HasReleasedLoadBB)

llvm/lib/Target/AArch64/AArch64ISelLowering.h

Value *emitLoadLinked(IRBuilderBase &Builder, Type *ValueTy, Value *Addr,
                         AtomicOrdering Ord) const override;
   Value *emitStoreConditional(IRBuilderBase &Builder, Value *Val, Value *Addr,
                               AtomicOrdering Ord) const override;
 
   void emitAtomicCmpXchgNoStoreLLBalance(IRBuilderBase &Builder) const override;
 
   bool isOpSuitableForLDPSTP(const Instruction *I) const;
   bool shouldInsertFencesForAtomic(const Instruction *I) const override;
+  bool
+  shouldInsertTrailingFenceForAtomicStore(const Instruction *I) const override;
 
   TargetLoweringBase::AtomicExpansionKind
   shouldExpandAtomicLoadInIR(LoadInst *LI) const override;
   TargetLoweringBase::AtomicExpansionKind
   shouldExpandAtomicStoreInIR(StoreInst *SI) const override;
   TargetLoweringBase::AtomicExpansionKind
   shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const override;
 

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

bool AArch64TargetLowering::isOpSuitableForLDPSTP(const Instruction *I) const {
   return false;
 }
 
 bool AArch64TargetLowering::shouldInsertFencesForAtomic(
     const Instruction *I) const {
   return isOpSuitableForLDPSTP(I);
 }
 
+bool AArch64TargetLowering::shouldInsertTrailingFenceForAtomicStore(
+    const Instruction *I) const {
+  // Store-Release instructions only provide seq_cst guarantees when paired with
+  // Load-Acquire instructions. MSVC CRT does not use these instructions to
+  // implement seq_cst loads and stores, so we need additional explicit fences
+  // after memory writes.
+  if (!Subtarget->getTargetTriple().isWindowsMSVCEnvironment())
+    return false;
+
+  switch (I->getOpcode()) {
+  default:
+    return false;
+  case Instruction::AtomicCmpXchg:
+    return cast<AtomicCmpXchgInst>(I)->getSuccessOrdering() ==
+           AtomicOrdering::SequentiallyConsistent;
+  case Instruction::AtomicRMW:
+    return cast<AtomicRMWInst>(I)->getOrdering() ==
+           AtomicOrdering::SequentiallyConsistent;
+  case Instruction::Store:
+    return cast<StoreInst>(I)->getOrdering() ==
+           AtomicOrdering::SequentiallyConsistent;
+  }
+}
+
 // Loads and stores less than 128-bits are already atomic; ones above that
 // are doomed anyway, so defer to the default libcall and blame the OS when
 // things go wrong.
 TargetLoweringBase::AtomicExpansionKind
 AArch64TargetLowering::shouldExpandAtomicStoreInIR(StoreInst *SI) const {
   unsigned Size = SI->getValueOperand()->getType()->getPrimitiveSizeInBits();
   if (Size != 128 || isOpSuitableForLDPSTP(SI))
     return AtomicExpansionKind::None;

llvm/test/CodeGen/AArch64/atomic-ops-msvc.ll

 ; CHECK-NEXT:  .LBB0_1: // %atomicrmw.start
 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    ldaxrb w8, [x9]
 ; CHECK-NEXT:    add w10, w8, w0
 ; CHECK-NEXT:    stlxrb w11, w10, [x9]
 ; CHECK-NEXT:    cbnz w11, .LBB0_1
 ; CHECK-NEXT:  // %bb.2: // %atomicrmw.end
 ; CHECK-NEXT:    mov w0, w8
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
    %old = atomicrmw add ptr @var8, i8 %offset seq_cst
    ret i8 %old
 }
 
 define dso_local i16 @test_atomic_load_add_i16(i16 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_add_i16:
 ; CHECK:       // %bb.0:

 ; CHECK-NEXT:    ret
    %old = atomicrmw sub ptr @var32, i32 %offset acquire
    ret i32 %old
 }
 
 define dso_local i64 @test_atomic_load_sub_i64(i64 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_sub_i64:
 ; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov x8, x0
 ; CHECK-NEXT:    adrp x9, var64
 ; CHECK-NEXT:    add x9, x9, :lo12:var64
 ; CHECK-NEXT:  .LBB7_1: // %atomicrmw.start
 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
-; CHECK-NEXT:    ldaxr x8, [x9]
-; CHECK-NEXT:    sub x10, x8, x0
+; CHECK-NEXT:    ldaxr x0, [x9]
+; CHECK-NEXT:    sub x10, x0, x8
 ; CHECK-NEXT:    stlxr w11, x10, [x9]
 ; CHECK-NEXT:    cbnz w11, .LBB7_1
 ; CHECK-NEXT:  // %bb.2: // %atomicrmw.end
-; CHECK-NEXT:    mov x0, x8
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
    %old = atomicrmw sub ptr @var64, i64 %offset seq_cst
    ret i64 %old
 }
 
 define dso_local i8 @test_atomic_load_and_i8(i8 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_and_i8:
 ; CHECK:       // %bb.0:

 ; CHECK-NEXT:  .LBB10_1: // %atomicrmw.start
 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    ldaxr w8, [x9]
 ; CHECK-NEXT:    and w10, w8, w0
 ; CHECK-NEXT:    stlxr w11, w10, [x9]
 ; CHECK-NEXT:    cbnz w11, .LBB10_1
 ; CHECK-NEXT:  // %bb.2: // %atomicrmw.end
 ; CHECK-NEXT:    mov w0, w8
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
    %old = atomicrmw and ptr @var32, i32 %offset seq_cst
    ret i32 %old
 }
 
 define dso_local i64 @test_atomic_load_and_i64(i64 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_and_i64:
 ; CHECK:       // %bb.0:

 ; CHECK-NEXT:  .LBB12_1: // %atomicrmw.start
 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    ldaxrb w8, [x9]
 ; CHECK-NEXT:    orr w10, w8, w0
 ; CHECK-NEXT:    stlxrb w11, w10, [x9]
 ; CHECK-NEXT:    cbnz w11, .LBB12_1
 ; CHECK-NEXT:  // %bb.2: // %atomicrmw.end
 ; CHECK-NEXT:    mov w0, w8
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
    %old = atomicrmw or ptr @var8, i8 %offset seq_cst
    ret i8 %old
 }
 
 define dso_local i16 @test_atomic_load_or_i16(i16 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_or_i16:
 ; CHECK:       // %bb.0:

 ; CHECK-NEXT:  .LBB18_1: // %atomicrmw.start
 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    ldaxr w8, [x9]
 ; CHECK-NEXT:    eor w10, w8, w0
 ; CHECK-NEXT:    stlxr w11, w10, [x9]
 ; CHECK-NEXT:    cbnz w11, .LBB18_1
 ; CHECK-NEXT:  // %bb.2: // %atomicrmw.end
 ; CHECK-NEXT:    mov w0, w8
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
    %old = atomicrmw xor ptr @var32, i32 %offset seq_cst
    ret i32 %old
 }
 
 define dso_local i64 @test_atomic_load_xor_i64(i64 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_xor_i64:
 ; CHECK:       // %bb.0:

 ; CHECK-NEXT:    add x9, x9, :lo12:var16
 ; CHECK-NEXT:  .LBB21_1: // %atomicrmw.start
 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    ldaxrh w8, [x9]
 ; CHECK-NEXT:    stlxrh w10, w0, [x9]
 ; CHECK-NEXT:    cbnz w10, .LBB21_1
 ; CHECK-NEXT:  // %bb.2: // %atomicrmw.end
 ; CHECK-NEXT:    mov w0, w8
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
    %old = atomicrmw xchg ptr @var16, i16 %offset seq_cst
    ret i16 %old
 }
 
 define dso_local i32 @test_atomic_load_xchg_i32(i32 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_xchg_i32:
 ; CHECK:       // %bb.0:

 ; CHECK-NEXT:    ret
    %old = atomicrmw min ptr @var32, i32 %offset monotonic
    ret i32 %old
 }
 
 define dso_local i64 @test_atomic_load_min_i64(i64 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_min_i64:
 ; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov x8, x0
 ; CHECK-NEXT:    adrp x9, var64
 ; CHECK-NEXT:    add x9, x9, :lo12:var64
 ; CHECK-NEXT:  .LBB27_1: // %atomicrmw.start
 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
-; CHECK-NEXT:    ldaxr x8, [x9]
-; CHECK-NEXT:    cmp x8, x0
-; CHECK-NEXT:    csel x10, x8, x0, le
+; CHECK-NEXT:    ldaxr x0, [x9]
+; CHECK-NEXT:    cmp x0, x8
+; CHECK-NEXT:    csel x10, x0, x8, le
 ; CHECK-NEXT:    stlxr w11, x10, [x9]
 ; CHECK-NEXT:    cbnz w11, .LBB27_1
 ; CHECK-NEXT:  // %bb.2: // %atomicrmw.end
-; CHECK-NEXT:    mov x0, x8
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
    %old = atomicrmw min ptr @var64, i64 %offset seq_cst
    ret i64 %old
 }
 
 define dso_local i8 @test_atomic_load_max_i8(i8 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_max_i8:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    adrp x9, var8
 ; CHECK-NEXT:    add x9, x9, :lo12:var8
 ; CHECK-NEXT:  .LBB28_1: // %atomicrmw.start
 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    ldaxrb w10, [x9]
 ; CHECK-NEXT:    sxtb w8, w10
 ; CHECK-NEXT:    cmp w8, w0, sxtb
 ; CHECK-NEXT:    csel w10, w10, w0, gt
 ; CHECK-NEXT:    stlxrb w11, w10, [x9]
 ; CHECK-NEXT:    cbnz w11, .LBB28_1
 ; CHECK-NEXT:  // %bb.2: // %atomicrmw.end
 ; CHECK-NEXT:    mov w0, w8
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
    %old = atomicrmw max ptr @var8, i8 %offset seq_cst
    ret i8 %old
 }
 
 define dso_local i16 @test_atomic_load_max_i16(i16 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_max_i16:
 ; CHECK:       // %bb.0:

 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    ldaxr w8, [x9]
 ; CHECK-NEXT:    cmp w8, w0
 ; CHECK-NEXT:    csel w10, w8, w0, ls
 ; CHECK-NEXT:    stlxr w11, w10, [x9]
 ; CHECK-NEXT:    cbnz w11, .LBB34_1
 ; CHECK-NEXT:  // %bb.2: // %atomicrmw.end
 ; CHECK-NEXT:    mov w0, w8
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
    %old = atomicrmw umin ptr @var32, i32 %offset seq_cst
    ret i32 %old
 }
 
 define dso_local i64 @test_atomic_load_umin_i64(i64 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_umin_i64:
 ; CHECK:       // %bb.0:

 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    ldaxr w8, [x9]
 ; CHECK-NEXT:    cmp w8, w0
 ; CHECK-NEXT:    csel w10, w8, w0, hi
 ; CHECK-NEXT:    stlxr w11, w10, [x9]
 ; CHECK-NEXT:    cbnz w11, .LBB38_1
 ; CHECK-NEXT:  // %bb.2: // %atomicrmw.end
 ; CHECK-NEXT:    mov w0, w8
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
    %old = atomicrmw umax ptr @var32, i32 %offset seq_cst
    ret i32 %old
 }
 
 define dso_local i64 @test_atomic_load_umax_i64(i64 %offset) nounwind {
 ; CHECK-LABEL: test_atomic_load_umax_i64:
 ; CHECK:       // %bb.0:

 ; CHECK-NEXT:    // =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    ldaxrh w0, [x9]
 ; CHECK-NEXT:    cmp w0, w8
 ; CHECK-NEXT:    b.ne .LBB41_4
 ; CHECK-NEXT:  // %bb.2: // %cmpxchg.trystore
 ; CHECK-NEXT:    // in Loop: Header=BB41_1 Depth=1
 ; CHECK-NEXT:    stlxrh w10, w1, [x9]
 ; CHECK-NEXT:    cbnz w10, .LBB41_1
-; CHECK-NEXT:  // %bb.3: // %cmpxchg.end
+; CHECK-NEXT:  // %bb.3: // %cmpxchg.success
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    // kill: def $w0 killed $w0 killed $x0
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB41_4: // %cmpxchg.nostore
 ; CHECK-NEXT:    clrex
 ; CHECK-NEXT:    // kill: def $w0 killed $w0 killed $x0
 ; CHECK-NEXT:    ret
    %pair = cmpxchg ptr @var16, i16 %wanted, i16 %new seq_cst seq_cst
    %old = extractvalue { i16, i1 } %pair, 0

 }
 
 define dso_local void @test_atomic_store_seq_cst_i8(i8 %val) nounwind {
 ; CHECK-LABEL: test_atomic_store_seq_cst_i8:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    adrp x8, var8
 ; CHECK-NEXT:    add x8, x8, :lo12:var8
 ; CHECK-NEXT:    stlrb w0, [x8]
+; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ret
   store atomic i8 %val, ptr @var8 seq_cst, align 1
   ret void
 }
 
 define dso_local void @test_atomic_store_monotonic_i16(i16 %val) nounwind {
 ; CHECK-LABEL: test_atomic_store_monotonic_i16:
 ; CHECK:       // %bb.0: