[llvm] [SROA] Clean up rewritePartition type selection process (PR #169106)
Yonah Goldberg via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 21 14:04:54 PST 2025
https://github.com/YonahGoldberg created https://github.com/llvm/llvm-project/pull/169106
This change reverts https://github.com/llvm/llvm-project/commit/257251247a267c3fa30fdeef17ffa4987d8a52e5, which landed on Aug 8, 2022. This change addressed the problem that if you have IR that looks something like:
```
%alloca = alloca <4 x float>
store <4 x float> %data, ptr %alloca
%load = load half, ptr %alloca
```
`getCommonType` would return `<4 x float>` because the `load half` isn't to the entire partition, so we skip the first `getTypePartition` check. https://github.com/llvm/llvm-project/commit/257251247a267c3fa30fdeef17ffa4987d8a52e5 added a later check that sees that `<4 x float>` is not vector promotable because of the `load half`, and then calls `getTypePartition`, which changes the `sliceTy` to `< 8 x half>`, which is vector promotable because the store can be changed to `store <8 x half>`. So we set the `sliceTy` to `<8 x half>`, we can promote the alloca, and everyone is happy.
This code became unnecessary after https://github.com/llvm/llvm-project/commit/529eafd9beff233ba8debfc73e0b5c04cac36835, which landed ~3 months later, which fixes the issue in a different way. `isVectorPromotionViable` was already smart enough to try `<8 x half>` as a type candidate because it sees the `load half`. However, this candidate didn't work because it conflicts with `store <4 x float>`. This commit added logic to try integer-ifying candidates if there is no common type. So the `<8 x half>` candidate gets converted to `<8 x i16>`, which works because we can convert the alloca to `alloca <8 x i16>` and the load to `load i16`, allowing promotion.
After https://github.com/llvm/llvm-project/commit/529eafd9beff233ba8debfc73e0b5c04cac36835, the original commit is pointless. It tries to refine the `SliceTy`, but if `isVectorPromotionViable` succeeds, it returns a new type to use and we will ignore the `SliceTy`.
This change is my first patch to try to simplify the type selection process in rewritePartition. A few other issues I've noticed are. I had some other ideas that I tried in https://github.com/llvm/llvm-project/pull/167771 and https://github.com/llvm/llvm-project/pull/168796, but they need refinement.
>From 2166e8ad5c3c94a57de691c9b62392c81a615df1 Mon Sep 17 00:00:00 2001
From: Yonah Goldberg <ygoldberg at nvidia.com>
Date: Fri, 21 Nov 2025 20:04:30 +0000
Subject: [PATCH 1/3] cleanup
---
llvm/lib/Transforms/Scalar/SROA.cpp | 47 +++++++++++++++--------------
1 file changed, 25 insertions(+), 22 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 5c60fad6f91aa..93d6f32d9a428 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -2290,12 +2290,31 @@ checkVectorTypesForPromotion(Partition &P, const DataLayout &DL,
return cast<FixedVectorType>(VTy)->getNumElements() >
std::numeric_limits<unsigned short>::max();
});
+
+ // Find a vector type viable for promotion by iterating over all slices.
+ auto *VTy = llvm::find_if(CandidateTys, [&](VectorType *VTy) -> bool {
+ uint64_t ElementSize =
+ DL.getTypeSizeInBits(VTy->getElementType()).getFixedValue();
+
+ // While the definition of LLVM vectors is bitpacked, we don't support sizes
+ // that aren't byte sized.
+ if (ElementSize % 8)
+ return false;
+ assert((DL.getTypeSizeInBits(VTy).getFixedValue() % 8) == 0 &&
+ "vector size not a multiple of element size?");
+ ElementSize /= 8;
- for (VectorType *VTy : CandidateTys)
- if (checkVectorTypeForPromotion(P, VTy, DL, VScale))
- return VTy;
+ for (const Slice &S : P)
+ if (!isVectorPromotionViableForSlice(P, S, VTy, ElementSize, DL, VScale))
+ return false;
- return nullptr;
+ for (const Slice *S : P.splitSliceTails())
+ if (!isVectorPromotionViableForSlice(P, *S, VTy, ElementSize, DL, VScale))
+ return false;
+
+ return true;
+ });
+ return VTy != CandidateTys.end() ? *VTy : nullptr;
}
static VectorType *createAndCheckVectorTypesForPromotion(
@@ -5213,7 +5232,6 @@ AllocaInst *SROA::rewritePartition(AllocaInst &AI, AllocaSlices &AS,
// won't always succeed, in which case we fall back to a legal integer type
// or an i8 array of an appropriate size.
Type *SliceTy = nullptr;
- VectorType *SliceVecTy = nullptr;
const DataLayout &DL = AI.getDataLayout();
unsigned VScale = AI.getFunction()->getVScaleValue();
@@ -5222,10 +5240,8 @@ AllocaInst *SROA::rewritePartition(AllocaInst &AI, AllocaSlices &AS,
// Do all uses operate on the same type?
if (CommonUseTy.first) {
TypeSize CommonUseSize = DL.getTypeAllocSize(CommonUseTy.first);
- if (CommonUseSize.isFixed() && CommonUseSize.getFixedValue() >= P.size()) {
+ if (CommonUseSize.isFixed() && CommonUseSize.getFixedValue() >= P.size())
SliceTy = CommonUseTy.first;
- SliceVecTy = dyn_cast<VectorType>(SliceTy);
- }
}
// If not, can we find an appropriate subtype in the original allocated type?
if (!SliceTy)
@@ -5235,27 +5251,14 @@ AllocaInst *SROA::rewritePartition(AllocaInst &AI, AllocaSlices &AS,
// If still not, can we use the largest bitwidth integer type used?
if (!SliceTy && CommonUseTy.second)
- if (DL.getTypeAllocSize(CommonUseTy.second).getFixedValue() >= P.size()) {
+ if (DL.getTypeAllocSize(CommonUseTy.second).getFixedValue() >= P.size())
SliceTy = CommonUseTy.second;
- SliceVecTy = dyn_cast<VectorType>(SliceTy);
- }
if ((!SliceTy || (SliceTy->isArrayTy() &&
SliceTy->getArrayElementType()->isIntegerTy())) &&
DL.isLegalInteger(P.size() * 8)) {
SliceTy = Type::getIntNTy(*C, P.size() * 8);
}
- // If the common use types are not viable for promotion then attempt to find
- // another type that is viable.
- if (SliceVecTy && !checkVectorTypeForPromotion(P, SliceVecTy, DL, VScale))
- if (Type *TypePartitionTy = getTypePartition(DL, AI.getAllocatedType(),
- P.beginOffset(), P.size())) {
- VectorType *TypePartitionVecTy = dyn_cast<VectorType>(TypePartitionTy);
- if (TypePartitionVecTy &&
- checkVectorTypeForPromotion(P, TypePartitionVecTy, DL, VScale))
- SliceTy = TypePartitionTy;
- }
-
if (!SliceTy)
SliceTy = ArrayType::get(Type::getInt8Ty(*C), P.size());
assert(DL.getTypeAllocSize(SliceTy).getFixedValue() >= P.size());
>From bf6c5af5a3bd2cdbe25407eb2d70e00ae87a4b36 Mon Sep 17 00:00:00 2001
From: Yonah Goldberg <ygoldberg at nvidia.com>
Date: Fri, 21 Nov 2025 20:04:38 +0000
Subject: [PATCH 2/3] clang format
---
llvm/lib/Transforms/Scalar/SROA.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 93d6f32d9a428..02caa9757535e 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -2290,7 +2290,7 @@ checkVectorTypesForPromotion(Partition &P, const DataLayout &DL,
return cast<FixedVectorType>(VTy)->getNumElements() >
std::numeric_limits<unsigned short>::max();
});
-
+
// Find a vector type viable for promotion by iterating over all slices.
auto *VTy = llvm::find_if(CandidateTys, [&](VectorType *VTy) -> bool {
uint64_t ElementSize =
>From 2f590c7b1da3363cca7acaaa683bd960b5e6d7ee Mon Sep 17 00:00:00 2001
From: Yonah Goldberg <ygoldberg at nvidia.com>
Date: Fri, 21 Nov 2025 20:18:14 +0000
Subject: [PATCH 3/3] delete
---
llvm/lib/Transforms/Scalar/SROA.cpp | 29 -----------------------------
1 file changed, 29 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 02caa9757535e..31ecb2391412a 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -2178,35 +2178,6 @@ static bool isVectorPromotionViableForSlice(Partition &P, const Slice &S,
return true;
}
-/// Test whether a vector type is viable for promotion.
-///
-/// This implements the necessary checking for \c checkVectorTypesForPromotion
-/// (and thus isVectorPromotionViable) over all slices of the alloca for the
-/// given VectorType.
-static bool checkVectorTypeForPromotion(Partition &P, VectorType *VTy,
- const DataLayout &DL, unsigned VScale) {
- uint64_t ElementSize =
- DL.getTypeSizeInBits(VTy->getElementType()).getFixedValue();
-
- // While the definition of LLVM vectors is bitpacked, we don't support sizes
- // that aren't byte sized.
- if (ElementSize % 8)
- return false;
- assert((DL.getTypeSizeInBits(VTy).getFixedValue() % 8) == 0 &&
- "vector size not a multiple of element size?");
- ElementSize /= 8;
-
- for (const Slice &S : P)
- if (!isVectorPromotionViableForSlice(P, S, VTy, ElementSize, DL, VScale))
- return false;
-
- for (const Slice *S : P.splitSliceTails())
- if (!isVectorPromotionViableForSlice(P, *S, VTy, ElementSize, DL, VScale))
- return false;
-
- return true;
-}
-
/// Test whether any vector type in \p CandidateTys is viable for promotion.
///
/// This implements the necessary checking for \c isVectorPromotionViable over
More information about the llvm-commits
mailing list