[flang-commits] [flang] [llvm] [AtomicExpand] Let targets keep the release fence out of the reservation (PR #214867)
Josef Schlehofer via flang-commits
flang-commits at lists.llvm.org
Sat Oct 3 11:39:02 PDT 2026
https://github.com/BKPepe updated https://github.com/llvm/llvm-project/pull/214867
>From 1d7029ff5e2ab77a932ce130632386bf27314810 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe.schlehofer at gmail.com>
Date: Sat, 3 Oct 2026 20:07:26 +0200
Subject: [PATCH] [AtomicExpand] Let targets keep the release fence out of the
reservation
expandAtomicCmpXchg sinks the leading fence into the conditional
cmpxchg.fencedstore block, between the load-linked and the
store-conditional. For a strong cmpxchg that is harmless, because a
failed store-conditional retries and re-reserves after the fence. A weak
cmpxchg has no retry, so where a fence clears the reservation it can
never succeed.
Add fenceClearsLoadLinkedReservation(), defaulting to false. Targets
that return true get the fence of a weak cmpxchg before the load-linked.
All other targets keep the current placement. Hoisting also executes the
fence on the path where the comparison fails. That is permitted, the
fence only orders a store that then does not happen.
On PowerPC, whether a fence clears the reservation is implementation
specific. e500v2 does, which is where this was found: there every
compare_exchange_weak with release, acq_rel or seq_cst ordering fails,
so a retry loop around it never terminates. Add the subtarget feature
fence-keeps-reservation, 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.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply at anthropic.com>
---
flang/test/Lower/target-features-ppc.f90 | 4 +-
llvm/include/llvm/CodeGen/TargetLowering.h | 5 ++
llvm/lib/CodeGen/AtomicExpandPass.cpp | 6 +-
llvm/lib/Target/PowerPC/PPC.td | 8 ++-
llvm/lib/Target/PowerPC/PPCISelLowering.cpp | 4 ++
llvm/lib/Target/PowerPC/PPCISelLowering.h | 3 +
llvm/test/CodeGen/PowerPC/atomics.ll | 37 ++++++----
.../PowerPC/weak-cmpxchg-fence-placement.ll | 69 +++++++++++++++++++
8 files changed, 120 insertions(+), 16 deletions(-)
create mode 100644 llvm/test/Transforms/AtomicExpand/PowerPC/weak-cmpxchg-fence-placement.ll
diff --git a/flang/test/Lower/target-features-ppc.f90 b/flang/test/Lower/target-features-ppc.f90
index 1c3429e6936a021..9826c96df4cf812 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 69f5e0e4e301112..1c427bc894f3d1b 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -2313,6 +2313,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 f2ffa40030cc0c5..c9ceaa8ee6082d5 100644
--- a/llvm/lib/CodeGen/AtomicExpandPass.cpp
+++ b/llvm/lib/CodeGen/AtomicExpandPass.cpp
@@ -1498,7 +1498,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/PPC.td b/llvm/lib/Target/PowerPC/PPC.td
index ba2a4e6a9695ce7..819e1493f47afd1 100644
--- a/llvm/lib/Target/PowerPC/PPC.td
+++ b/llvm/lib/Target/PowerPC/PPC.td
@@ -177,6 +177,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",
@@ -489,7 +494,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 b012e6c6093ae50..bac60029fbcd5a2 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -13320,6 +13320,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 fc0e776521e0847..1874a4660b02d84 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.h
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.h
@@ -338,6 +338,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 af2b8646d18ae03..b718b259ead32c9 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 000000000000000..520a23141de939a
--- /dev/null
+++ b/llvm/test/Transforms/AtomicExpand/PowerPC/weak-cmpxchg-fence-placement.ll
@@ -0,0 +1,69 @@
+; 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