Skip to content

Commit 41cef78

Browse files
spaitsspaits
authored
Reland "[BasicBlockUtils] Handle funclets when detaching EH pad blocks" (#158435)
When removing EH Pad blocks, the value defined by them becomes poison. These poison values are then used by `catchret` and `cleanupret`, which is invalid. This commit replaces those unreachable `catchret` and `cleanupret` instructions with `unreachable`.
1 parent ff05dc4 commit 41cef78

File tree

2 files changed

+321
-28
lines changed

2 files changed

+321
-28
lines changed

‎llvm/lib/Transforms/Utils/BasicBlockUtils.cpp‎

Lines changed: 85 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -58,37 +58,94 @@ static cl::opt<unsigned> MaxDeoptOrUnreachableSuccessorCheckDepth(
5858
"is followed by a block that either has a terminating "
5959
"deoptimizing call or is terminated with an unreachable"));
6060

61-
void llvm::detachDeadBlocks(
62-
ArrayRef<BasicBlock *> BBs,
63-
SmallVectorImpl<DominatorTree::UpdateType> *Updates,
64-
bool KeepOneInputPHIs) {
61+
/// Zap all the instructions in the block and replace them with an unreachable
62+
/// instruction and notify the basic block's successors that one of their
63+
/// predecessors is going away.
64+
static void
65+
emptyAndDetachBlock(BasicBlock *BB,
66+
SmallVectorImpl<DominatorTree::UpdateType> *Updates,
67+
bool KeepOneInputPHIs) {
68+
// Loop through all of our successors and make sure they know that one
69+
// of their predecessors is going away.
70+
SmallPtrSet<BasicBlock *, 4> UniqueSuccessors;
71+
for (BasicBlock *Succ : successors(BB)) {
72+
Succ->removePredecessor(BB, KeepOneInputPHIs);
73+
if (Updates && UniqueSuccessors.insert(Succ).second)
74+
Updates->push_back({DominatorTree::Delete, BB, Succ});
75+
}
76+
77+
// Zap all the instructions in the block.
78+
while (!BB->empty()) {
79+
Instruction &I = BB->back();
80+
// If this instruction is used, replace uses with an arbitrary value.
81+
// Because control flow can't get here, we don't care what we replace the
82+
// value with. Note that since this block is unreachable, and all values
83+
// contained within it must dominate their uses, that all uses will
84+
// eventually be removed (they are themselves dead).
85+
if (!I.use_empty())
86+
I.replaceAllUsesWith(PoisonValue::get(I.getType()));
87+
BB->back().eraseFromParent();
88+
}
89+
new UnreachableInst(BB->getContext(), BB);
90+
assert(BB->size() == 1 && isa<UnreachableInst>(BB->getTerminator()) &&
91+
"The successor list of BB isn't empty before "
92+
"applying corresponding DTU updates.");
93+
}
94+
95+
void llvm::detachDeadBlocks(ArrayRef<BasicBlock *> BBs,
96+
SmallVectorImpl<DominatorTree::UpdateType> *Updates,
97+
bool KeepOneInputPHIs) {
6598
for (auto *BB : BBs) {
66-
// Loop through all of our successors and make sure they know that one
67-
// of their predecessors is going away.
68-
SmallPtrSet<BasicBlock *, 4> UniqueSuccessors;
69-
for (BasicBlock *Succ : successors(BB)) {
70-
Succ->removePredecessor(BB, KeepOneInputPHIs);
71-
if (Updates && UniqueSuccessors.insert(Succ).second)
72-
Updates->push_back({DominatorTree::Delete, BB, Succ});
99+
auto NonFirstPhiIt = BB->getFirstNonPHIIt();
100+
if (NonFirstPhiIt != BB->end()) {
101+
Instruction &I = *NonFirstPhiIt;
102+
// Exception handling funclets need to be explicitly addressed.
103+
// These funclets must begin with cleanuppad or catchpad and end with
104+
// cleanupred or catchret. The return instructions can be in different
105+
// basic blocks than the pad instruction. If we would only delete the
106+
// first block, the we would have possible cleanupret and catchret
107+
// instructions with poison arguments, which wouldn't be valid.
108+
if (isa<FuncletPadInst>(I)) {
109+
for (User *User : make_early_inc_range(I.users())) {
110+
Instruction *ReturnInstr = dyn_cast<Instruction>(User);
111+
// If we have a cleanupret or catchret block, replace it with just an
112+
// unreachable. The other alternative, that may use a catchpad is a
113+
// catchswitch. That does not need special handling for now.
114+
if (isa<CatchReturnInst>(ReturnInstr) ||
115+
isa<CleanupReturnInst>(ReturnInstr)) {
116+
BasicBlock *ReturnInstrBB = ReturnInstr->getParent();
117+
// This catchret or catchpad basic block is detached now. Let the
118+
// successors know it.
119+
// This basic block also may have some predecessors too. For
120+
// example the following LLVM-IR is valid:
121+
//
122+
// [cleanuppad_block]
123+
// |
124+
// [regular_block]
125+
// |
126+
// [cleanupret_block]
127+
//
128+
// The IR after the cleanup will look like this:
129+
//
130+
// [cleanuppad_block]
131+
// |
132+
// [regular_block]
133+
// |
134+
// [unreachable]
135+
//
136+
// So regular_block will lead to an unreachable block, which is also
137+
// valid. There is no need to replace regular_block with unreachable
138+
// in this context now.
139+
// On the other hand, the cleanupret/catchret block's successors
140+
// need to know about the deletion of their predecessors.
141+
emptyAndDetachBlock(ReturnInstrBB, Updates, KeepOneInputPHIs);
142+
}
143+
}
144+
}
73145
}
74146

75-
// Zap all the instructions in the block.
76-
while (!BB->empty()) {
77-
Instruction &I = BB->back();
78-
// If this instruction is used, replace uses with an arbitrary value.
79-
// Because control flow can't get here, we don't care what we replace the
80-
// value with. Note that since this block is unreachable, and all values
81-
// contained within it must dominate their uses, that all uses will
82-
// eventually be removed (they are themselves dead).
83-
if (!I.use_empty())
84-
I.replaceAllUsesWith(PoisonValue::get(I.getType()));
85-
BB->back().eraseFromParent();
86-
}
87-
new UnreachableInst(BB->getContext(), BB);
88-
assert(BB->size() == 1 &&
89-
isa<UnreachableInst>(BB->getTerminator()) &&
90-
"The successor list of BB isn't empty before "
91-
"applying corresponding DTU updates.");
147+
// Detaching and emptying the current basic block.
148+
emptyAndDetachBlock(BB, Updates, KeepOneInputPHIs);
92149
}
93150
}
94151

‎llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll‎

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -passes=simplifycfg -S < %s | FileCheck %s
3+
4+
; cleanuppad/cleanupret
5+
6+
define void @unreachable_cleanuppad_linear(i64 %shapes.1) personality ptr null {
7+
; CHECK-LABEL: define void @unreachable_cleanuppad_linear(
8+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
9+
; CHECK-NEXT: [[START:.*:]]
10+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
11+
; CHECK-NEXT: ret void
12+
;
13+
start:
14+
%_7 = icmp ult i64 0, %shapes.1
15+
ret void
16+
17+
funclet:
18+
%cleanuppad = cleanuppad within none []
19+
br label %funclet_end
20+
21+
funclet_end:
22+
cleanupret from %cleanuppad unwind to caller
23+
}
24+
25+
define void @unreachable_cleanuppad_linear_middle_block(i64 %shapes.1) personality ptr null {
26+
; CHECK-LABEL: define void @unreachable_cleanuppad_linear_middle_block(
27+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
28+
; CHECK-NEXT: [[START:.*:]]
29+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
30+
; CHECK-NEXT: ret void
31+
;
32+
start:
33+
%_7 = icmp ult i64 0, %shapes.1
34+
ret void
35+
36+
funclet:
37+
%cleanuppad = cleanuppad within none []
38+
br label %middle_block
39+
40+
middle_block:
41+
%tmp1 = add i64 %shapes.1, 42
42+
br label %funclet_end
43+
44+
funclet_end:
45+
cleanupret from %cleanuppad unwind to caller
46+
}
47+
48+
define void @unreachable_cleanuppad_multiple_predecessors(i64 %shapes.1) personality ptr null {
49+
; CHECK-LABEL: define void @unreachable_cleanuppad_multiple_predecessors(
50+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
51+
; CHECK-NEXT: [[START:.*:]]
52+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
53+
; CHECK-NEXT: ret void
54+
;
55+
start:
56+
%_7 = icmp ult i64 0, %shapes.1
57+
ret void
58+
59+
funclet:
60+
%cleanuppad = cleanuppad within none []
61+
switch i64 %shapes.1, label %otherwise [ i64 0, label %one
62+
i64 1, label %two
63+
i64 42, label %three ]
64+
one:
65+
br label %funclet_end
66+
67+
two:
68+
br label %funclet_end
69+
70+
three:
71+
br label %funclet_end
72+
73+
otherwise:
74+
br label %funclet_end
75+
76+
funclet_end:
77+
cleanupret from %cleanuppad unwind to caller
78+
}
79+
80+
; catchpad/catchret
81+
82+
define void @unreachable_catchpad_linear(i64 %shapes.1) personality ptr null {
83+
; CHECK-LABEL: define void @unreachable_catchpad_linear(
84+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
85+
; CHECK-NEXT: [[START:.*:]]
86+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
87+
; CHECK-NEXT: ret void
88+
;
89+
start:
90+
%_7 = icmp ult i64 0, %shapes.1
91+
ret void
92+
93+
dispatch:
94+
%cs = catchswitch within none [label %funclet] unwind to caller
95+
96+
funclet:
97+
%cleanuppad = catchpad within %cs []
98+
br label %funclet_end
99+
100+
101+
funclet_end:
102+
catchret from %cleanuppad to label %unreachable
103+
104+
unreachable:
105+
unreachable
106+
}
107+
108+
define void @unreachable_catchpad_multiple_predecessors(i64 %shapes.1) personality ptr null {
109+
; CHECK-LABEL: define void @unreachable_catchpad_multiple_predecessors(
110+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
111+
; CHECK-NEXT: [[START:.*:]]
112+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
113+
; CHECK-NEXT: ret void
114+
;
115+
start:
116+
%_7 = icmp ult i64 0, %shapes.1
117+
ret void
118+
119+
dispatch:
120+
%cs = catchswitch within none [label %funclet] unwind to caller
121+
122+
funclet:
123+
%cleanuppad = catchpad within %cs []
124+
switch i64 %shapes.1, label %otherwise [ i64 0, label %one
125+
i64 1, label %two
126+
i64 42, label %three ]
127+
one:
128+
br label %funclet_end
129+
130+
two:
131+
br label %funclet_end
132+
133+
three:
134+
br label %funclet_end
135+
136+
otherwise:
137+
br label %funclet_end
138+
139+
funclet_end:
140+
catchret from %cleanuppad to label %unreachable
141+
142+
unreachable:
143+
unreachable
144+
}
145+
146+
; Issue reproducer
147+
148+
define void @gh148052(i64 %shapes.1) personality ptr null {
149+
; CHECK-LABEL: define void @gh148052(
150+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
151+
; CHECK-NEXT: [[START:.*:]]
152+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
153+
; CHECK-NEXT: call void @llvm.assume(i1 [[_7]])
154+
; CHECK-NEXT: ret void
155+
;
156+
start:
157+
%_7 = icmp ult i64 0, %shapes.1
158+
br i1 %_7, label %bb1, label %panic
159+
160+
bb1:
161+
%_11 = icmp ult i64 0, %shapes.1
162+
br i1 %_11, label %bb3, label %panic1
163+
164+
panic:
165+
unreachable
166+
167+
bb3:
168+
ret void
169+
170+
panic1:
171+
invoke void @func(i64 0, i64 0, ptr null)
172+
to label %unreachable unwind label %funclet_bb14
173+
174+
funclet_bb14:
175+
%cleanuppad = cleanuppad within none []
176+
br label %bb13
177+
178+
unreachable:
179+
unreachable
180+
181+
bb10:
182+
cleanupret from %cleanuppad5 unwind to caller
183+
184+
funclet_bb10:
185+
%cleanuppad5 = cleanuppad within none []
186+
br label %bb10
187+
188+
bb13:
189+
cleanupret from %cleanuppad unwind label %funclet_bb10
190+
}
191+
192+
%struct.foo = type { ptr, %struct.eggs, ptr }
193+
%struct.eggs = type { ptr, ptr, ptr }
194+
195+
declare x86_thiscallcc ptr @quux(ptr, ptr, i32)
196+
197+
define x86_thiscallcc ptr @baz(ptr %arg, ptr %arg1, ptr %arg2, i1 %arg3, ptr %arg4) personality ptr null {
198+
; CHECK-LABEL: define x86_thiscallcc ptr @baz(
199+
; CHECK-SAME: ptr [[ARG:%.*]], ptr [[ARG1:%.*]], ptr [[ARG2:%.*]], i1 [[ARG3:%.*]], ptr [[ARG4:%.*]]) personality ptr null {
200+
; CHECK-NEXT: [[BB:.*:]]
201+
; CHECK-NEXT: [[ALLOCA:%.*]] = alloca [2 x %struct.foo], align 4
202+
; CHECK-NEXT: [[INVOKE:%.*]] = call x86_thiscallcc ptr @quux(ptr null, ptr null, i32 0) #[[ATTR1:[0-9]+]]
203+
; CHECK-NEXT: unreachable
204+
;
205+
bb:
206+
%alloca = alloca [2 x %struct.foo], align 4
207+
%invoke = invoke x86_thiscallcc ptr @quux(ptr null, ptr null, i32 0)
208+
to label %bb5 unwind label %bb10
209+
210+
bb5: ; preds = %bb
211+
%getelementptr = getelementptr i8, ptr %arg, i32 20
212+
%call = call x86_thiscallcc ptr null(ptr null, ptr null, i32 0)
213+
br label %bb6
214+
215+
bb6: ; preds = %bb10, %bb5
216+
%phi = phi ptr [ null, %bb10 ], [ null, %bb5 ]
217+
ret ptr %phi
218+
219+
bb7: ; No predecessors!
220+
%cleanuppad = cleanuppad within none []
221+
%getelementptr8 = getelementptr i8, ptr %arg2, i32 -20
222+
%icmp = icmp eq ptr %arg, null
223+
br label %bb9
224+
225+
bb9: ; preds = %bb7
226+
cleanupret from %cleanuppad unwind label %bb10
227+
228+
bb10: ; preds = %bb9, %bb
229+
%phi11 = phi ptr [ %arg, %bb9 ], [ null, %bb ]
230+
%cleanuppad12 = cleanuppad within none []
231+
%getelementptr13 = getelementptr i8, ptr %phi11, i32 -20
232+
store i32 0, ptr %phi11, align 4
233+
br label %bb6
234+
}
235+
236+
declare void @func(i64, i64, ptr)

0 commit comments

Comments
 (0)