Skip to content

Commit f4bd307

Browse files
YonahGoldberggithub-actions[bot]
authored andcommitted
Automerge: [NFC][SROA] Clean up rewritePartition type selection process (#169106)
This change reverts llvm/llvm-project@2572512, 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. llvm/llvm-project@2572512 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 llvm/llvm-project@529eafd, 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 llvm/llvm-project@529eafd, 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. I had some other ideas that I tried in llvm/llvm-project#167771 and llvm/llvm-project#168796, but they need refinement.
2 parents 70a686b + 4ca61f5 commit f4bd307

File tree

1 file changed

+25
-51
lines changed

1 file changed

+25
-51
lines changed

llvm/lib/Transforms/Scalar/SROA.cpp

Lines changed: 25 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2178,35 +2178,6 @@ static bool isVectorPromotionViableForSlice(Partition &P, const Slice &S,
21782178
return true;
21792179
}
21802180

2181-
/// Test whether a vector type is viable for promotion.
2182-
///
2183-
/// This implements the necessary checking for \c checkVectorTypesForPromotion
2184-
/// (and thus isVectorPromotionViable) over all slices of the alloca for the
2185-
/// given VectorType.
2186-
static bool checkVectorTypeForPromotion(Partition &P, VectorType *VTy,
2187-
const DataLayout &DL, unsigned VScale) {
2188-
uint64_t ElementSize =
2189-
DL.getTypeSizeInBits(VTy->getElementType()).getFixedValue();
2190-
2191-
// While the definition of LLVM vectors is bitpacked, we don't support sizes
2192-
// that aren't byte sized.
2193-
if (ElementSize % 8)
2194-
return false;
2195-
assert((DL.getTypeSizeInBits(VTy).getFixedValue() % 8) == 0 &&
2196-
"vector size not a multiple of element size?");
2197-
ElementSize /= 8;
2198-
2199-
for (const Slice &S : P)
2200-
if (!isVectorPromotionViableForSlice(P, S, VTy, ElementSize, DL, VScale))
2201-
return false;
2202-
2203-
for (const Slice *S : P.splitSliceTails())
2204-
if (!isVectorPromotionViableForSlice(P, *S, VTy, ElementSize, DL, VScale))
2205-
return false;
2206-
2207-
return true;
2208-
}
2209-
22102181
/// Test whether any vector type in \p CandidateTys is viable for promotion.
22112182
///
22122183
/// This implements the necessary checking for \c isVectorPromotionViable over
@@ -2291,11 +2262,30 @@ checkVectorTypesForPromotion(Partition &P, const DataLayout &DL,
22912262
std::numeric_limits<unsigned short>::max();
22922263
});
22932264

2294-
for (VectorType *VTy : CandidateTys)
2295-
if (checkVectorTypeForPromotion(P, VTy, DL, VScale))
2296-
return VTy;
2265+
// Find a vector type viable for promotion by iterating over all slices.
2266+
auto *VTy = llvm::find_if(CandidateTys, [&](VectorType *VTy) -> bool {
2267+
uint64_t ElementSize =
2268+
DL.getTypeSizeInBits(VTy->getElementType()).getFixedValue();
22972269

2298-
return nullptr;
2270+
// While the definition of LLVM vectors is bitpacked, we don't support sizes
2271+
// that aren't byte sized.
2272+
if (ElementSize % 8)
2273+
return false;
2274+
assert((DL.getTypeSizeInBits(VTy).getFixedValue() % 8) == 0 &&
2275+
"vector size not a multiple of element size?");
2276+
ElementSize /= 8;
2277+
2278+
for (const Slice &S : P)
2279+
if (!isVectorPromotionViableForSlice(P, S, VTy, ElementSize, DL, VScale))
2280+
return false;
2281+
2282+
for (const Slice *S : P.splitSliceTails())
2283+
if (!isVectorPromotionViableForSlice(P, *S, VTy, ElementSize, DL, VScale))
2284+
return false;
2285+
2286+
return true;
2287+
});
2288+
return VTy != CandidateTys.end() ? *VTy : nullptr;
22992289
}
23002290

23012291
static VectorType *createAndCheckVectorTypesForPromotion(
@@ -5205,7 +5195,6 @@ AllocaInst *SROA::rewritePartition(AllocaInst &AI, AllocaSlices &AS,
52055195
// won't always succeed, in which case we fall back to a legal integer type
52065196
// or an i8 array of an appropriate size.
52075197
Type *SliceTy = nullptr;
5208-
VectorType *SliceVecTy = nullptr;
52095198
const DataLayout &DL = AI.getDataLayout();
52105199
unsigned VScale = AI.getFunction()->getVScaleValue();
52115200

@@ -5214,10 +5203,8 @@ AllocaInst *SROA::rewritePartition(AllocaInst &AI, AllocaSlices &AS,
52145203
// Do all uses operate on the same type?
52155204
if (CommonUseTy.first) {
52165205
TypeSize CommonUseSize = DL.getTypeAllocSize(CommonUseTy.first);
5217-
if (CommonUseSize.isFixed() && CommonUseSize.getFixedValue() >= P.size()) {
5206+
if (CommonUseSize.isFixed() && CommonUseSize.getFixedValue() >= P.size())
52185207
SliceTy = CommonUseTy.first;
5219-
SliceVecTy = dyn_cast<VectorType>(SliceTy);
5220-
}
52215208
}
52225209
// If not, can we find an appropriate subtype in the original allocated type?
52235210
if (!SliceTy)
@@ -5227,27 +5214,14 @@ AllocaInst *SROA::rewritePartition(AllocaInst &AI, AllocaSlices &AS,
52275214

52285215
// If still not, can we use the largest bitwidth integer type used?
52295216
if (!SliceTy && CommonUseTy.second)
5230-
if (DL.getTypeAllocSize(CommonUseTy.second).getFixedValue() >= P.size()) {
5217+
if (DL.getTypeAllocSize(CommonUseTy.second).getFixedValue() >= P.size())
52315218
SliceTy = CommonUseTy.second;
5232-
SliceVecTy = dyn_cast<VectorType>(SliceTy);
5233-
}
52345219
if ((!SliceTy || (SliceTy->isArrayTy() &&
52355220
SliceTy->getArrayElementType()->isIntegerTy())) &&
52365221
DL.isLegalInteger(P.size() * 8)) {
52375222
SliceTy = Type::getIntNTy(*C, P.size() * 8);
52385223
}
52395224

5240-
// If the common use types are not viable for promotion then attempt to find
5241-
// another type that is viable.
5242-
if (SliceVecTy && !checkVectorTypeForPromotion(P, SliceVecTy, DL, VScale))
5243-
if (Type *TypePartitionTy = getTypePartition(DL, AI.getAllocatedType(),
5244-
P.beginOffset(), P.size())) {
5245-
VectorType *TypePartitionVecTy = dyn_cast<VectorType>(TypePartitionTy);
5246-
if (TypePartitionVecTy &&
5247-
checkVectorTypeForPromotion(P, TypePartitionVecTy, DL, VScale))
5248-
SliceTy = TypePartitionTy;
5249-
}
5250-
52515225
if (!SliceTy)
52525226
SliceTy = ArrayType::get(Type::getInt8Ty(*C), P.size());
52535227
assert(DL.getTypeAllocSize(SliceTy).getFixedValue() >= P.size());

0 commit comments

Comments
 (0)