Skip to content

Commit 1c14d00

Browse files
ChengjunpDharuniRAcharya
authored andcommitted
[SROA] Add Stored Value Size Check for Tree-Structured Merge (llvm#162921)
The change fixes a bug in the SROA where tree-structured merge optimization was incorrectly applied when the size of the stored value was not a multiple of the new allocated element type size. The original change is llvm#152793. A simple repro would be ``` define <1 x i32> @foo(<1 x i16> %a, <1 x i16> %b) { entry: %alloca = alloca [1 x i32] %ptr0 = getelementptr inbounds [2 x i16], ptr %alloca, i32 0, i32 0 store <1 x i16> %a, ptr %ptr0 %ptr1 = getelementptr inbounds [2 x i16], ptr %alloca, i32 0, i32 1 store <1 x i16> %b, ptr %ptr1 %result = load <1 x i32>, ptr %alloca ret <1 x i32> %result } ``` Currently, this will lead to a compile time crash. In this change, we will skip the tree-structured merge for this case and fall back to normal SROA.
1 parent 00533ca commit 1c14d00

File tree

2 files changed

+22
-0
lines changed

2 files changed

+22
-0
lines changed

llvm/lib/Transforms/Scalar/SROA.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2961,6 +2961,7 @@ class AllocaSliceRewriter : public InstVisitor<AllocaSliceRewriter, bool> {
29612961
isa<FixedVectorType>(NewAI.getAllocatedType())
29622962
? cast<FixedVectorType>(NewAI.getAllocatedType())->getElementType()
29632963
: Type::getInt8Ty(NewAI.getContext());
2964+
unsigned AllocatedEltTySize = DL.getTypeSizeInBits(AllocatedEltTy);
29642965

29652966
// Helper to check if a type is
29662967
// 1. A fixed vector type
@@ -2991,10 +2992,17 @@ class AllocaSliceRewriter : public InstVisitor<AllocaSliceRewriter, bool> {
29912992
// Do not handle the case if
29922993
// 1. The store does not meet the conditions in the helper function
29932994
// 2. The store is volatile
2995+
// 3. The total store size is not a multiple of the allocated element
2996+
// type size
29942997
if (!IsTypeValidForTreeStructuredMerge(
29952998
SI->getValueOperand()->getType()) ||
29962999
SI->isVolatile())
29973000
return std::nullopt;
3001+
auto *VecTy = cast<FixedVectorType>(SI->getValueOperand()->getType());
3002+
unsigned NumElts = VecTy->getNumElements();
3003+
unsigned EltSize = DL.getTypeSizeInBits(VecTy->getElementType());
3004+
if (NumElts * EltSize % AllocatedEltTySize != 0)
3005+
return std::nullopt;
29983006
StoreInfos.emplace_back(SI, S.beginOffset(), S.endOffset(),
29993007
SI->getValueOperand());
30003008
} else {

llvm/test/Transforms/SROA/vector-promotion-cannot-tree-structure-merge.ll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,4 +219,18 @@ entry:
219219

220220
}
221221

222+
define <1 x i32> @test_store_value_size_not_multiple_of_allocated_element_type_size(<1 x i16> %a, <1 x i16> %b) {
223+
entry:
224+
%alloca = alloca [2 x i16]
225+
226+
%ptr0 = getelementptr inbounds [2 x i16], ptr %alloca, i32 0, i32 0
227+
store <1 x i16> %a, ptr %ptr0
228+
229+
%ptr1 = getelementptr inbounds [2 x i16], ptr %alloca, i32 0, i32 1
230+
store <1 x i16> %b, ptr %ptr1
231+
232+
%result = load <1 x i32>, ptr %alloca
233+
ret <1 x i32> %result
234+
}
235+
222236
declare void @llvm.memset.p0.i64(ptr nocapture writeonly, i8, i64, i1 immarg)

0 commit comments

Comments
 (0)