Skip to content

Commit 8813358

Browse files
nikictmandry
authored andcommitted
[InstCombine] Fix infinite loop due to bitcast <-> phi transforms
Fix for https://bugs.llvm.org/show_bug.cgi?id=44245. The optimizeBitCastFromPhi() and FoldPHIArgOpIntoPHI() end up fighting against each other, because optimizeBitCastFromPhi() assumes that bitcasts of loads will get folded. This doesn't happen here, because a dangling phi node prevents the one-use fold in https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp#L620-L628 from triggering. This patch fixes the issue by explicitly performing the load combine as part of the bitcast of phi transform. Other attempts to force the load to be combined first were ultimately too unreliable. Differential Revision: https://reviews.llvm.org/D71164
1 parent 24100f0 commit 8813358

File tree

2 files changed

+200
-3
lines changed

2 files changed

+200
-3
lines changed

llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

+8-3
Original file line numberDiff line numberDiff line change
@@ -2234,9 +2234,14 @@ Instruction *InstCombiner::optimizeBitCastFromPhi(CastInst &CI, PHINode *PN) {
22342234
if (auto *C = dyn_cast<Constant>(V)) {
22352235
NewV = ConstantExpr::getBitCast(C, DestTy);
22362236
} else if (auto *LI = dyn_cast<LoadInst>(V)) {
2237-
Builder.SetInsertPoint(LI->getNextNode());
2238-
NewV = Builder.CreateBitCast(LI, DestTy);
2239-
Worklist.Add(LI);
2237+
// Explicitly perform load combine to make sure no opposing transform
2238+
// can remove the bitcast in the meantime and trigger an infinite loop.
2239+
Builder.SetInsertPoint(LI);
2240+
NewV = combineLoadToNewType(*LI, DestTy);
2241+
// Remove the old load and its use in the old phi, which itself becomes
2242+
// dead once the whole transform finishes.
2243+
replaceInstUsesWith(*LI, UndefValue::get(LI->getType()));
2244+
eraseInstFromFunction(*LI);
22402245
} else if (auto *BCI = dyn_cast<BitCastInst>(V)) {
22412246
NewV = BCI->getOperand(0);
22422247
} else if (auto *PrevPN = dyn_cast<PHINode>(V)) {
+192
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
2+
; RUN: opt -S -instcombine -instcombine-infinite-loop-threshold=2 < %s | FileCheck %s
3+
4+
; This used to cause on infinite instcombine loop.
5+
6+
define void @test(i1 %c) {
7+
; CHECK-LABEL: @test(
8+
; CHECK-NEXT: bb16:
9+
; CHECK-NEXT: br i1 [[C:%.*]], label [[BB17:%.*]], label [[BB24:%.*]]
10+
; CHECK: bb17:
11+
; CHECK-NEXT: [[TMP0:%.*]] = phi i8* [ [[TMP1:%.*]], [[BB47:%.*]] ], [ undef, [[BB16:%.*]] ]
12+
; CHECK-NEXT: store i8* [[TMP0]], i8** undef, align 8
13+
; CHECK-NEXT: ret void
14+
; CHECK: bb24:
15+
; CHECK-NEXT: br i1 [[C]], label [[BB44:%.*]], label [[BB49:%.*]]
16+
; CHECK: bb44:
17+
; CHECK-NEXT: [[TMP467:%.*]] = load i8*, i8** inttoptr (i64 16 to i8**), align 16
18+
; CHECK-NEXT: br label [[BB47]]
19+
; CHECK: bb47:
20+
; CHECK-NEXT: [[TMP1]] = phi i8* [ [[TMP2:%.*]], [[BB150:%.*]] ], [ [[TMP1221:%.*]], [[BB119:%.*]] ], [ [[TMP1032:%.*]], [[BB101:%.*]] ], [ [[TMP933:%.*]], [[BB91:%.*]] ], [ [[TMP834:%.*]], [[BB81:%.*]] ], [ [[TMP705:%.*]], [[BB67:%.*]] ], [ [[TMP586:%.*]], [[BB56:%.*]] ], [ [[TMP467]], [[BB44]] ]
21+
; CHECK-NEXT: br label [[BB17]]
22+
; CHECK: bb49:
23+
; CHECK-NEXT: br i1 [[C]], label [[BB56]], label [[BB59:%.*]]
24+
; CHECK: bb56:
25+
; CHECK-NEXT: [[TMP586]] = load i8*, i8** inttoptr (i64 16 to i8**), align 16
26+
; CHECK-NEXT: br label [[BB47]]
27+
; CHECK: bb59:
28+
; CHECK-NEXT: br i1 [[C]], label [[BB67]], label [[BB71:%.*]]
29+
; CHECK: bb67:
30+
; CHECK-NEXT: [[TMP705]] = load i8*, i8** inttoptr (i64 16 to i8**), align 16
31+
; CHECK-NEXT: br label [[BB47]]
32+
; CHECK: bb71:
33+
; CHECK-NEXT: br i1 [[C]], label [[BB81]], label [[BB84:%.*]]
34+
; CHECK: bb81:
35+
; CHECK-NEXT: [[TMP834]] = load i8*, i8** inttoptr (i64 16 to i8**), align 16
36+
; CHECK-NEXT: br label [[BB47]]
37+
; CHECK: bb84:
38+
; CHECK-NEXT: br i1 [[C]], label [[BB91]], label [[BB94:%.*]]
39+
; CHECK: bb91:
40+
; CHECK-NEXT: [[TMP933]] = load i8*, i8** inttoptr (i64 16 to i8**), align 16
41+
; CHECK-NEXT: br label [[BB47]]
42+
; CHECK: bb94:
43+
; CHECK-NEXT: br i1 [[C]], label [[BB101]], label [[BB104:%.*]]
44+
; CHECK: bb101:
45+
; CHECK-NEXT: [[TMP1032]] = load i8*, i8** inttoptr (i64 16 to i8**), align 16
46+
; CHECK-NEXT: br label [[BB47]]
47+
; CHECK: bb104:
48+
; CHECK-NEXT: br i1 [[C]], label [[BB119]], label [[BB123:%.*]]
49+
; CHECK: bb119:
50+
; CHECK-NEXT: [[TMP1221]] = load i8*, i8** inttoptr (i64 16 to i8**), align 16
51+
; CHECK-NEXT: br label [[BB47]]
52+
; CHECK: bb123:
53+
; CHECK-NEXT: br i1 [[C]], label [[BB147:%.*]], label [[BB152:%.*]]
54+
; CHECK: bb147:
55+
; CHECK-NEXT: [[TMP1499:%.*]] = load i8*, i8** inttoptr (i64 16 to i8**), align 16
56+
; CHECK-NEXT: br label [[BB150]]
57+
; CHECK: bb150:
58+
; CHECK-NEXT: [[TMP2]] = phi i8* [ [[TMP1848:%.*]], [[BB152]] ], [ [[TMP1499]], [[BB147]] ]
59+
; CHECK-NEXT: br label [[BB47]]
60+
; CHECK: bb152:
61+
; CHECK-NEXT: [[TMP1848]] = load i8*, i8** inttoptr (i64 16 to i8**), align 16
62+
; CHECK-NEXT: call void undef()
63+
; CHECK-NEXT: br label [[BB150]]
64+
;
65+
bb16: ; preds = %bb
66+
br i1 %c, label %bb17, label %bb24
67+
68+
bb17: ; preds = %bb47, %bb17
69+
%0 = phi i8* [ %1, %bb47 ], [ undef, %bb16 ]
70+
store i8* %0, i8** undef, align 8
71+
ret void
72+
73+
bb24: ; preds = %bb24
74+
br i1 %c, label %bb44, label %bb49
75+
76+
bb44: ; preds = %bb43
77+
%tmp46 = load i64*, i64** inttoptr (i64 16 to i64**), align 16
78+
br label %bb47
79+
80+
bb47: ; preds = %bb150, %bb119, %bb101, %bb91, %bb81, %bb67, %bb56, %bb44
81+
%.in1 = phi i64* [ %.in, %bb150 ], [ %tmp122, %bb119 ], [ %tmp103, %bb101 ], [ %tmp93, %bb91 ], [ %tmp83, %bb81 ], [ %tmp70, %bb67 ], [ %tmp58, %bb56 ], [ %tmp46, %bb44 ]
82+
%1 = bitcast i64* %.in1 to i8*
83+
br label %bb17
84+
85+
bb49: ; preds = %bb49
86+
br i1 %c, label %bb56, label %bb59
87+
88+
bb56: ; preds = %bb55
89+
%tmp58 = load i64*, i64** inttoptr (i64 16 to i64**), align 16
90+
br label %bb47
91+
92+
bb59: ; preds = %bb59
93+
br i1 %c, label %bb67, label %bb71
94+
95+
bb67: ; preds = %bb66
96+
%tmp70 = load i64*, i64** inttoptr (i64 16 to i64**), align 16
97+
br label %bb47
98+
99+
bb71: ; preds = %bb71
100+
br i1 %c, label %bb81, label %bb84
101+
102+
bb81: ; preds = %bb80
103+
%tmp83 = load i64*, i64** inttoptr (i64 16 to i64**), align 16
104+
br label %bb47
105+
106+
bb84: ; preds = %bb84
107+
br i1 %c, label %bb91, label %bb94
108+
109+
bb91: ; preds = %bb90
110+
%tmp93 = load i64*, i64** inttoptr (i64 16 to i64**), align 16
111+
br label %bb47
112+
113+
bb94: ; preds = %bb94
114+
br i1 %c, label %bb101, label %bb104
115+
116+
bb101: ; preds = %bb100
117+
%tmp103 = load i64*, i64** inttoptr (i64 16 to i64**), align 16
118+
br label %bb47
119+
120+
bb104: ; preds = %bb104
121+
br i1 %c, label %bb119, label %bb123
122+
123+
bb119: ; preds = %bb118
124+
%tmp122 = load i64*, i64** inttoptr (i64 16 to i64**), align 16
125+
br label %bb47
126+
127+
bb123: ; preds = %bb123
128+
br i1 %c, label %bb147, label %bb152
129+
130+
bb147: ; preds = %bb146
131+
%tmp149 = load i64*, i64** inttoptr (i64 16 to i64**), align 16
132+
br label %bb150
133+
134+
bb150: ; preds = %bb152, %bb147
135+
%.in = phi i64* [ %tmp184, %bb152 ], [ %tmp149, %bb147 ]
136+
br label %bb47
137+
138+
bb152: ; preds = %bb146
139+
%tmp184 = load i64*, i64** inttoptr (i64 16 to i64**), align 16
140+
call void undef()
141+
br label %bb150
142+
}
143+
144+
; This used to cause an instcombine loop when the problem above was
145+
; addressed in a non-robust fashion.
146+
147+
%type_1 = type {}
148+
%type_2 = type {}
149+
%type_3 = type {}
150+
151+
define void @test_2(i1 %c) local_unnamed_addr {
152+
; CHECK-LABEL: @test_2(
153+
; CHECK-NEXT: entry:
154+
; CHECK-NEXT: br label [[WHILE_COND:%.*]]
155+
; CHECK: while.cond:
156+
; CHECK-NEXT: br label [[FOR_COND:%.*]]
157+
; CHECK: for.cond:
158+
; CHECK-NEXT: br i1 [[C:%.*]], label [[COND_TRUE133:%.*]], label [[COND_FALSE138:%.*]]
159+
; CHECK: cond.true133:
160+
; CHECK-NEXT: store %type_3* undef, %type_3** null, align 536870912
161+
; CHECK-NEXT: br label [[COND_END144:%.*]]
162+
; CHECK: cond.false138:
163+
; CHECK-NEXT: store %type_3* undef, %type_3** null, align 536870912
164+
; CHECK-NEXT: br label [[COND_END144]]
165+
; CHECK: cond.end144:
166+
; CHECK-NEXT: br label [[WHILE_COND]]
167+
;
168+
entry:
169+
br label %while.cond
170+
171+
while.cond: ; preds = %cond.end144, %entry
172+
%link.0 = phi %type_2* [ undef, %entry ], [ %cond145, %cond.end144 ]
173+
%os115 = bitcast %type_2* %link.0 to %type_3*
174+
%ou116 = getelementptr inbounds %type_3, %type_3* %os115, i32 0
175+
%os1117 = bitcast %type_3* %ou116 to %type_1*
176+
br label %for.cond
177+
178+
for.cond: ; preds = %while.cond
179+
br i1 %c, label %cond.true133, label %cond.false138
180+
181+
cond.true133: ; preds = %sw.epilog
182+
%0 = load %type_2*, %type_2** undef, align 8
183+
br label %cond.end144
184+
185+
cond.false138: ; preds = %sw.epilog
186+
%1 = load %type_2*, %type_2** undef, align 8
187+
br label %cond.end144
188+
189+
cond.end144: ; preds = %cond.false138, %cond.true133
190+
%cond145 = phi %type_2* [ %0, %cond.true133 ], [ %1, %cond.false138 ]
191+
br label %while.cond
192+
}

0 commit comments

Comments
 (0)