Skip to content

Commit ceee53c

Browse files
aeubankstru
authored andcommitted
[SROA] Don't speculate phis with different load user types
Fixes an SROA crash. Fallout from opaque pointers since with typed pointers we'd bail out at the bitcast. Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D136119 (cherry picked from commit 6219ec0)
1 parent 086365b commit ceee53c

File tree

2 files changed

+53
-7
lines changed

2 files changed

+53
-7
lines changed

llvm/lib/Transforms/Scalar/SROA.cpp

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,8 +1210,7 @@ static bool isSafePHIToSpeculate(PHINode &PN) {
12101210
BasicBlock *BB = PN.getParent();
12111211
Align MaxAlign;
12121212
uint64_t APWidth = DL.getIndexTypeSizeInBits(PN.getType());
1213-
APInt MaxSize(APWidth, 0);
1214-
bool HaveLoad = false;
1213+
Type *LoadType = nullptr;
12151214
for (User *U : PN.users()) {
12161215
LoadInst *LI = dyn_cast<LoadInst>(U);
12171216
if (!LI || !LI->isSimple())
@@ -1223,21 +1222,27 @@ static bool isSafePHIToSpeculate(PHINode &PN) {
12231222
if (LI->getParent() != BB)
12241223
return false;
12251224

1225+
if (LoadType) {
1226+
if (LoadType != LI->getType())
1227+
return false;
1228+
} else {
1229+
LoadType = LI->getType();
1230+
}
1231+
12261232
// Ensure that there are no instructions between the PHI and the load that
12271233
// could store.
12281234
for (BasicBlock::iterator BBI(PN); &*BBI != LI; ++BBI)
12291235
if (BBI->mayWriteToMemory())
12301236
return false;
12311237

1232-
uint64_t Size = DL.getTypeStoreSize(LI->getType()).getFixedSize();
12331238
MaxAlign = std::max(MaxAlign, LI->getAlign());
1234-
MaxSize = MaxSize.ult(Size) ? APInt(APWidth, Size) : MaxSize;
1235-
HaveLoad = true;
12361239
}
12371240

1238-
if (!HaveLoad)
1241+
if (!LoadType)
12391242
return false;
12401243

1244+
APInt LoadSize = APInt(APWidth, DL.getTypeStoreSize(LoadType).getFixedSize());
1245+
12411246
// We can only transform this if it is safe to push the loads into the
12421247
// predecessor blocks. The only thing to watch out for is that we can't put
12431248
// a possibly trapping load in the predecessor if it is a critical edge.
@@ -1259,7 +1264,7 @@ static bool isSafePHIToSpeculate(PHINode &PN) {
12591264
// If this pointer is always safe to load, or if we can prove that there
12601265
// is already a load in the block, then we can move the load to the pred
12611266
// block.
1262-
if (isSafeToLoadUnconditionally(InVal, MaxAlign, MaxSize, DL, TI))
1267+
if (isSafeToLoadUnconditionally(InVal, MaxAlign, LoadSize, DL, TI))
12631268
continue;
12641269

12651270
return false;
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
2+
; RUN: opt -passes=sroa < %s -S | FileCheck %s
3+
4+
define void @f(i1 %i) {
5+
; CHECK-LABEL: @f(
6+
; CHECK-NEXT: [[A1:%.*]] = alloca i64, align 8
7+
; CHECK-NEXT: [[A2:%.*]] = alloca i64, align 8
8+
; CHECK-NEXT: br i1 [[I:%.*]], label [[BB1:%.*]], label [[BB:%.*]]
9+
; CHECK: bb:
10+
; CHECK-NEXT: br label [[BB2:%.*]]
11+
; CHECK: bb1:
12+
; CHECK-NEXT: br label [[BB2]]
13+
; CHECK: bb2:
14+
; CHECK-NEXT: [[TMP3:%.*]] = phi ptr [ [[A1]], [[BB1]] ], [ [[A2]], [[BB]] ]
15+
; CHECK-NEXT: [[TMP5:%.*]] = load i32, ptr [[TMP3]], align 4
16+
; CHECK-NEXT: [[TMP4:%.*]] = load i64, ptr [[TMP3]], align 4
17+
; CHECK-NEXT: call void @use32(i32 [[TMP5]])
18+
; CHECK-NEXT: call void @use64(i64 [[TMP4]])
19+
; CHECK-NEXT: ret void
20+
;
21+
%a1 = alloca i64
22+
%a2 = alloca i64
23+
br i1 %i, label %bb1, label %bb
24+
25+
bb:
26+
br label %bb2
27+
28+
bb1:
29+
br label %bb2
30+
31+
bb2:
32+
%tmp3 = phi ptr [ %a1, %bb1 ], [ %a2, %bb ]
33+
%tmp5 = load i32, ptr %tmp3
34+
%tmp4 = load i64, ptr %tmp3
35+
call void @use32(i32 %tmp5)
36+
call void @use64(i64 %tmp4)
37+
ret void
38+
}
39+
40+
declare void @use32(i32)
41+
declare void @use64(i64)

0 commit comments

Comments
 (0)