[flang-commits] [flang] 446318f - [AtomicExpand] Let targets keep the release fence out of the reservation (#214867)
via flang-commits
flang-commits at lists.llvm.org
Fri Oct 23 15:52:50 PDT 2026
Author: Josef Schlehofer
Date: 2026-10-23T22:52:43Z
New Revision: 446318fc34818cd70fda63349c4038a5df1b9722
URL: https://github.com/llvm/llvm-project/commit/446318fc34818cd70fda63349c4038a5df1b9722
DIFF: https://github.com/llvm/llvm-project/commit/446318fc34818cd70fda63349c4038a5df1b9722.diff
LOG: [AtomicExpand] Let targets keep the release fence out of the reservation (#214867)
`expandAtomicCmpXchg` sinks the leading fence into
`cmpxchg.fencedstore`, between the load-linked and the
store-conditional. A strong cmpxchg recovers, because its retry
re-reserves after the fence. A weak cmpxchg has no retry, so where a
fence clears the reservation it can never succeed.
On a 32-bit PowerPC e500v2, `compare_exchange_weak` with any release
ordering never succeeds. One thread, an uncontended cell, a million
consecutive failures:
```
[atomic: compare_exchange_weak] NO PROGRESS after 1000000 tries in 2.28s
[atomic: compare_exchange] ok
[atomic: fetch_or + fetch_add] ok
```
`fetch_update` in Rust's core is a `compare_exchange_weak` retry loop,
so async runtimes can hang on the platform. Reported downstream as
rust-lang/rust#160723.
## The change
- Add `TargetLoweringBase::fenceClearsLoadLinkedReservation()`,
defaulting to false. When a target returns true, a weak cmpxchg gets its
release fence before the load-linked. Every other target keeps its
current placement and codegen.
- On PowerPC, whether a fence clears the reservation is implementation
specific. The new subtarget feature `fence-keeps-reservation` marks CPUs
that keep it and is inherited from pwr7 up, so pwr7 and later, ppc64le
and the AIX default CPU keep the current placement. All other CPUs,
including generic ppc and ppc64 and e500, get the fence before the larx.
Hoisting also executes the fence on the path where the comparison fails:
```
before after
LL fence
| |
compare LL
/ \ |
fail success compare
| | / \
return fence fail success
| | |
SC return SC
```
That is permitted, the fence only orders a store that then does not
happen. It costs the fence on that path and nothing else. GCC already
places the fence there for a weak release CAS (`lwsync` or `sync`, then
`lwarx`).
Pre-v7 ARM may need the same hook (its CP15 barrier can clear the
exclusive monitor), which is left for a follow-up.
## Testing
- `Transforms/AtomicExpand/PowerPC/weak-cmpxchg-fence-placement.ll`
checks the placement for ppc, e500, the ppc64 default, pwr6,
`-mattr=+fence-keeps-reservation`, pwr7, the AIX default and ppc64le.
`CodeGen/PowerPC/atomics.ll` covers both placements in the llc output.
`flang/test/Lower/target-features-ppc.f90` lists every pwr10 feature, so
it gains `+fence-keeps-reservation`.
- lit on `CodeGen` for PowerPC, ARM, AArch64, RISCV, Mips and SPARC and
on `Transforms/AtomicExpand`: 11803 tests, no failures.
AI tool use: the patch, its tests and this description were written with
Claude Opus 5.5 (Anthropic) from my investigation and under my review. I
have read the LLVM AI Tool Use Policy.
Co-authored-by: Claude Opus 5.5 (1M context) <noreply at anthropic.com>
Added:
llvm/test/Transforms/AtomicExpand/PowerPC/weak-cmpxchg-fence-placement.ll
Modified:
flang/test/Lower/target-features-ppc.f90
llvm/include/llvm/CodeGen/TargetLowering.h
llvm/lib/CodeGen/AtomicExpandPass.cpp
llvm/lib/Target/PowerPC/PPCFeatures.td
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
llvm/lib/Target/PowerPC/PPCISelLowering.h
llvm/test/CodeGen/PowerPC/atomics.ll
Removed:
################################################################################
diff --git a/flang/test/Lower/target-features-ppc.f90 b/flang/test/Lower/target-features-ppc.f90
index 1c3429e6936a02..9826c96df4cf81 100644
--- a/flang/test/Lower/target-features-ppc.f90
+++ b/flang/test/Lower/target-features-ppc.f90
@@ -7,9 +7,9 @@
! ALL: fir.target_cpu = "pwr10"
! FEATURE: fir.target_features = #llvm.target_features<[
-! FEATURE: "+64bit-support", "+allow-unaligned-fp-access", "+altivec", "+bpermd", "+cmpb", "+crbits", "+crypto", "+direct-move", "+extdiv", "+fast-MFLR", "+fcpsgn", "+fpcvt", "+fprnd", "+fpu", "+fre", "+fres", "+frsqrte", "+frsqrtes", "+fsqrt", "+fuse-add-logical", "+fuse-arith-add", "+fuse-logical", "+fuse-logical-add", "+fuse-sha3", "+fuse-store", "+fusion", "+hard-float", "+icbt", "+isa-v206-instructions", "+isa-v207-instructions", "+isa-v30-instructions", "+isa-v31-instructions", "+isel", "+ldbrx", "+lfiwax", "+mfocrf", "+mma", "+paired-vector-memops", "+partword-atomics", "+pcrelative-memops", "+popcntd", "+power10-vector", "+power8-altivec", "+power8-vector", "+power9-altivec", "+power9-vector", "+ppc-postra-sched", "+ppc-prera-sched", "+predictable-select-expensive", "+prefix-instrs", "+quadword-atomics", "+recipprec", "+stfiwx", "+two-const-nr", "+vsx"
+! FEATURE: "+64bit-support", "+allow-unaligned-fp-access", "+altivec", "+bpermd", "+cmpb", "+crbits", "+crypto", "+direct-move", "+extdiv", "+fast-MFLR", "+fcpsgn", "+fence-keeps-reservation", "+fpcvt", "+fprnd", "+fpu", "+fre", "+fres", "+frsqrte", "+frsqrtes", "+fsqrt", "+fuse-add-logical", "+fuse-arith-add", "+fuse-logical", "+fuse-logical-add", "+fuse-sha3", "+fuse-store", "+fusion", "+hard-float", "+icbt", "+isa-v206-instructions", "+isa-v207-instructions", "+isa-v30-instructions", "+isa-v31-instructions", "+isel", "+ldbrx", "+lfiwax", "+mfocrf", "+mma", "+paired-vector-memops", "+partword-atomics", "+pcrelative-memops", "+popcntd", "+power10-vector", "+power8-altivec", "+power8-vector", "+power9-altivec", "+power9-vector", "+ppc-postra-sched", "+ppc-prera-sched", "+predictable-select-expensive", "+prefix-instrs", "+quadword-atomics", "+recipprec", "+stfiwx", "+two-const-nr", "+vsx"
! FEATURE: ]>
! BOTH: fir.target_features = #llvm.target_features<[
-! BOTH: "+64bit-support", "+allow-unaligned-fp-access", "+altivec", "+bpermd", "+cmpb", "+crbits", "+crypto", "+direct-move", "+extdiv", "+fast-MFLR", "+fcpsgn", "+fpcvt", "+fprnd", "+fpu", "+fre", "+fres", "+frsqrte", "+frsqrtes", "+fsqrt", "+fuse-add-logical", "+fuse-arith-add", "+fuse-logical", "+fuse-logical-add", "+fuse-sha3", "+fuse-store", "+fusion", "+hard-float", "+icbt", "+isa-v206-instructions", "+isa-v207-instructions", "+isa-v30-instructions", "+isa-v31-instructions", "+isel", "+ldbrx", "+lfiwax", "+mfocrf", "+mma", "+paired-vector-memops", "+partword-atomics", "+pcrelative-memops", "+popcntd", "+power10-vector", "+power8-altivec", "+power8-vector", "+power9-altivec", "+power9-vector", "+ppc-postra-sched", "+ppc-prera-sched", "+predictable-select-expensive", "+prefix-instrs", "+privileged", "+quadword-atomics", "+recipprec", "+stfiwx", "+two-const-nr", "+vsx"
+! BOTH: "+64bit-support", "+allow-unaligned-fp-access", "+altivec", "+bpermd", "+cmpb", "+crbits", "+crypto", "+direct-move", "+extdiv", "+fast-MFLR", "+fcpsgn", "+fence-keeps-reservation", "+fpcvt", "+fprnd", "+fpu", "+fre", "+fres", "+frsqrte", "+frsqrtes", "+fsqrt", "+fuse-add-logical", "+fuse-arith-add", "+fuse-logical", "+fuse-logical-add", "+fuse-sha3", "+fuse-store", "+fusion", "+hard-float", "+icbt", "+isa-v206-instructions", "+isa-v207-instructions", "+isa-v30-instructions", "+isa-v31-instructions", "+isel", "+ldbrx", "+lfiwax", "+mfocrf", "+mma", "+paired-vector-memops", "+partword-atomics", "+pcrelative-memops", "+popcntd", "+power10-vector", "+power8-altivec", "+power8-vector", "+power9-altivec", "+power9-vector", "+ppc-postra-sched", "+ppc-prera-sched", "+predictable-select-expensive", "+prefix-instrs", "+privileged", "+quadword-atomics", "+recipprec", "+stfiwx", "+two-const-nr", "+vsx"
! BOTH: ]>
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 52655afad28215..2f7384b9569a0a 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -2345,6 +2345,11 @@ class LLVM_ABI TargetLoweringBase {
return false;
}
+ /// Whether a fence between the load-linked and the store-conditional can
+ /// clear the reservation. If true, AtomicExpandPass places the release fence
+ /// of a weak cmpxchg before the load-linked. Defaults to false.
+ virtual bool fenceClearsLoadLinkedReservation() const { return false; }
+
/// Whether AtomicExpandPass should automatically insert a seq_cst trailing
/// fence without reducing the ordering for this atomic store. Defaults to
/// false.
diff --git a/llvm/lib/CodeGen/AtomicExpandPass.cpp b/llvm/lib/CodeGen/AtomicExpandPass.cpp
index 2df4e176750b8e..b978d49e0575db 100644
--- a/llvm/lib/CodeGen/AtomicExpandPass.cpp
+++ b/llvm/lib/CodeGen/AtomicExpandPass.cpp
@@ -1678,7 +1678,11 @@ bool AtomicExpandImpl::expandAtomicCmpXchg(AtomicCmpXchgInst *CI) {
// There's no overhead for sinking the release barrier in a weak cmpxchg, so
// do it even on minsize.
- bool UseUnconditionalReleaseBarrier = F->hasMinSize() && !CI->isWeak();
+ // A fence between the LL and SC may clear the reservation on some targets.
+ // Strong cmpxchg retries, but weak cmpxchg cannot recover.
+ bool UseUnconditionalReleaseBarrier =
+ (F->hasMinSize() && !CI->isWeak()) ||
+ (CI->isWeak() && TLI->fenceClearsLoadLinkedReservation());
// Given: cmpxchg some_op iN* %addr, iN %desired, iN %new success_ord fail_ord
//
diff --git a/llvm/lib/Target/PowerPC/PPCFeatures.td b/llvm/lib/Target/PowerPC/PPCFeatures.td
index f76ed3968c6c20..0b650d25aa4d9a 100644
--- a/llvm/lib/Target/PowerPC/PPCFeatures.td
+++ b/llvm/lib/Target/PowerPC/PPCFeatures.td
@@ -169,6 +169,11 @@ def FeaturePartwordAtomic : SubtargetFeature<"partword-atomics",
def FeatureQuadwordAtomic : SubtargetFeature<"quadword-atomics",
"HasQuadwordAtomics", "true",
"Enable lqarx and stqcx.">;
+// Set on CPUs that keep the larx reservation across a fence before the stcx.
+def FeatureFenceKeepsReservation :
+ SubtargetFeature<"fence-keeps-reservation", "FenceKeepsReservation", "true",
+ "A fence between larx and stcx. keeps the reservation", [],
+ InlineIgnore>;
def FeatureInvariantFunctionDescriptors :
SubtargetFeature<"invariant-function-descriptors",
"HasInvariantFunctionDescriptors", "true",
@@ -475,7 +480,8 @@ def ProcessorFeatures {
DeprecatedDST,
FeatureTwoConstNR,
FeatureUnalignedFloats,
- FeatureISA2_06];
+ FeatureISA2_06,
+ FeatureFenceKeepsReservation];
list<SubtargetFeature> P7SpecificFeatures = [];
list<SubtargetFeature> P7Features =
!listconcat(P7InheritableFeatures, P7SpecificFeatures);
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index 59bdc0e5972ac9..8af5b799483a66 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -13326,6 +13326,10 @@ Instruction *PPCTargetLowering::emitTrailingFence(IRBuilderBase &Builder,
return nullptr;
}
+bool PPCTargetLowering::fenceClearsLoadLinkedReservation() const {
+ return !Subtarget.fenceKeepsReservation();
+}
+
MachineBasicBlock *PPCTargetLowering::EmitAtomicBinary(MachineInstr &MI,
MachineBasicBlock *BB,
unsigned BinOpcode,
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.h b/llvm/lib/Target/PowerPC/PPCISelLowering.h
index a5aa3ca88de135..917d4fee8e5c12 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.h
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.h
@@ -340,6 +340,9 @@ namespace llvm {
return true;
}
+ /// True unless the subtarget has FeatureFenceKeepsReservation.
+ bool fenceClearsLoadLinkedReservation() const override;
+
Value *emitLoadLinked(IRBuilderBase &Builder, Type *ValueTy, Value *Addr,
AtomicOrdering Ord) const override;
diff --git a/llvm/test/CodeGen/PowerPC/atomics.ll b/llvm/test/CodeGen/PowerPC/atomics.ll
index af2b8646d18ae0..b718b259ead32c 100644
--- a/llvm/test/CodeGen/PowerPC/atomics.ll
+++ b/llvm/test/CodeGen/PowerPC/atomics.ll
@@ -1,7 +1,8 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-unknown-linux-gnu -verify-machineinstrs -ppc-asm-full-reg-names | FileCheck %s --check-prefix=CHECK --check-prefix=PPC32
; This is already checked for in Atomics-64.ll
-; RUN: llc -verify-machineinstrs < %s -mcpu=ppc -mtriple=powerpc64-unknown-linux-gnu -ppc-asm-full-reg-names | FileCheck %s --check-prefix=CHECK --check-prefix=PPC64
+; RUN: llc -verify-machineinstrs < %s -mcpu=ppc -mtriple=powerpc64-unknown-linux-gnu -ppc-asm-full-reg-names | FileCheck %s --check-prefix=CHECK --check-prefix=PPC64 --check-prefix=PPC64-HOIST
+; RUN: llc -verify-machineinstrs < %s -mcpu=ppc -mattr=+fence-keeps-reservation -mtriple=powerpc64-unknown-linux-gnu -ppc-asm-full-reg-names | FileCheck %s --check-prefix=CHECK --check-prefix=PPC64 --check-prefix=PPC64-SINK
; FIXME: we don't currently check for the operations themselves with CHECK-NEXT,
; because they are implemented in a very messy way with lwarx/stwcx.
@@ -308,17 +309,29 @@ define i64 @cas_weak_i64_release_monotonic(ptr %mem) {
; PPC32-NEXT: mtlr r0
; PPC32-NEXT: blr
;
-; PPC64-LABEL: cas_weak_i64_release_monotonic:
-; PPC64: # %bb.0: # %cmpxchg.start
-; PPC64-NEXT: mr r4, r3
-; PPC64-NEXT: ldarx r3, 0, r3
-; PPC64-NEXT: cmpldi r3, 0
-; PPC64-NEXT: bnelr- cr0
-; PPC64-NEXT: # %bb.1: # %cmpxchg.fencedstore
-; PPC64-NEXT: lwsync
-; PPC64-NEXT: li r5, 1
-; PPC64-NEXT: stdcx. r5, 0, r4
-; PPC64-NEXT: blr
+; PPC64-HOIST-LABEL: cas_weak_i64_release_monotonic:
+; PPC64-HOIST: # %bb.0: # %cmpxchg.start
+; PPC64-HOIST-NEXT: lwsync
+; PPC64-HOIST-NEXT: mr r4, r3
+; PPC64-HOIST-NEXT: ldarx r3, 0, r3
+; PPC64-HOIST-NEXT: cmpldi r3, 0
+; PPC64-HOIST-NEXT: bnelr- cr0
+; PPC64-HOIST-NEXT: # %bb.1: # %cmpxchg.fencedstore
+; PPC64-HOIST-NEXT: li r5, 1
+; PPC64-HOIST-NEXT: stdcx. r5, 0, r4
+; PPC64-HOIST-NEXT: blr
+;
+; PPC64-SINK-LABEL: cas_weak_i64_release_monotonic:
+; PPC64-SINK: # %bb.0: # %cmpxchg.start
+; PPC64-SINK-NEXT: mr r4, r3
+; PPC64-SINK-NEXT: ldarx r3, 0, r3
+; PPC64-SINK-NEXT: cmpldi r3, 0
+; PPC64-SINK-NEXT: bnelr- cr0
+; PPC64-SINK-NEXT: # %bb.1: # %cmpxchg.fencedstore
+; PPC64-SINK-NEXT: lwsync
+; PPC64-SINK-NEXT: li r5, 1
+; PPC64-SINK-NEXT: stdcx. r5, 0, r4
+; PPC64-SINK-NEXT: blr
%val = cmpxchg weak ptr %mem, i64 0, i64 1 release monotonic
%loaded = extractvalue { i64, i1} %val, 0
ret i64 %loaded
diff --git a/llvm/test/Transforms/AtomicExpand/PowerPC/weak-cmpxchg-fence-placement.ll b/llvm/test/Transforms/AtomicExpand/PowerPC/weak-cmpxchg-fence-placement.ll
new file mode 100644
index 00000000000000..a1197c746674e7
--- /dev/null
+++ b/llvm/test/Transforms/AtomicExpand/PowerPC/weak-cmpxchg-fence-placement.ll
@@ -0,0 +1,73 @@
+; NOTE: Do not autogenerate. This test focuses on fence placement across multiple
+; target configurations; autogenerated checks duplicate target-specific IR
+; unrelated to the behavior under test.
+
+; RUN: opt -S -mtriple=powerpc-unknown-linux-musl \
+; RUN: -passes='require<libcall-lowering-info>,atomic-expand' %s \
+; RUN: | FileCheck %s --check-prefixes=CHECK,HOIST
+; RUN: opt -S -mtriple=powerpc-unknown-linux-musl -mcpu=e500 \
+; RUN: -passes='require<libcall-lowering-info>,atomic-expand' %s \
+; RUN: | FileCheck %s --check-prefixes=CHECK,HOIST
+; RUN: opt -S -mtriple=powerpc64-unknown-linux-gnu \
+; RUN: -passes='require<libcall-lowering-info>,atomic-expand' %s \
+; RUN: | FileCheck %s --check-prefixes=CHECK,HOIST
+; RUN: opt -S -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr6 \
+; RUN: -passes='require<libcall-lowering-info>,atomic-expand' %s \
+; RUN: | FileCheck %s --check-prefixes=CHECK,HOIST
+; RUN: opt -S -mtriple=powerpc-unknown-linux-musl \
+; RUN: -mattr=+fence-keeps-reservation \
+; RUN: -passes='require<libcall-lowering-info>,atomic-expand' %s \
+; RUN: | FileCheck %s --check-prefixes=CHECK,SINK
+; RUN: opt -S -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr7 \
+; RUN: -passes='require<libcall-lowering-info>,atomic-expand' %s \
+; RUN: | FileCheck %s --check-prefixes=CHECK,SINK
+; RUN: opt -S -mtriple=powerpc64-ibm-aix \
+; RUN: -passes='require<libcall-lowering-info>,atomic-expand' %s \
+; RUN: | FileCheck %s --check-prefixes=CHECK,SINK
+; RUN: opt -S -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN: -passes='require<libcall-lowering-info>,atomic-expand' %s \
+; RUN: | FileCheck %s --check-prefixes=CHECK,SINK
+
+; A weak cmpxchg gets its release fence before the lwarx (HOIST) unless the CPU
+; has fence-keeps-reservation (SINK). A strong cmpxchg sinks it unless the
+; function is minsize.
+
+define i1 @weak_release(ptr %p) {
+; CHECK-LABEL: define i1 @weak_release(
+; HOIST-NEXT: call void @llvm.ppc.lwsync()
+; SINK-NOT: @llvm.ppc.{{(lw)?}}sync
+; CHECK: call i32 @llvm.ppc.lwarx(ptr %p)
+; HOIST-NOT: @llvm.ppc.{{(lw)?}}sync
+; SINK: cmpxchg.fencedstore:
+; SINK-NEXT: call void @llvm.ppc.lwsync()
+; CHECK: call i32 @llvm.ppc.stwcx(ptr %p, i32 1)
+ %pair = cmpxchg weak ptr %p, i32 0, i32 1 release monotonic
+ %ok = extractvalue { i32, i1 } %pair, 1
+ ret i1 %ok
+}
+
+define i1 @weak_seq_cst(ptr %p) {
+; CHECK-LABEL: define i1 @weak_seq_cst(
+; HOIST-NEXT: call void @llvm.ppc.sync()
+; SINK-NOT: @llvm.ppc.{{(lw)?}}sync
+; CHECK: call i32 @llvm.ppc.lwarx(ptr %p)
+; HOIST-NOT: @llvm.ppc.{{(lw)?}}sync
+; SINK: cmpxchg.fencedstore:
+; SINK-NEXT: call void @llvm.ppc.sync()
+; CHECK: call i32 @llvm.ppc.stwcx(ptr %p, i32 1)
+ %pair = cmpxchg weak ptr %p, i32 0, i32 1 seq_cst seq_cst
+ %ok = extractvalue { i32, i1 } %pair, 1
+ ret i1 %ok
+}
+
+define i1 @strong_release(ptr %p) {
+; CHECK-LABEL: define i1 @strong_release(
+; CHECK-NOT: @llvm.ppc.{{(lw)?}}sync
+; CHECK: call i32 @llvm.ppc.lwarx(ptr %p)
+; CHECK: cmpxchg.fencedstore:
+; CHECK-NEXT: call void @llvm.ppc.lwsync()
+; CHECK: call i32 @llvm.ppc.stwcx(ptr %p, i32 1)
+ %pair = cmpxchg ptr %p, i32 0, i32 1 release monotonic
+ %ok = extractvalue { i32, i1 } %pair, 1
+ ret i1 %ok
+}
More information about the flang-commits
mailing list