[llvm] [InstCombine] Preserve elementwise atomic access sizes (PR #223897)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 15 19:06:56 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-analysis
Author: Yonah Goldberg (YonahGoldberg)
<details>
<summary>Changes</summary>
There are two cases where instcombine currently will change the atomic access size of an unordered elementwise atomic. Note that, for example, `load atomic elementwise <4 x i32>` and `load atomic elementwise <2 x i64>` do not have equivalent semantics. The former has 4 atomic accesses and the latter has 2. Rewriting the former to the latter can introduce mixed-size, concurrent, atomic accesses, which race in LLVM. Without `elementwise`, rewriting _is_ legal.
The two cases are:
1. `mergeStoreIntoSuccessor`:
```
/// Try to transform:
/// if () { *P = v1; } else { *P = v2 }
/// or:
/// *P = v1; if () { *P = v2; }
/// into a phi node with a store in the successor.
```
You can't do this if the stores have different atomic access sizes.
2. Load CSE and load-to-store forwarding. These are also illegal if the memory operations in question have different atomic access sizes.
Fixes https://github.com/llvm/llvm-project/issues/223738.
AI assisted.
---
Full diff: https://github.com/llvm/llvm-project/pull/223897.diff
5 Files Affected:
- (modified) llvm/include/llvm/Analysis/Loads.h (+4-2)
- (modified) llvm/lib/Analysis/Loads.cpp (+34-10)
- (modified) llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (+10-2)
- (modified) llvm/lib/Transforms/Scalar/JumpThreading.cpp (+5-5)
- (modified) llvm/test/Transforms/InstCombine/atomic.ll (+119)
``````````diff
diff --git a/llvm/include/llvm/Analysis/Loads.h b/llvm/include/llvm/Analysis/Loads.h
index 22ce31d61ddb8..4213f6c073405 100644
--- a/llvm/include/llvm/Analysis/Loads.h
+++ b/llvm/include/llvm/Analysis/Loads.h
@@ -177,6 +177,7 @@ FindAvailableLoadedValue(LoadInst *Load, BatchAAResults &AA, bool *IsLoadCSE,
/// \param AtLeastAtomic Are we looking for at-least an atomic load/store ? In
/// case it is false, we can return an atomic or non-atomic load or store. In
/// case it is true, we need to return an atomic load or store.
+/// \param IsElementwise Whether the requested atomic access is elementwise.
/// \param ScanBB The basic block to scan.
/// \param [in,out] ScanFrom The location to start scanning from. When this
/// function returns, it points at the last instruction scanned.
@@ -190,8 +191,9 @@ FindAvailableLoadedValue(LoadInst *Load, BatchAAResults &AA, bool *IsLoadCSE,
/// \returns The found value, or nullptr if no value is found.
LLVM_ABI Value *findAvailablePtrLoadStore(
const MemoryLocation &Loc, Type *AccessTy, bool AtLeastAtomic,
- BasicBlock *ScanBB, BasicBlock::iterator &ScanFrom, unsigned MaxInstsToScan,
- BatchAAResults *AA, bool *IsLoadCSE, unsigned *NumScanedInst);
+ bool IsElementwise, BasicBlock *ScanBB, BasicBlock::iterator &ScanFrom,
+ unsigned MaxInstsToScan, BatchAAResults *AA, bool *IsLoadCSE,
+ unsigned *NumScanedInst);
/// Returns true if a pointer value \p From can be replaced with another pointer
/// value \To if they are deemed equal through some means (e.g. information from
diff --git a/llvm/lib/Analysis/Loads.cpp b/llvm/lib/Analysis/Loads.cpp
index de9022c540d42..65e81c1624773 100644
--- a/llvm/lib/Analysis/Loads.cpp
+++ b/llvm/lib/Analysis/Loads.cpp
@@ -559,9 +559,9 @@ Value *llvm::FindAvailableLoadedValue(LoadInst *Load, BasicBlock *ScanBB,
return nullptr;
MemoryLocation Loc = MemoryLocation::get(Load);
- return findAvailablePtrLoadStore(Loc, Load->getType(), Load->isAtomic(),
- ScanBB, ScanFrom, MaxInstsToScan, AA, IsLoad,
- NumScanedInst);
+ return findAvailablePtrLoadStore(
+ Loc, Load->getType(), Load->isAtomic(), Load->isElementwise(), ScanBB,
+ ScanFrom, MaxInstsToScan, AA, IsLoad, NumScanedInst);
}
// Check if the load and the store have the same base, constant offsets and
@@ -592,7 +592,24 @@ static bool areNonOverlapSameBaseLoadAndStore(const Value *LoadPtr,
static Value *getAvailableLoadStore(Instruction *Inst, const Value *Ptr,
Type *AccessTy, bool AtLeastAtomic,
- const DataLayout &DL, bool *IsLoadCSE) {
+ bool IsElementwise, const DataLayout &DL,
+ bool *IsLoadCSE) {
+ // For elementwise atomics, each vector element is a separate atomic access.
+ // Reusing an operation with a different access size would change atomicity.
+ auto hasCompatibleAtomicAccessSize = [&](Type *OtherAccessTy,
+ bool OtherIsElementwise) {
+ if (!AtLeastAtomic)
+ return true;
+
+ Type *AtomicAccessTy =
+ IsElementwise ? AccessTy->getScalarType() : AccessTy;
+ Type *OtherAtomicAccessTy = OtherIsElementwise
+ ? OtherAccessTy->getScalarType()
+ : OtherAccessTy;
+ return DL.getTypeStoreSize(AtomicAccessTy) ==
+ DL.getTypeStoreSize(OtherAtomicAccessTy);
+ };
+
// If this is a load of Ptr, the loaded value is available.
// (This is true even if the load is volatile or atomic, although
// those cases are unlikely.)
@@ -606,7 +623,8 @@ static Value *getAvailableLoadStore(Instruction *Inst, const Value *Ptr,
if (!AreEquivalentAddressValues(LoadPtr, Ptr))
return nullptr;
- if (CastInst::isBitOrNoopPointerCastable(LI->getType(), AccessTy, DL)) {
+ if (hasCompatibleAtomicAccessSize(LI->getType(), LI->isElementwise()) &&
+ CastInst::isBitOrNoopPointerCastable(LI->getType(), AccessTy, DL)) {
if (IsLoadCSE)
*IsLoadCSE = true;
return LI;
@@ -630,6 +648,9 @@ static Value *getAvailableLoadStore(Instruction *Inst, const Value *Ptr,
*IsLoadCSE = false;
Value *Val = SI->getValueOperand();
+ if (!hasCompatibleAtomicAccessSize(Val->getType(), SI->isElementwise()))
+ return nullptr;
+
if (CastInst::isBitOrNoopPointerCastable(Val->getType(), AccessTy, DL))
return Val;
@@ -686,8 +707,9 @@ static Value *getAvailableLoadStore(Instruction *Inst, const Value *Ptr,
Value *llvm::findAvailablePtrLoadStore(
const MemoryLocation &Loc, Type *AccessTy, bool AtLeastAtomic,
- BasicBlock *ScanBB, BasicBlock::iterator &ScanFrom, unsigned MaxInstsToScan,
- BatchAAResults *AA, bool *IsLoadCSE, unsigned *NumScanedInst) {
+ bool IsElementwise, BasicBlock *ScanBB, BasicBlock::iterator &ScanFrom,
+ unsigned MaxInstsToScan, BatchAAResults *AA, bool *IsLoadCSE,
+ unsigned *NumScanedInst) {
if (MaxInstsToScan == 0)
MaxInstsToScan = ~0U;
@@ -713,8 +735,9 @@ Value *llvm::findAvailablePtrLoadStore(
--ScanFrom;
- if (Value *Available = getAvailableLoadStore(Inst, StrippedPtr, AccessTy,
- AtLeastAtomic, DL, IsLoadCSE))
+ if (Value *Available = getAvailableLoadStore(
+ Inst, StrippedPtr, AccessTy, AtLeastAtomic, IsElementwise, DL,
+ IsLoadCSE))
return Available;
// Try to get the store size for the type.
@@ -793,7 +816,8 @@ Value *llvm::FindAvailableLoadedValue(LoadInst *Load, BatchAAResults &AA,
return nullptr;
Available = getAvailableLoadStore(&Inst, StrippedPtr, AccessTy,
- AtLeastAtomic, DL, IsLoadCSE);
+ AtLeastAtomic, Load->isElementwise(), DL,
+ IsLoadCSE);
if (Available)
break;
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 8e2a0c5376d8e..74322d599bd28 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -1669,8 +1669,16 @@ bool InstCombinerImpl::mergeStoreIntoSuccessor(StoreInst &SI) {
auto *SIVTy = SI.getValueOperand()->getType();
auto *OSVTy = OtherStore->getValueOperand()->getType();
- return CastInst::isBitOrNoopPointerCastable(OSVTy, SIVTy, DL) &&
- SI.hasSameSpecialState(OtherStore);
+ if (!CastInst::isBitOrNoopPointerCastable(OSVTy, SIVTy, DL) ||
+ !SI.hasSameSpecialState(OtherStore))
+ return false;
+
+ // Elementwise atomic stores behave as one atomic store per vector
+ // element. Do not split or merge those atomic accesses by changing the
+ // element size.
+ return !SI.isElementwise() ||
+ DL.getTypeStoreSize(SIVTy->getScalarType()) ==
+ DL.getTypeStoreSize(OSVTy->getScalarType());
};
// If the other block ends in an unconditional branch, check for the 'if then
diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index 66233407a5548..4f1ae940d3b7e 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -1314,8 +1314,8 @@ bool JumpThreadingPass::simplifyPartiallyRedundantLoad(LoadInst *LoadI) {
LocationSize::precise(DL.getTypeStoreSize(AccessTy)),
AATags);
PredAvailable = findAvailablePtrLoadStore(
- Loc, AccessTy, LoadI->isAtomic(), PredBB, BBIt, DefMaxInstsToScan,
- &BatchAA, &IsLoadCSE, &NumScanedInst);
+ Loc, AccessTy, LoadI->isAtomic(), LoadI->isElementwise(), PredBB, BBIt,
+ DefMaxInstsToScan, &BatchAA, &IsLoadCSE, &NumScanedInst);
// If PredBB has a single predecessor, continue scanning through the
// single predecessor.
@@ -1326,9 +1326,9 @@ bool JumpThreadingPass::simplifyPartiallyRedundantLoad(LoadInst *LoadI) {
if (SinglePredBB) {
BBIt = SinglePredBB->end();
PredAvailable = findAvailablePtrLoadStore(
- Loc, AccessTy, LoadI->isAtomic(), SinglePredBB, BBIt,
- (DefMaxInstsToScan - NumScanedInst), &BatchAA, &IsLoadCSE,
- &NumScanedInst);
+ Loc, AccessTy, LoadI->isAtomic(), LoadI->isElementwise(),
+ SinglePredBB, BBIt, (DefMaxInstsToScan - NumScanedInst), &BatchAA,
+ &IsLoadCSE, &NumScanedInst);
}
}
diff --git a/llvm/test/Transforms/InstCombine/atomic.ll b/llvm/test/Transforms/InstCombine/atomic.ll
index 062aa3db34759..660194f6e7e19 100644
--- a/llvm/test/Transforms/InstCombine/atomic.ll
+++ b/llvm/test/Transforms/InstCombine/atomic.ll
@@ -474,4 +474,123 @@ define void @store_elementwise_bitcast(ptr %p, i64 %v) {
ret void
}
+define void @store_merge_elementwise_different_element_size(
+; CHECK-LABEL: @store_merge_elementwise_different_element_size(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[C:%.*]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK: then:
+; CHECK-NEXT: store atomic elementwise <2 x i64> [[B:%.*]], ptr [[P:%.*]] unordered, align 16
+; CHECK-NEXT: br label [[END:%.*]]
+; CHECK: else:
+; CHECK-NEXT: store atomic elementwise <4 x i32> [[A:%.*]], ptr [[P]] unordered, align 16
+; CHECK-NEXT: br label [[END]]
+; CHECK: end:
+; CHECK-NEXT: ret void
+;
+ ptr %p, i1 %c, <4 x i32> %a, <2 x i64> %b) {
+entry:
+ br i1 %c, label %then, label %else
+then:
+ store atomic elementwise <2 x i64> %b, ptr %p unordered, align 16
+ br label %end
+else:
+ store atomic elementwise <4 x i32> %a, ptr %p unordered, align 16
+ br label %end
+end:
+ ret void
+}
+
+define void @store_merge_elementwise_same_element_size(
+; CHECK-LABEL: @store_merge_elementwise_same_element_size(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[C:%.*]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK: then:
+; CHECK-NEXT: [[TMP0:%.*]] = bitcast <4 x float> [[B:%.*]] to <4 x i32>
+; CHECK-NEXT: br label [[END:%.*]]
+; CHECK: else:
+; CHECK-NEXT: br label [[END]]
+; CHECK: end:
+; CHECK-NEXT: [[STOREMERGE:%.*]] = phi <4 x i32> [ [[A:%.*]], [[ELSE]] ], [ [[TMP0]], [[THEN]] ]
+; CHECK-NEXT: store atomic elementwise <4 x i32> [[STOREMERGE]], ptr [[P:%.*]] unordered, align 16
+; CHECK-NEXT: ret void
+;
+ ptr %p, i1 %c, <4 x i32> %a, <4 x float> %b) {
+entry:
+ br i1 %c, label %then, label %else
+then:
+ store atomic elementwise <4 x float> %b, ptr %p unordered, align 16
+ br label %end
+else:
+ store atomic elementwise <4 x i32> %a, ptr %p unordered, align 16
+ br label %end
+end:
+ ret void
+}
+
+declare void @use_v2i64(<2 x i64>) memory(none)
+declare void @use_v4i32(<4 x i32>) memory(none)
+
+define <4 x i32> @load_cse_elementwise_different_element_size(ptr %p) {
+; CHECK-LABEL: @load_cse_elementwise_different_element_size(
+; CHECK-NEXT: [[A:%.*]] = load atomic elementwise <2 x i64>, ptr [[P:%.*]] unordered, align 16
+; CHECK-NEXT: call void @use_v2i64(<2 x i64> [[A]])
+; CHECK-NEXT: [[B_CAST:%.*]] = load atomic elementwise <4 x i32>, ptr [[P]] unordered, align 16
+; CHECK-NEXT: ret <4 x i32> [[B_CAST]]
+;
+ %a = load atomic elementwise <2 x i64>, ptr %p unordered, align 16
+ call void @use_v2i64(<2 x i64> %a)
+ %b = load atomic elementwise <4 x i32>, ptr %p unordered, align 16
+ ret <4 x i32> %b
+}
+
+define <4 x float> @load_cse_elementwise_same_element_size(ptr %p) {
+; CHECK-LABEL: @load_cse_elementwise_same_element_size(
+; CHECK-NEXT: [[A:%.*]] = load atomic elementwise <4 x i32>, ptr [[P:%.*]] unordered, align 16
+; CHECK-NEXT: call void @use_v4i32(<4 x i32> [[A]])
+; CHECK-NEXT: [[B_CAST:%.*]] = bitcast <4 x i32> [[A]] to <4 x float>
+; CHECK-NEXT: ret <4 x float> [[B_CAST]]
+;
+ %a = load atomic elementwise <4 x i32>, ptr %p unordered, align 16
+ call void @use_v4i32(<4 x i32> %a)
+ %b = load atomic elementwise <4 x float>, ptr %p unordered, align 16
+ ret <4 x float> %b
+}
+
+define <4 x i32> @store_to_load_elementwise_different_element_size(
+; CHECK-LABEL: @store_to_load_elementwise_different_element_size(
+; CHECK-NEXT: store atomic elementwise <2 x i64> [[A:%.*]], ptr [[P:%.*]] unordered, align 16
+; CHECK-NEXT: [[B_CAST:%.*]] = load atomic elementwise <4 x i32>, ptr [[P]] unordered, align 16
+; CHECK-NEXT: ret <4 x i32> [[B_CAST]]
+;
+ ptr %p, <2 x i64> %a) {
+ store atomic elementwise <2 x i64> %a, ptr %p unordered, align 16
+ %b = load atomic elementwise <4 x i32>, ptr %p unordered, align 16
+ ret <4 x i32> %b
+}
+
+define <4 x float> @store_to_load_elementwise_same_element_size(
+; CHECK-LABEL: @store_to_load_elementwise_same_element_size(
+; CHECK-NEXT: store atomic elementwise <4 x i32> [[A:%.*]], ptr [[P:%.*]] unordered, align 16
+; CHECK-NEXT: [[B_CAST:%.*]] = bitcast <4 x i32> [[A]] to <4 x float>
+; CHECK-NEXT: ret <4 x float> [[B_CAST]]
+;
+ ptr %p, <4 x i32> %a) {
+ store atomic elementwise <4 x i32> %a, ptr %p unordered, align 16
+ %b = load atomic elementwise <4 x float>, ptr %p unordered, align 16
+ ret <4 x float> %b
+}
+
+define <4 x i32> @load_cse_whole_to_elementwise(ptr %p) {
+; CHECK-LABEL: @load_cse_whole_to_elementwise(
+; CHECK-NEXT: [[A:%.*]] = load atomic <4 x i32>, ptr [[P:%.*]] unordered, align 16
+; CHECK-NEXT: call void @use_v4i32(<4 x i32> [[A]])
+; CHECK-NEXT: [[B:%.*]] = load atomic elementwise <4 x i32>, ptr [[P]] unordered, align 16
+; CHECK-NEXT: ret <4 x i32> [[B]]
+;
+ %a = load atomic <4 x i32>, ptr %p unordered, align 16
+ call void @use_v4i32(<4 x i32> %a)
+ %b = load atomic elementwise <4 x i32>, ptr %p unordered, align 16
+ ret <4 x i32> %b
+}
+
attributes #0 = { null_pointer_is_valid }
``````````
</details>
https://github.com/llvm/llvm-project/pull/223897
More information about the llvm-commits
mailing list