From 71b0a4e36b43e406d6af7fd71b2b23da6041cfda Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Sat, 2 Nov 2024 08:18:31 +0000 Subject: [PATCH 1/6] [WebAssembly] Fix rethrow's index calculation So far we have assumed that we only rethrow the exception caught in the innermost EH pad. This is true in code we directly generate, but after inlining this may not be the case. For example, consider this code: ```ll ehcleanup: %0 = cleanuppad ... call @destructor cleanupret from %0 unwind label %catch.dispatch ``` If `destructor` gets inlined into this function, the code can be like ```ll ehcleanup: %0 = cleanuppad ... invoke @throwing_func to label %unreachale unwind label %catch.dispatch.i catch.dispatch.i: catchswitch ... [ label %catch.start.i ] catch.start.i: %1 = catchpad ... invoke @some_function to label %invoke.cont.i unwind label %terminate.i invoke.cont.i: catchret from %1 to label %destructor.exit destructor.exit: cleanupret from %0 unwind label %catch.dispatch ``` We lower a `cleanupret` into `rethrow`, which assumes it rethrows the exception caught by the nearest dominating EH pad. But after the inlining, the nearest dominating EH pad is not `ehcleanup` but `catch.start.i`. The problem exists in the same manner in the new (exnref) EH, because it assumes the exception comes from the nearest EH pad and saves an exnref from that EH pad and rethrows it (using `throw_ref`). This problem can be fixed easily if `cleanupret` has the basic block where its matching `cleanuppad` is. The bitcode instruction `cleanupret` kind of has that info (it has a token from the `cleanuppad`), but that info is lost when when we enter ISel, because `TargetSelectionDAG.td`'s `cleanupret` node does not have any arguments: https://github.com/llvm/llvm-project/blob/5091a359d9807db8f7d62375696f93fc34226969/llvm/include/llvm/Target/TargetSelectionDAG.td#L700 Note that `catchret` already has two basic block arguments, even though neither of them means `catchpad`'s BB. This PR adds the `cleanuppad`'s BB as an argument to `cleanupret` node in ISel and uses it in the Wasm backend. Because this node is also used in X86 backend we need to note its argument there too but nothing more needs to change there as long as X86 doesn't need it. --- - Details about changes in the Wasm backend: After this PR, our pseudo `RETHROW` instruction takes a BB, which means the EH pad whose exception it needs to rethrow. There are currently two ways to generate a `RETHROW`: one is from `llvm.wasm.rethrow` intrinsic and the other is from `CLEANUPRET` we discussed above. In case of `llvm.wasm.rethrow`, we add a '0' as a placeholder argument when it is lowered to a `RETHROW`, and change it to a BB in LateEHPrepare. As written in the comments, this PR doesn't change how this BB is computed. The BB argument will be converted to an immediate argument as with other control flow instructions in CFGStackify. In case of `CLEANUPRET`, it already has a BB argument pointing to an EH pad, so it is just converted to a `RETHROW` with the same BB argument in LateEHPrepare. This will also be lowered to an immediate in CFGStackify with other control flow instructions. --- Fixes #114600. --- .../include/llvm/Target/TargetSelectionDAG.td | 6 +- .../SelectionDAG/SelectionDAGBuilder.cpp | 6 +- .../WebAssembly/WebAssemblyCFGStackify.cpp | 48 ++------ .../WebAssembly/WebAssemblyISelDAGToDAG.cpp | 13 ++ .../WebAssembly/WebAssemblyInstrControl.td | 9 +- .../WebAssembly/WebAssemblyLateEHPrepare.cpp | 55 ++++++--- llvm/lib/Target/X86/X86InstrCompiler.td | 3 +- .../WebAssembly/cfg-stackify-eh-legacy.mir | 11 +- .../CodeGen/WebAssembly/exception-legacy.ll | 108 ++++++++++++++++ .../CodeGen/WebAssembly/exception-legacy.mir | 2 +- llvm/test/CodeGen/WebAssembly/exception.ll | 115 ++++++++++++++++++ 11 files changed, 304 insertions(+), 72 deletions(-) diff --git a/llvm/include/llvm/Target/TargetSelectionDAG.td b/llvm/include/llvm/Target/TargetSelectionDAG.td index fa516fc9b10175..e4485d997c34cc 100644 --- a/llvm/include/llvm/Target/TargetSelectionDAG.td +++ b/llvm/include/llvm/Target/TargetSelectionDAG.td @@ -234,6 +234,10 @@ def SDTCatchret : SDTypeProfile<0, 2, [ // catchret SDTCisVT<0, OtherVT>, SDTCisVT<1, OtherVT> ]>; +def SDTCleanupret : SDTypeProfile<0, 1, [ // cleanupret + SDTCisVT<0, OtherVT> +]>; + def SDTNone : SDTypeProfile<0, 0, []>; // ret, trap def SDTUBSANTrap : SDTypeProfile<0, 1, []>; // ubsantrap @@ -697,7 +701,7 @@ def brind : SDNode<"ISD::BRIND" , SDTBrind, [SDNPHasChain]>; def br : SDNode<"ISD::BR" , SDTBr, [SDNPHasChain]>; def catchret : SDNode<"ISD::CATCHRET" , SDTCatchret, [SDNPHasChain, SDNPSideEffect]>; -def cleanupret : SDNode<"ISD::CLEANUPRET" , SDTNone, [SDNPHasChain]>; +def cleanupret : SDNode<"ISD::CLEANUPRET" , SDTCleanupret, [SDNPHasChain]>; def trap : SDNode<"ISD::TRAP" , SDTNone, [SDNPHasChain, SDNPSideEffect]>; diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 8450553743074c..14351e98c567d7 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -2154,8 +2154,10 @@ void SelectionDAGBuilder::visitCleanupRet(const CleanupReturnInst &I) { FuncInfo.MBB->normalizeSuccProbs(); // Create the terminator node. - SDValue Ret = - DAG.getNode(ISD::CLEANUPRET, getCurSDLoc(), MVT::Other, getControlRoot()); + MachineBasicBlock *CleanupPadMBB = + FuncInfo.getMBB(I.getCleanupPad()->getParent()); + SDValue Ret = DAG.getNode(ISD::CLEANUPRET, getCurSDLoc(), MVT::Other, + getControlRoot(), DAG.getBasicBlock(CleanupPadMBB)); DAG.setRoot(Ret); } diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp index a5f73fabca3542..59a47f9f4626bf 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp @@ -93,9 +93,8 @@ class WebAssemblyCFGStackify final : public MachineFunctionPass { const MachineBasicBlock *MBB); unsigned getDelegateDepth(const SmallVectorImpl &Stack, const MachineBasicBlock *MBB); - unsigned - getRethrowDepth(const SmallVectorImpl &Stack, - const SmallVectorImpl &EHPadStack); + unsigned getRethrowDepth(const SmallVectorImpl &Stack, + const MachineBasicBlock *EHPadToRethrow); void rewriteDepthImmediates(MachineFunction &MF); void fixEndsAtEndOfFunction(MachineFunction &MF); void cleanupFunctionData(MachineFunction &MF); @@ -1875,34 +1874,13 @@ unsigned WebAssemblyCFGStackify::getDelegateDepth( unsigned WebAssemblyCFGStackify::getRethrowDepth( const SmallVectorImpl &Stack, - const SmallVectorImpl &EHPadStack) { + const MachineBasicBlock *EHPadToRethrow) { unsigned Depth = 0; - // In our current implementation, rethrows always rethrow the exception caught - // by the innermost enclosing catch. This means while traversing Stack in the - // reverse direction, when we encounter END_TRY, we should check if the - // END_TRY corresponds to the current innermost EH pad. For example: - // try - // ... - // catch ;; (a) - // try - // rethrow 1 ;; (b) - // catch ;; (c) - // rethrow 0 ;; (d) - // end ;; (e) - // end ;; (f) - // - // When we are at 'rethrow' (d), while reversely traversing Stack the first - // 'end' we encounter is the 'end' (e), which corresponds to the 'catch' (c). - // And 'rethrow' (d) rethrows the exception caught by 'catch' (c), so we stop - // there and the depth should be 0. But when we are at 'rethrow' (b), it - // rethrows the exception caught by 'catch' (a), so when traversing Stack - // reversely, we should skip the 'end' (e) and choose 'end' (f), which - // corresponds to 'catch' (a). for (auto X : reverse(Stack)) { const MachineInstr *End = X.second; if (End->getOpcode() == WebAssembly::END_TRY) { auto *EHPad = TryToEHPad[EndToBegin[End]]; - if (EHPadStack.back() == EHPad) + if (EHPadToRethrow == EHPad) break; } ++Depth; @@ -1914,7 +1892,6 @@ unsigned WebAssemblyCFGStackify::getRethrowDepth( void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) { // Now rewrite references to basic blocks to be depth immediates. SmallVector Stack; - SmallVector EHPadStack; auto RewriteOperands = [&](MachineInstr &MI) { // Rewrite MBB operands to be depth immediates. @@ -1925,6 +1902,8 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) { if (MO.isMBB()) { if (MI.getOpcode() == WebAssembly::DELEGATE) MO = MachineOperand::CreateImm(getDelegateDepth(Stack, MO.getMBB())); + else if (MI.getOpcode() == WebAssembly::RETHROW) + MO = MachineOperand::CreateImm(getRethrowDepth(Stack, MO.getMBB())); else MO = MachineOperand::CreateImm(getBranchDepth(Stack, MO.getMBB())); } @@ -1951,12 +1930,8 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) { Stack.pop_back(); break; - case WebAssembly::END_TRY: { - auto *EHPad = TryToEHPad[EndToBegin[&MI]]; - EHPadStack.push_back(EHPad); - [[fallthrough]]; - } case WebAssembly::END_BLOCK: + case WebAssembly::END_TRY: case WebAssembly::END_TRY_TABLE: Stack.push_back(std::make_pair(&MBB, &MI)); break; @@ -1965,15 +1940,6 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) { Stack.push_back(std::make_pair(EndToBegin[&MI]->getParent(), &MI)); break; - case WebAssembly::CATCH_LEGACY: - case WebAssembly::CATCH_ALL_LEGACY: - EHPadStack.pop_back(); - break; - - case WebAssembly::RETHROW: - MI.getOperand(0).setImm(getRethrowDepth(Stack, EHPadStack)); - break; - case WebAssembly::DELEGATE: RewriteOperands(MI); Stack.push_back(std::make_pair(&MBB, &MI)); diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp index b5b9cbeacfa18f..630fcfe879196c 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp @@ -248,6 +248,19 @@ void WebAssemblyDAGToDAGISel::Select(SDNode *Node) { ReplaceNode(Node, Throw); return; } + case Intrinsic::wasm_rethrow: { + // RETHROW's BB argument will be populated in LateEHPrepare. Just use a + // '0' as a placeholder for now. + MachineSDNode *Rethrow = CurDAG->getMachineNode( + WebAssembly::RETHROW, DL, + MVT::Other, // outchain type + { + CurDAG->getConstant(0, DL, MVT::i32), // placeholder + Node->getOperand(0) // inchain + }); + ReplaceNode(Node, Rethrow); + return; + } } break; } diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td b/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td index 97ff6d77f54b15..3fac140f5606b8 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td +++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td @@ -168,7 +168,8 @@ defm CATCH_ALL_REF : I<(outs EXNREF:$dst), (ins), (outs), (ins), []>; // Pseudo instructions: cleanupret / catchret let isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1, isPseudo = 1, isEHScopeReturn = 1 in { - defm CLEANUPRET : NRI<(outs), (ins), [(cleanupret)], "cleanupret", 0>; + defm CLEANUPRET : NRI<(outs), (ins bb_op:$ehpad), [(cleanupret bb:$ehpad)], + "cleanupret", 0>; defm CATCHRET : NRI<(outs), (ins bb_op:$dst, bb_op:$from), [(catchret bb:$dst, bb:$from)], "catchret", 0>; } // isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1, @@ -180,11 +181,9 @@ let isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1, // Rethrowing an exception: rethrow // The new exnref proposal also uses this instruction as an interim pseudo // instruction before we convert it to a THROW_REF. +// $ehpad is the EH pad where the exception the rethrow has been caught. let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in -defm RETHROW : NRI<(outs), (ins i32imm:$depth), [], "rethrow \t$depth", 0x09>; -// The depth argument will be computed in CFGStackify. We set it to 0 here for -// now. -def : Pat<(int_wasm_rethrow), (RETHROW 0)>; +defm RETHROW : NRI<(outs), (ins bb_op:$ehpad), [], "rethrow \t$ehpad", 0x09>; // Region within which an exception is caught: try / end_try let Uses = [VALUE_STACK], Defs = [VALUE_STACK] in { diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp index 70b406b6552bf0..f5eec0e3337cbb 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp @@ -251,15 +251,40 @@ bool WebAssemblyLateEHPrepare::replaceFuncletReturns(MachineFunction &MF) { Changed = true; break; } + case WebAssembly::RETHROW: + // These RETHROWs here were lowered from llvm.wasm.rethrow() intrinsics, + // generated in Clang for when an exception is not caught by the given + // type (e.g. catch (int)). + // + // RETHROW's BB argument is the EH pad the exception to rethrow has been + // caught. (Until this point, RETHROW has just a '0' as a placeholder + // argument.) For these llvm.wasm.rethrow()s, we can safely assume the + // exception comes from the nearest dominating EH pad, because catch.start + // EH pad is structured like this: + // + // catch.start: + // catchpad ... + // %matches = compare ehselector with typeid + // br i1 %matches, label %catch, label %rethrow + // + // rethrow: + // ;; rethrows the exception caught in 'catch.start' + // call @llvm.wasm.rethrow() + while (TI->getNumOperands() > 0) + TI->removeOperand(TI->getNumOperands() - 1); + TI->addOperand(MachineOperand::CreateMBB(getMatchingEHPad(TI))); + Changed = true; + break; case WebAssembly::CLEANUPRET: { - // Replace a cleanupret with a rethrow. For C++ support, currently - // rethrow's immediate argument is always 0 (= the latest exception). + // CLEANUPRETs have the EH pad BB the exception to rethrow has been caught + // as an argument. Use it and change the instruction opcode to 'RETHROW' + // to make rethrowing instructions consistent. // - // Even when -wasm-enable-exnref is true, we use a RETHROW here for the - // moment. This will be converted to a THROW_REF in - // addCatchRefsAndThrowRefs. + // This is because we cannot safely assume that it is always the nearest + // dominating EH pad, in case there are code transformations such as + // inlining. BuildMI(MBB, TI, TI->getDebugLoc(), TII.get(WebAssembly::RETHROW)) - .addImm(0); + .addMBB(TI->getOperand(0).getMBB()); TI->eraseFromParent(); Changed = true; break; @@ -272,21 +297,17 @@ bool WebAssemblyLateEHPrepare::replaceFuncletReturns(MachineFunction &MF) { // Add CATCH_REF and CATCH_ALL_REF pseudo instructions to EH pads, and convert // RETHROWs to THROW_REFs. bool WebAssemblyLateEHPrepare::addCatchRefsAndThrowRefs(MachineFunction &MF) { - bool Changed = false; const auto &TII = *MF.getSubtarget().getInstrInfo(); auto &MRI = MF.getRegInfo(); DenseMap> EHPadToRethrows; // Create a map of - for (auto &MBB : MF) { - for (auto &MI : MBB) { - if (MI.getOpcode() == WebAssembly::RETHROW) { - Changed = true; - auto *EHPad = getMatchingEHPad(&MI); - EHPadToRethrows[EHPad].push_back(&MI); - } - } - } + for (auto &MBB : MF) + for (auto &MI : MBB) + if (MI.getOpcode() == WebAssembly::RETHROW) + EHPadToRethrows[MI.getOperand(0).getMBB()].push_back(&MI); + if (EHPadToRethrows.empty()) + return false; // Convert CATCH into CATCH_REF and CATCH_ALL into CATCH_ALL_REF, when the // caught exception is rethrown. And convert RETHROWs to THROW_REFs. @@ -325,7 +346,7 @@ bool WebAssemblyLateEHPrepare::addCatchRefsAndThrowRefs(MachineFunction &MF) { } } - return Changed; + return true; } // Remove unnecessary unreachables after a throw/rethrow/throw_ref. diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td index a05c3f028442c0..880061d82036d5 100644 --- a/llvm/lib/Target/X86/X86InstrCompiler.td +++ b/llvm/lib/Target/X86/X86InstrCompiler.td @@ -195,7 +195,8 @@ def EH_RETURN64 : I<0xC3, RawFrm, (outs), (ins GR64:$addr), let isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1, isCodeGenOnly = 1, isReturn = 1, isEHScopeReturn = 1 in { - def CLEANUPRET : I<0, Pseudo, (outs), (ins), "# CLEANUPRET", [(cleanupret)]>; + def CLEANUPRET : I<0, Pseudo, (outs), (ins), "# CLEANUPRET", + [(cleanupret bb)]>; // CATCHRET needs a custom inserter for SEH. let usesCustomInserter = 1 in diff --git a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.mir b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.mir index c37a82fe80826d..2494ad1ad581d5 100644 --- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.mir +++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.mir @@ -40,15 +40,16 @@ body: | ; CHECK: RETHROW 1 EH_LABEL %0:i32 = CATCH_LEGACY &__cpp_exception, implicit-def dead $arguments - RETHROW 0, implicit-def dead $arguments + RETHROW %bb.1, implicit-def dead $arguments bb.2 (landing-pad): + successors: %bb.3 ; CHECK: bb.2 (landing-pad): ; CHECK: CATCH_LEGACY ; CHECK: RETHROW 0 EH_LABEL %1:i32 = CATCH_LEGACY &__cpp_exception, implicit-def dead $arguments - RETHROW 0, implicit-def dead $arguments + RETHROW %bb.2, implicit-def dead $arguments bb.3: ; CHECK: bb.3: @@ -105,12 +106,14 @@ body: | RETURN %0:i32, implicit-def dead $arguments bb.3 (landing-pad): + successors: EH_LABEL %0:i32 = CATCH_LEGACY &__cpp_exception, implicit-def dead $arguments - RETHROW 0, implicit-def dead $arguments + RETHROW %bb.3, implicit-def dead $arguments bb.4 (landing-pad): + successors: EH_LABEL %1:i32 = CATCH_LEGACY &__cpp_exception, implicit-def dead $arguments - RETHROW 0, implicit-def dead $arguments + RETHROW %bb.4, implicit-def dead $arguments ... diff --git a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll index 3327d8be894f0c..3aac61aee340c2 100644 --- a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll +++ b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll @@ -400,6 +400,110 @@ unreachable: ; preds = %rethrow unreachable } +; The bitcode below is generated when the code below is compiled and +; Temp::~Temp() is inlined into inlined_cleanupret(): +; +; void inlined_cleanupret() { +; try { +; Temp t; +; throw 2; +; } catch (...) +; } +; +; Temp::~Temp() { +; try { +; throw 1; +; } catch (...) { +; } +; } +; +; ~Temp() generates cleanupret, which is lowered to a 'rethrow' later. That +; rethrow's immediate argument should correctly target the top-level cleanuppad +; (catch_all). This is a regression test for the bug where we did not compute +; rethrow's argument correctly. + +; CHECK-LABEL: inlined_cleanupret: +; CHECK: try +; CHECK: call __cxa_throw +; CHECK: catch_all +; CHECK: try +; CHECK: try +; CHECK: call __cxa_throw +; CHECK: catch +; CHECK: call __cxa_end_catch +; CHECK: try +; CHECK: try +; Note that this rethrow targets the top-level catch_all +; CHECK: rethrow 4 +; CHECK: catch +; CHECK: try +; CHECK: call __cxa_end_catch +; CHECK: delegate 5 +; CHECK: return +; CHECK: end_try +; CHECK: delegate 3 +; CHECK: end_try +; CHECK: unreachable +; CHECK: catch_all +; CHECK: call _ZSt9terminatev +; CHECK: unreachable +; CHECK: end_try +; CHECK: end_try +; CHECK: unreachable +define void @inlined_cleanupret() personality ptr @__gxx_wasm_personality_v0 { +entry: + %exception = tail call ptr @__cxa_allocate_exception(i32 4) + store i32 2, ptr %exception, align 16 + invoke void @__cxa_throw(ptr nonnull %exception, ptr nonnull @_ZTIi, ptr null) + to label %unreachable unwind label %ehcleanup + +ehcleanup: ; preds = %entry + %0 = cleanuppad within none [] + %exception.i = call ptr @__cxa_allocate_exception(i32 4) [ "funclet"(token %0) ] + store i32 1, ptr %exception.i, align 16 + invoke void @__cxa_throw(ptr nonnull %exception.i, ptr nonnull @_ZTIi, ptr null) [ "funclet"(token %0) ] + to label %unreachable unwind label %catch.dispatch.i + +catch.dispatch.i: ; preds = %ehcleanup + %1 = catchswitch within %0 [label %catch.start.i] unwind label %terminate.i + +catch.start.i: ; preds = %catch.dispatch.i + %2 = catchpad within %1 [ptr null] + %3 = tail call ptr @llvm.wasm.get.exception(token %2) + %4 = tail call i32 @llvm.wasm.get.ehselector(token %2) + %5 = call ptr @__cxa_begin_catch(ptr %3) [ "funclet"(token %2) ] + invoke void @__cxa_end_catch() [ "funclet"(token %2) ] + to label %invoke.cont.i unwind label %terminate.i + +invoke.cont.i: ; preds = %catch.start.i + catchret from %2 to label %_ZN4TempD2Ev.exit + +terminate.i: ; preds = %catch.start.i, %catch.dispatch.i + %6 = cleanuppad within %0 [] + call void @_ZSt9terminatev() [ "funclet"(token %6) ] + unreachable + +_ZN4TempD2Ev.exit: ; preds = %invoke.cont.i + cleanupret from %0 unwind label %catch.dispatch + +catch.dispatch: ; preds = %_ZN4TempD2Ev.exit + %7 = catchswitch within none [label %catch.start] unwind to caller + +catch.start: ; preds = %catch.dispatch + %8 = catchpad within %7 [ptr null] + %9 = tail call ptr @llvm.wasm.get.exception(token %8) + %10 = tail call i32 @llvm.wasm.get.ehselector(token %8) + %11 = call ptr @__cxa_begin_catch(ptr %9) #8 [ "funclet"(token %8) ] + call void @__cxa_end_catch() [ "funclet"(token %8) ] + catchret from %8 to label %try.cont + +try.cont: ; preds = %catch.start + ret void + +unreachable: ; preds = %entry + unreachable +} + declare void @foo() declare void @bar(ptr) @@ -415,8 +519,12 @@ declare i32 @llvm.wasm.get.ehselector(token) #0 declare void @llvm.wasm.rethrow() #1 ; Function Attrs: nounwind declare i32 @llvm.eh.typeid.for(ptr) #0 +; Function Attrs: nounwind +declare ptr @__cxa_allocate_exception(i32) #0 declare ptr @__cxa_begin_catch(ptr) declare void @__cxa_end_catch() +; Function Attrs: noreturn +declare void @__cxa_throw(ptr, ptr, ptr) #1 declare void @_ZSt9terminatev() declare ptr @_ZN4TempD2Ev(ptr returned) diff --git a/llvm/test/CodeGen/WebAssembly/exception-legacy.mir b/llvm/test/CodeGen/WebAssembly/exception-legacy.mir index f9eb40bb03cc7a..fbed7db1dcb110 100644 --- a/llvm/test/CodeGen/WebAssembly/exception-legacy.mir +++ b/llvm/test/CodeGen/WebAssembly/exception-legacy.mir @@ -105,7 +105,7 @@ body: | bb.2: successors: %bb.3 - CLEANUPRET implicit-def dead $arguments + CLEANUPRET %bb.1, implicit-def dead $arguments bb.3: RETURN implicit-def dead $arguments diff --git a/llvm/test/CodeGen/WebAssembly/exception.ll b/llvm/test/CodeGen/WebAssembly/exception.ll index 1ad4c84f1c0237..5b684ab71643b4 100644 --- a/llvm/test/CodeGen/WebAssembly/exception.ll +++ b/llvm/test/CodeGen/WebAssembly/exception.ll @@ -445,6 +445,117 @@ unreachable: ; preds = %rethrow unreachable } +; The bitcode below is generated when the code below is compiled and +; Temp::~Temp() is inlined into inlined_cleanupret(): +; +; void inlined_cleanupret() { +; try { +; Temp t; +; throw 2; +; } catch (...) +; } +; +; Temp::~Temp() { +; try { +; throw 1; +; } catch (...) { +; } +; } +; +; ~Temp() generates cleanupret, which is lowered to a 'throw_ref' later. That +; throw_ref's argument should correctly target the top-level cleanuppad +; (catch_all_ref). This is a regression test for the bug where we did not +; compute throw_ref's argument correctly. + +; CHECK-LABEL: inlined_cleanupret: +; CHECK: block +; CHECK: block exnref +; CHECK: try_table (catch_all_ref 0) +; CHECK: call __cxa_throw +; CHECK: end_try_table +; CHECK: end_block +; try_table (catch_all_ref 0)'s caught exception is stored in local 2 +; CHECK: local.set 2 +; CHECK: block +; CHECK: try_table (catch_all 0) +; CHECK: block +; CHECK: block i32 +; CHECK: try_table (catch __cpp_exception 0) +; CHECK: call __cxa_throw +; CHECK: end_try_table +; CHECK: end_block +; CHECK: call __cxa_end_catch +; CHECK: block i32 +; CHECK: try_table (catch __cpp_exception 0) +; Note that the throw_ref below targets the top-level catch_all_ref (local 2) +; CHECK: local.get 2 +; CHECK: throw_ref +; CHECK: end_try_table +; CHECK: end_block +; CHECK: call __cxa_end_catch +; CHECK: return +; CHECK: end_block +; CHECK: unreachable +; CHECK: end_try_table +; CHECK: end_block +; CHECK: call _ZSt9terminatev +; CHECK: unreachable +; CHECK: end_block +; CHECK: unreachable +define void @inlined_cleanupret() personality ptr @__gxx_wasm_personality_v0 { +entry: + %exception = tail call ptr @__cxa_allocate_exception(i32 4) + store i32 2, ptr %exception, align 16 + invoke void @__cxa_throw(ptr nonnull %exception, ptr nonnull @_ZTIi, ptr null) + to label %unreachable unwind label %ehcleanup + +ehcleanup: ; preds = %entry + %0 = cleanuppad within none [] + %exception.i = call ptr @__cxa_allocate_exception(i32 4) [ "funclet"(token %0) ] + store i32 1, ptr %exception.i, align 16 + invoke void @__cxa_throw(ptr nonnull %exception.i, ptr nonnull @_ZTIi, ptr null) [ "funclet"(token %0) ] + to label %unreachable unwind label %catch.dispatch.i + +catch.dispatch.i: ; preds = %ehcleanup + %1 = catchswitch within %0 [label %catch.start.i] unwind label %terminate.i + +catch.start.i: ; preds = %catch.dispatch.i + %2 = catchpad within %1 [ptr null] + %3 = tail call ptr @llvm.wasm.get.exception(token %2) + %4 = tail call i32 @llvm.wasm.get.ehselector(token %2) + %5 = call ptr @__cxa_begin_catch(ptr %3) [ "funclet"(token %2) ] + invoke void @__cxa_end_catch() [ "funclet"(token %2) ] + to label %invoke.cont.i unwind label %terminate.i + +invoke.cont.i: ; preds = %catch.start.i + catchret from %2 to label %_ZN4TempD2Ev.exit + +terminate.i: ; preds = %catch.start.i, %catch.dispatch.i + %6 = cleanuppad within %0 [] + call void @_ZSt9terminatev() [ "funclet"(token %6) ] + unreachable + +_ZN4TempD2Ev.exit: ; preds = %invoke.cont.i + cleanupret from %0 unwind label %catch.dispatch + +catch.dispatch: ; preds = %_ZN4TempD2Ev.exit + %7 = catchswitch within none [label %catch.start] unwind to caller + +catch.start: ; preds = %catch.dispatch + %8 = catchpad within %7 [ptr null] + %9 = tail call ptr @llvm.wasm.get.exception(token %8) + %10 = tail call i32 @llvm.wasm.get.ehselector(token %8) + %11 = call ptr @__cxa_begin_catch(ptr %9) #8 [ "funclet"(token %8) ] + call void @__cxa_end_catch() [ "funclet"(token %8) ] + catchret from %8 to label %try.cont + +try.cont: ; preds = %catch.start + ret void + +unreachable: ; preds = %entry + unreachable +} + declare void @foo() declare void @bar(ptr) @@ -460,8 +571,12 @@ declare i32 @llvm.wasm.get.ehselector(token) #0 declare void @llvm.wasm.rethrow() #1 ; Function Attrs: nounwind declare i32 @llvm.eh.typeid.for(ptr) #0 +; Function Attrs: nounwind +declare ptr @__cxa_allocate_exception(i32) #0 declare ptr @__cxa_begin_catch(ptr) declare void @__cxa_end_catch() +; Function Attrs: noreturn +declare void @__cxa_throw(ptr, ptr, ptr) #1 declare void @_ZSt9terminatev() declare ptr @_ZN4TempD2Ev(ptr returned) From 82ab656575161c8423de363bd2e195294d5a109d Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Tue, 5 Nov 2024 00:27:43 +0000 Subject: [PATCH 2/6] Add 'bb' argument to AArch64 too --- llvm/lib/Target/AArch64/AArch64InstrInfo.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td index 424848252f6aa6..5f1805ca3aa7c4 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td @@ -5372,7 +5372,7 @@ let isPseudo = 1 in { //===----------------------------------------------------------------------===// let isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1, isCodeGenOnly = 1, isReturn = 1, isEHScopeReturn = 1, isPseudo = 1 in { - def CLEANUPRET : Pseudo<(outs), (ins), [(cleanupret)]>, Sched<[]>; + def CLEANUPRET : Pseudo<(outs), (ins), [(cleanupret bb)]>, Sched<[]>; let usesCustomInserter = 1 in def CATCHRET : Pseudo<(outs), (ins am_brcond:$dst, am_brcond:$src), [(catchret bb:$dst, bb:$src)]>, Sched<[]>; From 275b383cf82e622004e56c8230777fa6a3c77619 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Mon, 4 Nov 2024 17:39:47 -0800 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Derek Schuff --- llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td | 2 +- llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td b/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td index 3fac140f5606b8..b68dd8809bb920 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td +++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td @@ -181,7 +181,7 @@ let isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1, // Rethrowing an exception: rethrow // The new exnref proposal also uses this instruction as an interim pseudo // instruction before we convert it to a THROW_REF. -// $ehpad is the EH pad where the exception the rethrow has been caught. +// $ehpad is the EH pad where the exception to rethrow has been caught. let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in defm RETHROW : NRI<(outs), (ins bb_op:$ehpad), [], "rethrow \t$ehpad", 0x09>; diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp index f5eec0e3337cbb..0f133ab1edb91b 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp @@ -256,7 +256,7 @@ bool WebAssemblyLateEHPrepare::replaceFuncletReturns(MachineFunction &MF) { // generated in Clang for when an exception is not caught by the given // type (e.g. catch (int)). // - // RETHROW's BB argument is the EH pad the exception to rethrow has been + // RETHROW's BB argument is the EH pad where the exception to rethrow has been // caught. (Until this point, RETHROW has just a '0' as a placeholder // argument.) For these llvm.wasm.rethrow()s, we can safely assume the // exception comes from the nearest dominating EH pad, because catch.start From 80cd16ee53b83ea4d16d97b2ebb5dca864c80e6c Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Tue, 5 Nov 2024 01:40:38 +0000 Subject: [PATCH 4/6] 80-col wrapping --- llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp index 0f133ab1edb91b..2cbe0a715c71e0 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp @@ -256,8 +256,8 @@ bool WebAssemblyLateEHPrepare::replaceFuncletReturns(MachineFunction &MF) { // generated in Clang for when an exception is not caught by the given // type (e.g. catch (int)). // - // RETHROW's BB argument is the EH pad where the exception to rethrow has been - // caught. (Until this point, RETHROW has just a '0' as a placeholder + // RETHROW's BB argument is the EH pad where the exception to rethrow has + // been caught. (Until this point, RETHROW has just a '0' as a placeholder // argument.) For these llvm.wasm.rethrow()s, we can safely assume the // exception comes from the nearest dominating EH pad, because catch.start // EH pad is structured like this: From 2a182bacaac16dfdfb68508490704a690ace1e82 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Tue, 5 Nov 2024 04:39:59 +0000 Subject: [PATCH 5/6] RemoveOperand(0) --- llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp index 2cbe0a715c71e0..e9b2df11b49157 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp @@ -270,8 +270,7 @@ bool WebAssemblyLateEHPrepare::replaceFuncletReturns(MachineFunction &MF) { // rethrow: // ;; rethrows the exception caught in 'catch.start' // call @llvm.wasm.rethrow() - while (TI->getNumOperands() > 0) - TI->removeOperand(TI->getNumOperands() - 1); + TI->removeOperand(0); TI->addOperand(MachineOperand::CreateMBB(getMatchingEHPad(TI))); Changed = true; break; From 16df64b90b6214cf2214fd764f1fe75420cec6b2 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Tue, 5 Nov 2024 19:38:42 +0000 Subject: [PATCH 6/6] Update test expectation after #114361 --- .../CodeGen/WebAssembly/exception-legacy.ll | 3 - llvm/test/CodeGen/WebAssembly/exception.ll | 60 ++++++++++--------- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll index 3aac61aee340c2..9ebdc06d9666d3 100644 --- a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll +++ b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll @@ -443,13 +443,10 @@ unreachable: ; preds = %rethrow ; CHECK: end_try ; CHECK: delegate 3 ; CHECK: end_try -; CHECK: unreachable ; CHECK: catch_all ; CHECK: call _ZSt9terminatev -; CHECK: unreachable ; CHECK: end_try ; CHECK: end_try -; CHECK: unreachable define void @inlined_cleanupret() personality ptr @__gxx_wasm_personality_v0 { entry: %exception = tail call ptr @__cxa_allocate_exception(i32 4) diff --git a/llvm/test/CodeGen/WebAssembly/exception.ll b/llvm/test/CodeGen/WebAssembly/exception.ll index 5b684ab71643b4..779775b676071a 100644 --- a/llvm/test/CodeGen/WebAssembly/exception.ll +++ b/llvm/test/CodeGen/WebAssembly/exception.ll @@ -468,40 +468,44 @@ unreachable: ; preds = %rethrow ; compute throw_ref's argument correctly. ; CHECK-LABEL: inlined_cleanupret: -; CHECK: block -; CHECK: block exnref -; CHECK: try_table (catch_all_ref 0) -; CHECK: call __cxa_throw -; CHECK: end_try_table -; CHECK: end_block -; try_table (catch_all_ref 0)'s caught exception is stored in local 2 -; CHECK: local.set 2 +; CHECK: block exnref ; CHECK: block -; CHECK: try_table (catch_all 0) -; CHECK: block -; CHECK: block i32 -; CHECK: try_table (catch __cpp_exception 0) -; CHECK: call __cxa_throw -; CHECK: end_try_table -; CHECK: end_block -; CHECK: call __cxa_end_catch -; CHECK: block i32 -; CHECK: try_table (catch __cpp_exception 0) +; CHECK: block exnref +; CHECK: try_table (catch_all_ref 0) +; CHECK: call __cxa_throw +; CHECK: end_try_table +; CHECK: end_block +; try_table (catch_all_ref 0)'s caught exception is stored in local 2 +; CHECK: local.set 2 +; CHECK: block +; CHECK: try_table (catch_all 0) +; CHECK: block +; CHECK: block i32 +; CHECK: try_table (catch __cpp_exception 0) +; CHECK: call __cxa_throw +; CHECK: end_try_table +; CHECK: end_block +; CHECK: call __cxa_end_catch +; CHECK: block i32 +; CHECK: try_table (catch_all_ref 5) +; CHECK: try_table (catch __cpp_exception 1) ; Note that the throw_ref below targets the top-level catch_all_ref (local 2) -; CHECK: local.get 2 -; CHECK: throw_ref +; CHECK: local.get 2 +; CHECK: throw_ref +; CHECK: end_try_table +; CHECK: end_try_table +; CHECK: end_block +; CHECK: try_table (catch_all_ref 4) +; CHECK: call __cxa_end_catch ; CHECK: end_try_table +; CHECK: return ; CHECK: end_block -; CHECK: call __cxa_end_catch -; CHECK: return -; CHECK: end_block -; CHECK: unreachable -; CHECK: end_try_table +; CHECK: end_try_table +; CHECK: end_block +; CHECK: call _ZSt9terminatev ; CHECK: end_block -; CHECK: call _ZSt9terminatev -; CHECK: unreachable ; CHECK: end_block -; CHECK: unreachable +; CHECK: throw_ref define void @inlined_cleanupret() personality ptr @__gxx_wasm_personality_v0 { entry: %exception = tail call ptr @__cxa_allocate_exception(i32 4)