Skip to content

[WebAssembly] Fix phi handling for Wasm SjLj #99730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,8 @@ void WebAssemblyLowerEmscriptenEHSjLj::rebuildSSA(Function &F) {
SSAUpdaterBulk SSA;
for (BasicBlock &BB : F) {
for (Instruction &I : BB) {
if (I.getType()->isVoidTy())
continue;
Comment on lines +780 to +781
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not related to this PR and a NFC drive-by small fix. Because we are adding defs, void type instructions don't have any effects at all when added to SSAUpdaterBulk anyway.

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.
Expand Down Expand Up @@ -1687,6 +1689,8 @@ void WebAssemblyLowerEmscriptenEHSjLj::handleLongjmpableCallsForWasmSjLj(
}
}

SmallDenseMap<BasicBlock *, SmallPtrSet<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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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());
}
}
}
126 changes: 126 additions & 0 deletions llvm/test/CodeGen/WebAssembly/lower-wasm-ehsjlj-phi.ll
Original file line number Diff line number Diff line change
@@ -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] }
@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 }
Loading