release/19.x: [WebAssembly] Fix rethrow's index calculation (#114693) #115844
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
backend:AArch64
backend:WebAssembly
backend:X86
llvm:SelectionDAG
|
@llvm/pr-subscribers-backend-webassembly @llvm/pr-subscribers-backend-x86 Author: Heejin Ahn (aheejin) ChangesSo 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: ehcleanup:
%0 = cleanuppad ...
call @<!-- -->destructor
cleanupret from %0 unwind label %catch.dispatchIf 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.dispatchWe lower a 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 This problem can be fixed easily if catchret already has two basic block arguments, even though neither of them means catchpad's BB.
This PR adds the
After this PR, our pseudo In case of Fixes #114600. Full diff: https://github.com/llvm/llvm-project/pull/115844.diff 11 Files Affected:
diff --git a/llvm/include/llvm/Target/TargetSelectionDAG.td b/llvm/include/llvm/Target/TargetSelectionDAG.td
index 46044aab79a832..e7895258438d26 100644
--- a/llvm/include/llvm/Target/TargetSelectionDAG.td
+++ b/llvm/include/llvm/Target/TargetSelectionDAG.td
@@ -231,6 +231,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
@@ -680,7 +684,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 37b1131d2f8a33..7fa3b8a73a4190 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2155,8 +2155,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.MBBMap[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/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index 1053ba9242768a..95f2f91f82bd41 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -5159,7 +5159,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<[]>;
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
index 70b91c266c4970..cd0aea313da0db 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
@@ -87,9 +87,8 @@ class WebAssemblyCFGStackify final : public MachineFunctionPass {
const MachineBasicBlock *MBB);
unsigned getDelegateDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
const MachineBasicBlock *MBB);
- unsigned
- getRethrowDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
- const SmallVectorImpl<const MachineBasicBlock *> &EHPadStack);
+ unsigned getRethrowDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
+ const MachineBasicBlock *EHPadToRethrow);
void rewriteDepthImmediates(MachineFunction &MF);
void fixEndsAtEndOfFunction(MachineFunction &MF);
void cleanupFunctionData(MachineFunction &MF);
@@ -1612,34 +1611,13 @@ unsigned WebAssemblyCFGStackify::getDelegateDepth(
unsigned WebAssemblyCFGStackify::getRethrowDepth(
const SmallVectorImpl<EndMarkerInfo> &Stack,
- const SmallVectorImpl<const MachineBasicBlock *> &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;
@@ -1651,7 +1629,6 @@ unsigned WebAssemblyCFGStackify::getRethrowDepth(
void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
// Now rewrite references to basic blocks to be depth immediates.
SmallVector<EndMarkerInfo, 8> Stack;
- SmallVector<const MachineBasicBlock *, 8> EHPadStack;
for (auto &MBB : reverse(MF)) {
for (MachineInstr &MI : llvm::reverse(MBB)) {
switch (MI.getOpcode()) {
@@ -1669,31 +1646,14 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
break;
case WebAssembly::END_BLOCK:
+ case WebAssembly::END_TRY:
Stack.push_back(std::make_pair(&MBB, &MI));
break;
- case WebAssembly::END_TRY: {
- // We handle DELEGATE in the default level, because DELEGATE has
- // immediate operands to rewrite.
- Stack.push_back(std::make_pair(&MBB, &MI));
- auto *EHPad = TryToEHPad[EndToBegin[&MI]];
- EHPadStack.push_back(EHPad);
- break;
- }
-
case WebAssembly::END_LOOP:
Stack.push_back(std::make_pair(EndToBegin[&MI]->getParent(), &MI));
break;
- case WebAssembly::CATCH:
- case WebAssembly::CATCH_ALL:
- EHPadStack.pop_back();
- break;
-
- case WebAssembly::RETHROW:
- MI.getOperand(0).setImm(getRethrowDepth(Stack, EHPadStack));
- break;
-
default:
if (MI.isTerminator()) {
// Rewrite MBB operands to be depth immediates.
@@ -1705,6 +1665,9 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
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()));
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
index 0f06f54f219f9a..18545e92886ac5 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
@@ -245,6 +245,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 be6547007aaf7a..261277f8a02cf8 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
@@ -132,11 +132,9 @@ let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in {
defm THROW : I<(outs), (ins tag_op:$tag, variable_ops),
(outs), (ins tag_op:$tag), [],
"throw \t$tag", "throw \t$tag", 0x08>;
-defm RETHROW : NRI<(outs), (ins i32imm:$depth), [], "rethrow \t$depth", 0x09>;
+// $ehpad is the EH pad where the exception to rethrow has been caught.
+defm RETHROW : NRI<(outs), (ins bb_op:$ehpad), [], "rethrow \t$ehpad", 0x09>;
} // isTerminator = 1, hasCtrlDep = 1, isBarrier = 1
-// The depth argument will be computed in CFGStackify. We set it to 0 here for
-// now.
-def : Pat<(int_wasm_rethrow), (RETHROW 0)>;
// Region within which an exception is caught: try / end_try
let Uses = [VALUE_STACK], Defs = [VALUE_STACK] in {
@@ -160,7 +158,8 @@ defm DELEGATE : NRI<(outs), (ins bb_op:$dst), [], "delegate \t $dst", 0x18>;
// 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,
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
index 94037b9ab189db..b8f3bcb57f6bf3 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
@@ -245,11 +245,39 @@ 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 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:
+ //
+ // 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()
+ TI->removeOperand(0);
+ 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.
+ //
+ // 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;
diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td
index 5a8177e2b3607b..9b13447754e4cb 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.mir b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.mir
index 0386410d1b6120..c434e14b30d153 100644
--- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.mir
+++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.mir
@@ -39,15 +39,16 @@ body: |
; CHECK: RETHROW 1
EH_LABEL <mcsymbol .Ltmp2>
%0:i32 = CATCH &__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
; CHECK: RETHROW 0
EH_LABEL <mcsymbol .Ltmp3>
%1:i32 = CATCH &__cpp_exception, implicit-def dead $arguments
- RETHROW 0, implicit-def dead $arguments
+ RETHROW %bb.2, implicit-def dead $arguments
bb.3:
; CHECK: bb.3:
@@ -104,12 +105,14 @@ body: |
RETURN %0:i32, implicit-def dead $arguments
bb.3 (landing-pad):
+ successors:
EH_LABEL <mcsymbol .Ltmp4>
%0:i32 = CATCH &__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 <mcsymbol .Ltmp5>
%1:i32 = CATCH &__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 3537baa425164c..a0429f40b25407 100644
--- a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
+++ b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
@@ -400,6 +400,107 @@ 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: catch_all
+; CHECK: call _ZSt9terminatev
+; CHECK: end_try
+; CHECK: end_try
+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 +516,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.mir b/llvm/test/CodeGen/WebAssembly/exception.mir
index 895e8d8864ea25..a5f78c18db16a2 100644
--- a/llvm/test/CodeGen/WebAssembly/exception.mir
+++ b/llvm/test/CodeGen/WebAssembly/exception.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
|
|
@llvm/pr-subscribers-backend-aarch64 Author: Heejin Ahn (aheejin) ChangesSo 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: ehcleanup:
%0 = cleanuppad ...
call @<!-- -->destructor
cleanupret from %0 unwind label %catch.dispatchIf 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.dispatchWe lower a 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 This problem can be fixed easily if catchret already has two basic block arguments, even though neither of them means catchpad's BB.
This PR adds the
After this PR, our pseudo In case of Fixes #114600. Full diff: https://github.com/llvm/llvm-project/pull/115844.diff 11 Files Affected:
diff --git a/llvm/include/llvm/Target/TargetSelectionDAG.td b/llvm/include/llvm/Target/TargetSelectionDAG.td
index 46044aab79a832..e7895258438d26 100644
--- a/llvm/include/llvm/Target/TargetSelectionDAG.td
+++ b/llvm/include/llvm/Target/TargetSelectionDAG.td
@@ -231,6 +231,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
@@ -680,7 +684,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 37b1131d2f8a33..7fa3b8a73a4190 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2155,8 +2155,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.MBBMap[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/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index 1053ba9242768a..95f2f91f82bd41 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -5159,7 +5159,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<[]>;
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
index 70b91c266c4970..cd0aea313da0db 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
@@ -87,9 +87,8 @@ class WebAssemblyCFGStackify final : public MachineFunctionPass {
const MachineBasicBlock *MBB);
unsigned getDelegateDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
const MachineBasicBlock *MBB);
- unsigned
- getRethrowDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
- const SmallVectorImpl<const MachineBasicBlock *> &EHPadStack);
+ unsigned getRethrowDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
+ const MachineBasicBlock *EHPadToRethrow);
void rewriteDepthImmediates(MachineFunction &MF);
void fixEndsAtEndOfFunction(MachineFunction &MF);
void cleanupFunctionData(MachineFunction &MF);
@@ -1612,34 +1611,13 @@ unsigned WebAssemblyCFGStackify::getDelegateDepth(
unsigned WebAssemblyCFGStackify::getRethrowDepth(
const SmallVectorImpl<EndMarkerInfo> &Stack,
- const SmallVectorImpl<const MachineBasicBlock *> &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;
@@ -1651,7 +1629,6 @@ unsigned WebAssemblyCFGStackify::getRethrowDepth(
void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
// Now rewrite references to basic blocks to be depth immediates.
SmallVector<EndMarkerInfo, 8> Stack;
- SmallVector<const MachineBasicBlock *, 8> EHPadStack;
for (auto &MBB : reverse(MF)) {
for (MachineInstr &MI : llvm::reverse(MBB)) {
switch (MI.getOpcode()) {
@@ -1669,31 +1646,14 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
break;
case WebAssembly::END_BLOCK:
+ case WebAssembly::END_TRY:
Stack.push_back(std::make_pair(&MBB, &MI));
break;
- case WebAssembly::END_TRY: {
- // We handle DELEGATE in the default level, because DELEGATE has
- // immediate operands to rewrite.
- Stack.push_back(std::make_pair(&MBB, &MI));
- auto *EHPad = TryToEHPad[EndToBegin[&MI]];
- EHPadStack.push_back(EHPad);
- break;
- }
-
case WebAssembly::END_LOOP:
Stack.push_back(std::make_pair(EndToBegin[&MI]->getParent(), &MI));
break;
- case WebAssembly::CATCH:
- case WebAssembly::CATCH_ALL:
- EHPadStack.pop_back();
- break;
-
- case WebAssembly::RETHROW:
- MI.getOperand(0).setImm(getRethrowDepth(Stack, EHPadStack));
- break;
-
default:
if (MI.isTerminator()) {
// Rewrite MBB operands to be depth immediates.
@@ -1705,6 +1665,9 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
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()));
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
index 0f06f54f219f9a..18545e92886ac5 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
@@ -245,6 +245,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 be6547007aaf7a..261277f8a02cf8 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
@@ -132,11 +132,9 @@ let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in {
defm THROW : I<(outs), (ins tag_op:$tag, variable_ops),
(outs), (ins tag_op:$tag), [],
"throw \t$tag", "throw \t$tag", 0x08>;
-defm RETHROW : NRI<(outs), (ins i32imm:$depth), [], "rethrow \t$depth", 0x09>;
+// $ehpad is the EH pad where the exception to rethrow has been caught.
+defm RETHROW : NRI<(outs), (ins bb_op:$ehpad), [], "rethrow \t$ehpad", 0x09>;
} // isTerminator = 1, hasCtrlDep = 1, isBarrier = 1
-// The depth argument will be computed in CFGStackify. We set it to 0 here for
-// now.
-def : Pat<(int_wasm_rethrow), (RETHROW 0)>;
// Region within which an exception is caught: try / end_try
let Uses = [VALUE_STACK], Defs = [VALUE_STACK] in {
@@ -160,7 +158,8 @@ defm DELEGATE : NRI<(outs), (ins bb_op:$dst), [], "delegate \t $dst", 0x18>;
// 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,
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
index 94037b9ab189db..b8f3bcb57f6bf3 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
@@ -245,11 +245,39 @@ 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 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:
+ //
+ // 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()
+ TI->removeOperand(0);
+ 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.
+ //
+ // 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;
diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td
index 5a8177e2b3607b..9b13447754e4cb 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.mir b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.mir
index 0386410d1b6120..c434e14b30d153 100644
--- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.mir
+++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.mir
@@ -39,15 +39,16 @@ body: |
; CHECK: RETHROW 1
EH_LABEL <mcsymbol .Ltmp2>
%0:i32 = CATCH &__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
; CHECK: RETHROW 0
EH_LABEL <mcsymbol .Ltmp3>
%1:i32 = CATCH &__cpp_exception, implicit-def dead $arguments
- RETHROW 0, implicit-def dead $arguments
+ RETHROW %bb.2, implicit-def dead $arguments
bb.3:
; CHECK: bb.3:
@@ -104,12 +105,14 @@ body: |
RETURN %0:i32, implicit-def dead $arguments
bb.3 (landing-pad):
+ successors:
EH_LABEL <mcsymbol .Ltmp4>
%0:i32 = CATCH &__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 <mcsymbol .Ltmp5>
%1:i32 = CATCH &__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 3537baa425164c..a0429f40b25407 100644
--- a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
+++ b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
@@ -400,6 +400,107 @@ 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: catch_all
+; CHECK: call _ZSt9terminatev
+; CHECK: end_try
+; CHECK: end_try
+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 +516,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.mir b/llvm/test/CodeGen/WebAssembly/exception.mir
index 895e8d8864ea25..a5f78c18db16a2 100644
--- a/llvm/test/CodeGen/WebAssembly/exception.mir
+++ b/llvm/test/CodeGen/WebAssembly/exception.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
|
|
@llvm/pr-subscribers-llvm-selectiondag Author: Heejin Ahn (aheejin) ChangesSo 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: ehcleanup:
%0 = cleanuppad ...
call @<!-- -->destructor
cleanupret from %0 unwind label %catch.dispatchIf 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.dispatchWe lower a 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 This problem can be fixed easily if catchret already has two basic block arguments, even though neither of them means catchpad's BB.
This PR adds the
After this PR, our pseudo In case of Fixes #114600. Full diff: https://github.com/llvm/llvm-project/pull/115844.diff 11 Files Affected:
diff --git a/llvm/include/llvm/Target/TargetSelectionDAG.td b/llvm/include/llvm/Target/TargetSelectionDAG.td
index 46044aab79a832..e7895258438d26 100644
--- a/llvm/include/llvm/Target/TargetSelectionDAG.td
+++ b/llvm/include/llvm/Target/TargetSelectionDAG.td
@@ -231,6 +231,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
@@ -680,7 +684,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 37b1131d2f8a33..7fa3b8a73a4190 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2155,8 +2155,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.MBBMap[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/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index 1053ba9242768a..95f2f91f82bd41 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -5159,7 +5159,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<[]>;
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
index 70b91c266c4970..cd0aea313da0db 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
@@ -87,9 +87,8 @@ class WebAssemblyCFGStackify final : public MachineFunctionPass {
const MachineBasicBlock *MBB);
unsigned getDelegateDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
const MachineBasicBlock *MBB);
- unsigned
- getRethrowDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
- const SmallVectorImpl<const MachineBasicBlock *> &EHPadStack);
+ unsigned getRethrowDepth(const SmallVectorImpl<EndMarkerInfo> &Stack,
+ const MachineBasicBlock *EHPadToRethrow);
void rewriteDepthImmediates(MachineFunction &MF);
void fixEndsAtEndOfFunction(MachineFunction &MF);
void cleanupFunctionData(MachineFunction &MF);
@@ -1612,34 +1611,13 @@ unsigned WebAssemblyCFGStackify::getDelegateDepth(
unsigned WebAssemblyCFGStackify::getRethrowDepth(
const SmallVectorImpl<EndMarkerInfo> &Stack,
- const SmallVectorImpl<const MachineBasicBlock *> &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;
@@ -1651,7 +1629,6 @@ unsigned WebAssemblyCFGStackify::getRethrowDepth(
void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
// Now rewrite references to basic blocks to be depth immediates.
SmallVector<EndMarkerInfo, 8> Stack;
- SmallVector<const MachineBasicBlock *, 8> EHPadStack;
for (auto &MBB : reverse(MF)) {
for (MachineInstr &MI : llvm::reverse(MBB)) {
switch (MI.getOpcode()) {
@@ -1669,31 +1646,14 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
break;
case WebAssembly::END_BLOCK:
+ case WebAssembly::END_TRY:
Stack.push_back(std::make_pair(&MBB, &MI));
break;
- case WebAssembly::END_TRY: {
- // We handle DELEGATE in the default level, because DELEGATE has
- // immediate operands to rewrite.
- Stack.push_back(std::make_pair(&MBB, &MI));
- auto *EHPad = TryToEHPad[EndToBegin[&MI]];
- EHPadStack.push_back(EHPad);
- break;
- }
-
case WebAssembly::END_LOOP:
Stack.push_back(std::make_pair(EndToBegin[&MI]->getParent(), &MI));
break;
- case WebAssembly::CATCH:
- case WebAssembly::CATCH_ALL:
- EHPadStack.pop_back();
- break;
-
- case WebAssembly::RETHROW:
- MI.getOperand(0).setImm(getRethrowDepth(Stack, EHPadStack));
- break;
-
default:
if (MI.isTerminator()) {
// Rewrite MBB operands to be depth immediates.
@@ -1705,6 +1665,9 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
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()));
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
index 0f06f54f219f9a..18545e92886ac5 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
@@ -245,6 +245,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 be6547007aaf7a..261277f8a02cf8 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
@@ -132,11 +132,9 @@ let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in {
defm THROW : I<(outs), (ins tag_op:$tag, variable_ops),
(outs), (ins tag_op:$tag), [],
"throw \t$tag", "throw \t$tag", 0x08>;
-defm RETHROW : NRI<(outs), (ins i32imm:$depth), [], "rethrow \t$depth", 0x09>;
+// $ehpad is the EH pad where the exception to rethrow has been caught.
+defm RETHROW : NRI<(outs), (ins bb_op:$ehpad), [], "rethrow \t$ehpad", 0x09>;
} // isTerminator = 1, hasCtrlDep = 1, isBarrier = 1
-// The depth argument will be computed in CFGStackify. We set it to 0 here for
-// now.
-def : Pat<(int_wasm_rethrow), (RETHROW 0)>;
// Region within which an exception is caught: try / end_try
let Uses = [VALUE_STACK], Defs = [VALUE_STACK] in {
@@ -160,7 +158,8 @@ defm DELEGATE : NRI<(outs), (ins bb_op:$dst), [], "delegate \t $dst", 0x18>;
// 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,
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
index 94037b9ab189db..b8f3bcb57f6bf3 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
@@ -245,11 +245,39 @@ 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 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:
+ //
+ // 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()
+ TI->removeOperand(0);
+ 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.
+ //
+ // 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;
diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td
index 5a8177e2b3607b..9b13447754e4cb 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.mir b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.mir
index 0386410d1b6120..c434e14b30d153 100644
--- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.mir
+++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.mir
@@ -39,15 +39,16 @@ body: |
; CHECK: RETHROW 1
EH_LABEL <mcsymbol .Ltmp2>
%0:i32 = CATCH &__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
; CHECK: RETHROW 0
EH_LABEL <mcsymbol .Ltmp3>
%1:i32 = CATCH &__cpp_exception, implicit-def dead $arguments
- RETHROW 0, implicit-def dead $arguments
+ RETHROW %bb.2, implicit-def dead $arguments
bb.3:
; CHECK: bb.3:
@@ -104,12 +105,14 @@ body: |
RETURN %0:i32, implicit-def dead $arguments
bb.3 (landing-pad):
+ successors:
EH_LABEL <mcsymbol .Ltmp4>
%0:i32 = CATCH &__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 <mcsymbol .Ltmp5>
%1:i32 = CATCH &__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 3537baa425164c..a0429f40b25407 100644
--- a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
+++ b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
@@ -400,6 +400,107 @@ 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: catch_all
+; CHECK: call _ZSt9terminatev
+; CHECK: end_try
+; CHECK: end_try
+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 +516,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.mir b/llvm/test/CodeGen/WebAssembly/exception.mir
index 895e8d8864ea25..a5f78c18db16a2 100644
--- a/llvm/test/CodeGen/WebAssembly/exception.mir
+++ b/llvm/test/CodeGen/WebAssembly/exception.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
|
aheejin
changed the title
dschuff
approved these changes
Nov 12, 2024
|
What do you mean by already done? This is a request to backport the fix to 19.4 release. |
|
oh nevermind, I was confused; #114693 is closed, but this is the PR that implements the backport. |
|
This is a pretty big patch to bring into a backport. How risky is this and how important is this to end up in a backport? |
|
FWIW I believe this is sufficiently safe. And this is a fix for a serious bug, which I think should land sooner than LLVM 20. |
tru
force-pushed
the
rethrow_fix_backport
branch
from
54d71d7 to
aadaa00
Compare
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 llvm#114600.
|
@aheejin (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
WebAssembly: Fixed a bug that miscalculated the argument of 'rethrow' instruction. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport 492812f
Requested by: @aheejin