[llvm] 0af7542 - Reapply "[WebAssembly] Fix phi handling for Wasm SjLj (#99730)"
Heejin Ahn via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 24 17:01:23 PDT 2024
Author: Heejin Ahn
Date: 2024-07-25T00:00:59Z
New Revision: 0af754213507972a0d0301bc195d65414d8dc193
URL: https://github.com/llvm/llvm-project/commit/0af754213507972a0d0301bc195d65414d8dc193
DIFF: https://github.com/llvm/llvm-project/commit/0af754213507972a0d0301bc195d65414d8dc193.diff
LOG: 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
https://github.com/llvm/llvm-project/commit/f3f0d9928f982cfd302351f418bcc5b63cc1bb9d.
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
```
Added:
llvm/test/CodeGen/WebAssembly/lower-wasm-ehsjlj-phi.ll
Modified:
llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
index 7cc030460e30f..17bec8e2a6a45 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
@@ -777,6 +777,8 @@ void WebAssemblyLowerEmscriptenEHSjLj::rebuildSSA(Function &F) {
SSAUpdaterBulk SSA;
for (BasicBlock &BB : F) {
for (Instruction &I : BB) {
+ if (I.getType()->isVoidTy())
+ continue;
unsigned VarID = SSA.AddVariable(I.getName(), I.getType());
// If a value is defined by an invoke instruction, it is only available in
// its normal destination and not in its unwind destination.
@@ -1687,6 +1689,8 @@ void WebAssemblyLowerEmscriptenEHSjLj::handleLongjmpableCallsForWasmSjLj(
}
}
+ SmallDenseMap<BasicBlock *, SmallSetVector<BasicBlock *, 4>, 4>
+ UnwindDestToNewPreds;
for (auto *CI : LongjmpableCalls) {
// Even if the callee function has attribute 'nounwind', which is true for
// all C functions, it can longjmp, which means it can throw a Wasm
@@ -1724,6 +1728,11 @@ void WebAssemblyLowerEmscriptenEHSjLj::handleLongjmpableCallsForWasmSjLj(
}
if (!UnwindDest)
UnwindDest = CatchDispatchLongjmpBB;
+ // Because we are changing a longjmpable call to an invoke, its unwind
+ // destination can be an existing EH pad that already have phis, and the BB
+ // with the newly created invoke will become a new predecessor of that EH
+ // pad. In this case we need to add the new predecessor to those phis.
+ UnwindDestToNewPreds[UnwindDest].insert(CI->getParent());
changeToInvokeAndSplitBasicBlock(CI, UnwindDest);
}
@@ -1752,4 +1761,45 @@ void WebAssemblyLowerEmscriptenEHSjLj::handleLongjmpableCallsForWasmSjLj(
for (Instruction *I : ToErase)
I->eraseFromParent();
+
+ // Add entries for new predecessors to phis in unwind destinations. We use
+ // 'undef' as a placeholder value. We should make sure the phis have a valid
+ // set of predecessors before running SSAUpdater, because SSAUpdater
+ // internally can use existing phis to gather predecessor info rather than
+ // scanning the actual CFG (See FindPredecessorBlocks in SSAUpdater.cpp for
+ // details).
+ for (auto &[UnwindDest, NewPreds] : UnwindDestToNewPreds) {
+ for (PHINode &PN : UnwindDest->phis()) {
+ for (auto *NewPred : NewPreds) {
+ assert(PN.getBasicBlockIndex(NewPred) == -1);
+ PN.addIncoming(UndefValue::get(PN.getType()), NewPred);
+ }
+ }
+ }
+
+ // For unwind destinations for newly added invokes to longjmpable functions,
+ // calculate incoming values for the newly added predecessors using
+ // SSAUpdater. We add existing values in the phis to SSAUpdater as available
+ // values and let it calculate what the value should be at the end of new
+ // incoming blocks.
+ for (auto &[UnwindDest, NewPreds] : UnwindDestToNewPreds) {
+ for (PHINode &PN : UnwindDest->phis()) {
+ SSAUpdater SSA;
+ SSA.Initialize(PN.getType(), PN.getName());
+ for (unsigned Idx = 0, E = PN.getNumIncomingValues(); Idx != E; ++Idx) {
+ if (NewPreds.contains(PN.getIncomingBlock(Idx)))
+ continue;
+ Value *V = PN.getIncomingValue(Idx);
+ if (auto *II = dyn_cast<InvokeInst>(V))
+ SSA.AddAvailableValue(II->getNormalDest(), II);
+ else if (auto *I = dyn_cast<Instruction>(V))
+ SSA.AddAvailableValue(I->getParent(), I);
+ else
+ SSA.AddAvailableValue(PN.getIncomingBlock(Idx), V);
+ }
+ for (auto *NewPred : NewPreds)
+ PN.setIncomingValueForBlock(NewPred, SSA.GetValueAtEndOfBlock(NewPred));
+ assert(PN.isComplete());
+ }
+ }
}
diff --git a/llvm/test/CodeGen/WebAssembly/lower-wasm-ehsjlj-phi.ll b/llvm/test/CodeGen/WebAssembly/lower-wasm-ehsjlj-phi.ll
new file mode 100644
index 0000000000000..97f6d797a63bd
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/lower-wasm-ehsjlj-phi.ll
@@ -0,0 +1,126 @@
+; RUN: opt < %s -wasm-lower-em-ehsjlj -wasm-enable-eh -wasm-enable-sjlj -S | FileCheck %s
+
+target triple = "wasm32-unknown-emscripten"
+
+%struct.__jmp_buf_tag = type { [6 x i32], i32, [32 x i32] }
+ at buf = internal global [1 x %struct.__jmp_buf_tag] zeroinitializer, align 16
+
+; When longjmpable calls are coverted into invokes in Wasm SjLj transformation
+; and their unwind destination is an existing catchpad or cleanuppad due to
+; maintain the scope structure, the new pred BBs created by invokes and the
+; correct incoming values should be added the existing phis in those unwind
+; destinations.
+
+; When longjmpable calls are within a cleanuppad.
+define void @longjmpable_invoke_phi0() personality ptr @__gxx_wasm_personality_v0 {
+; CHECK-LABEL: @longjmpable_invoke_phi0
+entry:
+ %val.entry = call i32 @llvm.wasm.memory.size.i32(i32 0)
+ %0 = call i32 @setjmp(ptr @buf) #2
+ invoke void @foo()
+ to label %bb1 unwind label %ehcleanup1
+
+bb1: ; preds = %entry
+ ; We use llvm.wasm.memory.size intrinsic just to get/use an i32 value. The
+ ; reason we use an intrinsic here is to make it not longjmpable. If this can
+ ; longjmp, the result will be more complicated and hard to check.
+ %val.bb1 = call i32 @llvm.wasm.memory.size.i32(i32 0)
+ invoke void @foo()
+ to label %bb2 unwind label %ehcleanup0
+
+bb2: ; preds = %bb1
+ unreachable
+
+ehcleanup0: ; preds = %bb1
+ %1 = cleanuppad within none []
+ call void @longjmpable() [ "funclet"(token %1) ]
+; CHECK: ehcleanup0
+; CHECK: invoke void @longjmpable
+; CHECK-NEXT: to label %.noexc unwind label %ehcleanup1
+ invoke void @foo() [ "funclet"(token %1) ]
+ to label %bb3 unwind label %ehcleanup1
+
+bb3: ; preds = %ehcleanup0
+ %val.bb3 = call i32 @llvm.wasm.memory.size.i32(i32 0)
+ call void @longjmpable() [ "funclet"(token %1) ]
+; CHECK: bb3:
+; CHECK: invoke void @longjmpable
+; CHECK-NEXT: to label %.noexc1 unwind label %ehcleanup1
+ cleanupret from %1 unwind label %ehcleanup1
+
+ehcleanup1: ; preds = %bb3, %ehcleanup0, %entry
+ %phi = phi i32 [ %val.entry, %entry ], [ %val.bb1, %ehcleanup0 ], [ %val.bb3, %bb3 ]
+; CHECK: ehcleanup1:
+; CHECK-NEXT: %phi = phi i32 [ %val.entry2, %entry.split.split ], [ %val.bb1, %.noexc ], [ %val.bb3, %.noexc1 ], [ %val.bb1, %ehcleanup0 ], [ %val.bb3, %bb3 ]
+ %2 = cleanuppad within none []
+ %3 = call i32 @llvm.wasm.memory.size.i32(i32 %phi)
+ cleanupret from %2 unwind to caller
+}
+
+; When longjmpable calls are within a catchpad.
+define void @longjmpable_invoke_phi1() personality ptr @__gxx_wasm_personality_v0 {
+; CHECK-LABEL: @longjmpable_invoke_phi1
+entry:
+ %val.entry = call i32 @llvm.wasm.memory.size.i32(i32 0)
+ %0 = call i32 @setjmp(ptr @buf) #2
+ invoke void @foo()
+ to label %bb1 unwind label %ehcleanup
+
+bb1: ; preds = %entry
+ %val.bb1 = call i32 @llvm.wasm.memory.size.i32(i32 0)
+ invoke void @foo()
+ to label %bb2 unwind label %catch.dispatch
+
+bb2: ; preds = %bb1
+ unreachable
+
+catch.dispatch: ; preds = %bb1
+ %1 = catchswitch within none [label %catch.start] unwind label %ehcleanup
+
+catch.start: ; preds = %catch.dispatch
+ %2 = catchpad within %1 [ptr null]
+ %3 = call ptr @llvm.wasm.get.exception(token %2)
+ %4 = call i32 @llvm.wasm.get.ehselector(token %2)
+ call void @longjmpable() [ "funclet"(token %2) ]
+; CHECK: catch.start:
+; CHECK: invoke void @longjmpable
+; CHECK-NEXT: to label %.noexc unwind label %ehcleanup
+ invoke void @foo() [ "funclet"(token %2) ]
+ to label %bb3 unwind label %ehcleanup
+
+bb3: ; preds = %catch.start
+ %val.bb3 = call i32 @llvm.wasm.memory.size.i32(i32 0)
+ call void @longjmpable() [ "funclet"(token %2) ]
+; CHECK: bb3:
+; CHECK: invoke void @longjmpable
+; CHECK-NEXT: to label %.noexc1 unwind label %ehcleanup
+ invoke void @foo() [ "funclet"(token %2) ]
+ to label %bb4 unwind label %ehcleanup
+
+bb4: ; preds = %bb3
+ unreachable
+
+ehcleanup: ; preds = %bb3, %catch.start, %catch.dispatch, %entry
+ %phi = phi i32 [ %val.entry, %entry ], [ %val.bb1, %catch.dispatch ], [ %val.bb1, %catch.start ], [ %val.bb3, %bb3 ]
+; CHECK: ehcleanup:
+; 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 ]
+ %5 = cleanuppad within none []
+ %6 = call i32 @llvm.wasm.memory.size.i32(i32 %phi)
+ cleanupret from %5 unwind to caller
+}
+
+declare i32 @setjmp(ptr)
+declare i32 @__gxx_wasm_personality_v0(...)
+declare void @foo()
+declare void @longjmpable()
+declare void @use_i32(i32)
+; Function Attrs: nocallback nofree nosync nounwind willreturn
+declare i32 @llvm.wasm.get.ehselector(token) #0
+; Function Attrs: nocallback nofree nosync nounwind willreturn
+declare ptr @llvm.wasm.get.exception(token) #0
+; Function Attrs: nocallback nofree nosync nounwind willreturn memory(read)
+declare i32 @llvm.wasm.memory.size.i32(i32) #1
+
+attributes #0 = { nocallback nofree nosync nounwind willreturn }
+attributes #1 = { nocallback nofree nosync nounwind willreturn memory(read) }
+attributes #2 = { returns_twice }
More information about the llvm-commits
mailing list