Skip to content

Commit 0af7542

Browse files
aheejinaheejin
committed
Reapply "[WebAssembly] Fix phi handling for Wasm SjLj (#99730)"
This reapplies #99730. #99730 contained a nondeterministic iteration which failed the reverse-iteration bot (https://lab.llvm.org/buildbot/#/builders/110/builds/474) and reverted in f3f0d99. The fix is make the order of iteration of new predecessors determintistic by using `SmallSetVector`. ```diff --- a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp @@ -1689,7 +1689,7 @@ void WebAssemblyLowerEmscriptenEHSjLj::handleLongjmpableCallsForWasmSjLj( } } - SmallDenseMap<BasicBlock *, SmallPtrSet<BasicBlock *, 4>, 4> + SmallDenseMap<BasicBlock *, SmallSetVector<BasicBlock *, 4>, 4> UnwindDestToNewPreds; for (auto *CI : LongjmpableCalls) { // Even if the callee function has attribute 'nounwind', which is true for ```
1 parent 393a957 commit 0af7542

File tree

2 files changed

+176
-0
lines changed

2 files changed

+176
-0
lines changed

‎llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,8 @@ void WebAssemblyLowerEmscriptenEHSjLj::rebuildSSA(Function &F) {
777777
SSAUpdaterBulk SSA;
778778
for (BasicBlock &BB : F) {
779779
for (Instruction &I : BB) {
780+
if (I.getType()->isVoidTy())
781+
continue;
780782
unsigned VarID = SSA.AddVariable(I.getName(), I.getType());
781783
// If a value is defined by an invoke instruction, it is only available in
782784
// its normal destination and not in its unwind destination.
@@ -1687,6 +1689,8 @@ void WebAssemblyLowerEmscriptenEHSjLj::handleLongjmpableCallsForWasmSjLj(
16871689
}
16881690
}
16891691

1692+
SmallDenseMap<BasicBlock *, SmallSetVector<BasicBlock *, 4>, 4>
1693+
UnwindDestToNewPreds;
16901694
for (auto *CI : LongjmpableCalls) {
16911695
// Even if the callee function has attribute 'nounwind', which is true for
16921696
// all C functions, it can longjmp, which means it can throw a Wasm
@@ -1724,6 +1728,11 @@ void WebAssemblyLowerEmscriptenEHSjLj::handleLongjmpableCallsForWasmSjLj(
17241728
}
17251729
if (!UnwindDest)
17261730
UnwindDest = CatchDispatchLongjmpBB;
1731+
// Because we are changing a longjmpable call to an invoke, its unwind
1732+
// destination can be an existing EH pad that already have phis, and the BB
1733+
// with the newly created invoke will become a new predecessor of that EH
1734+
// pad. In this case we need to add the new predecessor to those phis.
1735+
UnwindDestToNewPreds[UnwindDest].insert(CI->getParent());
17271736
changeToInvokeAndSplitBasicBlock(CI, UnwindDest);
17281737
}
17291738

@@ -1752,4 +1761,45 @@ void WebAssemblyLowerEmscriptenEHSjLj::handleLongjmpableCallsForWasmSjLj(
17521761

17531762
for (Instruction *I : ToErase)
17541763
I->eraseFromParent();
1764+
1765+
// Add entries for new predecessors to phis in unwind destinations. We use
1766+
// 'undef' as a placeholder value. We should make sure the phis have a valid
1767+
// set of predecessors before running SSAUpdater, because SSAUpdater
1768+
// internally can use existing phis to gather predecessor info rather than
1769+
// scanning the actual CFG (See FindPredecessorBlocks in SSAUpdater.cpp for
1770+
// details).
1771+
for (auto &[UnwindDest, NewPreds] : UnwindDestToNewPreds) {
1772+
for (PHINode &PN : UnwindDest->phis()) {
1773+
for (auto *NewPred : NewPreds) {
1774+
assert(PN.getBasicBlockIndex(NewPred) == -1);
1775+
PN.addIncoming(UndefValue::get(PN.getType()), NewPred);
1776+
}
1777+
}
1778+
}
1779+
1780+
// For unwind destinations for newly added invokes to longjmpable functions,
1781+
// calculate incoming values for the newly added predecessors using
1782+
// SSAUpdater. We add existing values in the phis to SSAUpdater as available
1783+
// values and let it calculate what the value should be at the end of new
1784+
// incoming blocks.
1785+
for (auto &[UnwindDest, NewPreds] : UnwindDestToNewPreds) {
1786+
for (PHINode &PN : UnwindDest->phis()) {
1787+
SSAUpdater SSA;
1788+
SSA.Initialize(PN.getType(), PN.getName());
1789+
for (unsigned Idx = 0, E = PN.getNumIncomingValues(); Idx != E; ++Idx) {
1790+
if (NewPreds.contains(PN.getIncomingBlock(Idx)))
1791+
continue;
1792+
Value *V = PN.getIncomingValue(Idx);
1793+
if (auto *II = dyn_cast<InvokeInst>(V))
1794+
SSA.AddAvailableValue(II->getNormalDest(), II);
1795+
else if (auto *I = dyn_cast<Instruction>(V))
1796+
SSA.AddAvailableValue(I->getParent(), I);
1797+
else
1798+
SSA.AddAvailableValue(PN.getIncomingBlock(Idx), V);
1799+
}
1800+
for (auto *NewPred : NewPreds)
1801+
PN.setIncomingValueForBlock(NewPred, SSA.GetValueAtEndOfBlock(NewPred));
1802+
assert(PN.isComplete());
1803+
}
1804+
}
17551805
}

‎llvm/test/CodeGen/WebAssembly/lower-wasm-ehsjlj-phi.ll

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
; RUN: opt < %s -wasm-lower-em-ehsjlj -wasm-enable-eh -wasm-enable-sjlj -S | FileCheck %s
2+
3+
target triple = "wasm32-unknown-emscripten"
4+
5+
%struct.__jmp_buf_tag = type { [6 x i32], i32, [32 x i32] }
6+
@buf = internal global [1 x %struct.__jmp_buf_tag] zeroinitializer, align 16
7+
8+
; When longjmpable calls are coverted into invokes in Wasm SjLj transformation
9+
; and their unwind destination is an existing catchpad or cleanuppad due to
10+
; maintain the scope structure, the new pred BBs created by invokes and the
11+
; correct incoming values should be added the existing phis in those unwind
12+
; destinations.
13+
14+
; When longjmpable calls are within a cleanuppad.
15+
define void @longjmpable_invoke_phi0() personality ptr @__gxx_wasm_personality_v0 {
16+
; CHECK-LABEL: @longjmpable_invoke_phi0
17+
entry:
18+
%val.entry = call i32 @llvm.wasm.memory.size.i32(i32 0)
19+
%0 = call i32 @setjmp(ptr @buf) #2
20+
invoke void @foo()
21+
to label %bb1 unwind label %ehcleanup1
22+
23+
bb1: ; preds = %entry
24+
; We use llvm.wasm.memory.size intrinsic just to get/use an i32 value. The
25+
; reason we use an intrinsic here is to make it not longjmpable. If this can
26+
; longjmp, the result will be more complicated and hard to check.
27+
%val.bb1 = call i32 @llvm.wasm.memory.size.i32(i32 0)
28+
invoke void @foo()
29+
to label %bb2 unwind label %ehcleanup0
30+
31+
bb2: ; preds = %bb1
32+
unreachable
33+
34+
ehcleanup0: ; preds = %bb1
35+
%1 = cleanuppad within none []
36+
call void @longjmpable() [ "funclet"(token %1) ]
37+
; CHECK: ehcleanup0
38+
; CHECK: invoke void @longjmpable
39+
; CHECK-NEXT: to label %.noexc unwind label %ehcleanup1
40+
invoke void @foo() [ "funclet"(token %1) ]
41+
to label %bb3 unwind label %ehcleanup1
42+
43+
bb3: ; preds = %ehcleanup0
44+
%val.bb3 = call i32 @llvm.wasm.memory.size.i32(i32 0)
45+
call void @longjmpable() [ "funclet"(token %1) ]
46+
; CHECK: bb3:
47+
; CHECK: invoke void @longjmpable
48+
; CHECK-NEXT: to label %.noexc1 unwind label %ehcleanup1
49+
cleanupret from %1 unwind label %ehcleanup1
50+
51+
ehcleanup1: ; preds = %bb3, %ehcleanup0, %entry
52+
%phi = phi i32 [ %val.entry, %entry ], [ %val.bb1, %ehcleanup0 ], [ %val.bb3, %bb3 ]
53+
; CHECK: ehcleanup1:
54+
; CHECK-NEXT: %phi = phi i32 [ %val.entry2, %entry.split.split ], [ %val.bb1, %.noexc ], [ %val.bb3, %.noexc1 ], [ %val.bb1, %ehcleanup0 ], [ %val.bb3, %bb3 ]
55+
%2 = cleanuppad within none []
56+
%3 = call i32 @llvm.wasm.memory.size.i32(i32 %phi)
57+
cleanupret from %2 unwind to caller
58+
}
59+
60+
; When longjmpable calls are within a catchpad.
61+
define void @longjmpable_invoke_phi1() personality ptr @__gxx_wasm_personality_v0 {
62+
; CHECK-LABEL: @longjmpable_invoke_phi1
63+
entry:
64+
%val.entry = call i32 @llvm.wasm.memory.size.i32(i32 0)
65+
%0 = call i32 @setjmp(ptr @buf) #2
66+
invoke void @foo()
67+
to label %bb1 unwind label %ehcleanup
68+
69+
bb1: ; preds = %entry
70+
%val.bb1 = call i32 @llvm.wasm.memory.size.i32(i32 0)
71+
invoke void @foo()
72+
to label %bb2 unwind label %catch.dispatch
73+
74+
bb2: ; preds = %bb1
75+
unreachable
76+
77+
catch.dispatch: ; preds = %bb1
78+
%1 = catchswitch within none [label %catch.start] unwind label %ehcleanup
79+
80+
catch.start: ; preds = %catch.dispatch
81+
%2 = catchpad within %1 [ptr null]
82+
%3 = call ptr @llvm.wasm.get.exception(token %2)
83+
%4 = call i32 @llvm.wasm.get.ehselector(token %2)
84+
call void @longjmpable() [ "funclet"(token %2) ]
85+
; CHECK: catch.start:
86+
; CHECK: invoke void @longjmpable
87+
; CHECK-NEXT: to label %.noexc unwind label %ehcleanup
88+
invoke void @foo() [ "funclet"(token %2) ]
89+
to label %bb3 unwind label %ehcleanup
90+
91+
bb3: ; preds = %catch.start
92+
%val.bb3 = call i32 @llvm.wasm.memory.size.i32(i32 0)
93+
call void @longjmpable() [ "funclet"(token %2) ]
94+
; CHECK: bb3:
95+
; CHECK: invoke void @longjmpable
96+
; CHECK-NEXT: to label %.noexc1 unwind label %ehcleanup
97+
invoke void @foo() [ "funclet"(token %2) ]
98+
to label %bb4 unwind label %ehcleanup
99+
100+
bb4: ; preds = %bb3
101+
unreachable
102+
103+
ehcleanup: ; preds = %bb3, %catch.start, %catch.dispatch, %entry
104+
%phi = phi i32 [ %val.entry, %entry ], [ %val.bb1, %catch.dispatch ], [ %val.bb1, %catch.start ], [ %val.bb3, %bb3 ]
105+
; CHECK: ehcleanup:
106+
; CHECK-NEXT: %phi = phi i32 [ %val.entry2, %entry.split.split ], [ %val.bb1, %catch.dispatch ], [ %val.bb1, %.noexc ], [ %val.bb3, %.noexc1 ], [ %val.bb1, %catch.start ], [ %val.bb3, %bb3 ]
107+
%5 = cleanuppad within none []
108+
%6 = call i32 @llvm.wasm.memory.size.i32(i32 %phi)
109+
cleanupret from %5 unwind to caller
110+
}
111+
112+
declare i32 @setjmp(ptr)
113+
declare i32 @__gxx_wasm_personality_v0(...)
114+
declare void @foo()
115+
declare void @longjmpable()
116+
declare void @use_i32(i32)
117+
; Function Attrs: nocallback nofree nosync nounwind willreturn
118+
declare i32 @llvm.wasm.get.ehselector(token) #0
119+
; Function Attrs: nocallback nofree nosync nounwind willreturn
120+
declare ptr @llvm.wasm.get.exception(token) #0
121+
; Function Attrs: nocallback nofree nosync nounwind willreturn memory(read)
122+
declare i32 @llvm.wasm.memory.size.i32(i32) #1
123+
124+
attributes #0 = { nocallback nofree nosync nounwind willreturn }
125+
attributes #1 = { nocallback nofree nosync nounwind willreturn memory(read) }
126+
attributes #2 = { returns_twice }

0 commit comments

Comments
 (0)