[llvm] [WebAssembly] Remove ExceptionInfo grouping tweaks (NFC) (PR #191495)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 10 12:09:22 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-webassembly
Author: Heejin Ahn (aheejin)
<details>
<summary>Changes</summary>
This removes fixes implemented in https://github.com/llvm/llvm-project/commit/ea8c6375e3330f181105106b3adb84ff9fa76a7c, https://github.com/llvm/llvm-project/commit/4a58116b7e5e1439c5fefdf59a89fc4f1d42875c, and https://github.com/llvm/llvm-project/commit/2b957ed4ff3344d8f761a053566e307277a1cdeb.
We don't need them anymore after #<!-- -->130374.
---
A little (unfortunate) winding history, mostly for my mental bookeeping. Read the below only if you are curious:
There is a function called `findUnwindDestinations` in `SelectionDAGBuilder.cpp`.
https://github.com/llvm/llvm-project/blob/c94f79886035a61bb5f3dc992f75fe0c08bdcd4b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp#L2107-L2164
This function adds unwind successors to BBs with `invoke`s. In case of Itanium EH, you only add one `landingpad` BB. In WinEH, `catchswitch` may not catch an exception, so you add all possible unwind destionations. For example,
```ll
entry:
invoke void @<!-- -->foo()
to label %try.cont unwind label %catch.dispatch
catch.dispatch:
%0 = catchswitch within none [label %catch.start] unwind label %catch.dispatch1
catch.start:
...
catch.dispatch1:
%7 = catchswitch within none [label %catch.start1] unwind to caller
catch.start1:
...
```
`catchswitch` BBs are removed in iSel. So in this case, both `catch.start` and `catch.start1` BBs are added as unwind successors to `entry`, because an exception may not be caught by `catch.dispatch` and unwind further to `catch.dispatch1`.
In the beginning of 2019, I added our own `findWasmUnwindDestinations` in https://github.com/llvm/llvm-project/commit/d6f487863dc951d467b545b86b9ea62980569b5a. This was when I was implementing [the V2 (pre-legacy) proposal,](https://github.com/WebAssembly/exception-handling/blob/main/proposals/exception-handling/pre-legacy/Exceptions-v2.md) which had `exnref` and `try`-`catch_all` (It was named `catch`, but semantically it was `catch_all`) The rationale was, even though we were using WinEH, we only had one catchpad and `catch` caught everything. So I figured adding only the first catchpad successor, `catch.start` in the example above, would simpify things.
By the end of 2020, we changed the proposal to [the V3 (legacy) proposal](https://github.com/WebAssembly/exception-handling/blob/main/proposals/exception-handling/legacy/Exceptions.md), which removed `exnref` and introduced separate `catch` and `catch_all` instructions. The previous invariant "`catch` always catches everything" didn't hold anymore, but I left `findWasmUnwindDestinations` as was with some updated comments in https://github.com/llvm/llvm-project/commit/9e4eadeb135d140b3a9e499354472170017cbe58. The comments could be summed up as "there will always be an `invoke` instruction in the first catchpad that unwinds to the next unwind destination. (which later turned out to be false)
And in 2021, I tweaked the ExceptionInfo algorithm to fix exception grouping (https://github.com/llvm/llvm-project/commit/ea8c6375e3330f181105106b3adb84ff9fa76a7c, https://github.com/llvm/llvm-project/commit/4a58116b7e5e1439c5fefdf59a89fc4f1d42875c, and https://github.com/llvm/llvm-project/commit/2b957ed4ff3344d8f761a053566e307277a1cdeb) The bug was, in tl;dr: "Your next unwind destination can be (accidentally) dominated by your current catchpad, making your unwind destination a subexception of the current exception). For example:
```cpp
try {
try {
foo();
} catch (int) { // EH pad
...
}
} catch (...) { // unwind destination
}
```
Here the outer `catch` is (accidentally) dominated by the inner `catch`, because we only added the first catchpad (inner `catch`) as an unwind successor of `foo()` BB, and hoped that some `invoke`s within the inner `catch` to unwind it to the outer `catch`. But this caused us to `delegate` to a middle of an inner scope. So I tweaked the algorithm to take the outer `catch` out to form a separate exception. I didn't realize `findWasmUnwindDestinations` was actually the source of problem then.
Fast forward to 2025. The 2020 assumption of "There will always be an `invoke` instruction in the first catchpad" turned out to be false. So I just removed `findWasmUnwindDestinations` and switched to use the common `findUnwindDestinations` in #<!-- -->130374, which recently accidentally discovered another bug (#<!-- -->187302).
While investigating #<!-- -->187302, I realized we don't need those tweaks in WebAssemblyExceptionInfo anymore, because `findUnwindDestinations` adds all unwind destinations as successors. (#<!-- -->187302 is actually not related to this; it was just a trigger to investigate things) So in case of the little C++ example above, the outer `catch` BB will also be added as an unwind successor of the `foo()` BB. I actually think we may not even need WebAssemblyExceptionInfo analysis at all if we only use [the latest standard (exnref) proposal](https://github.com/WebAssembly/exception-handling/blob/main/proposals/exception-handling/Exceptions.md). But we still need to keep the legacy support, so we need it for now.
---
Full diff: https://github.com/llvm/llvm-project/pull/191495.diff
2 Files Affected:
- (modified) llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp (-165)
- (modified) llvm/test/CodeGen/WebAssembly/function-info.mir (+1-1)
``````````diff
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp
index a8a2454cd14ca..97d5ab5c5fb0a 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp
@@ -13,11 +13,9 @@
#include "WebAssemblyExceptionInfo.h"
#include "WebAssemblyUtilities.h"
-#include "llvm/ADT/DepthFirstIterator.h"
#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/CodeGen/MachineDominanceFrontier.h"
#include "llvm/CodeGen/MachineDominators.h"
-#include "llvm/CodeGen/WasmEHFuncInfo.h"
#include "llvm/IR/Function.h"
#include "llvm/InitializePasses.h"
#include "llvm/MC/MCAsmInfo.h"
@@ -52,29 +50,6 @@ bool WebAssemblyExceptionInfo::runOnMachineFunction(MachineFunction &MF) {
return false;
}
-// Check if Dst is reachable from Src using BFS. Search only within BBs
-// dominated by Header.
-static bool isReachableAmongDominated(const MachineBasicBlock *Src,
- const MachineBasicBlock *Dst,
- const MachineBasicBlock *Header,
- const MachineDominatorTree &MDT) {
- assert(MDT.dominates(Header, Dst));
- SmallVector<const MachineBasicBlock *, 8> WL;
- SmallPtrSet<const MachineBasicBlock *, 8> Visited;
- WL.push_back(Src);
-
- while (!WL.empty()) {
- const auto *MBB = WL.pop_back_val();
- if (MBB == Dst)
- return true;
- Visited.insert(MBB);
- for (auto *Succ : MBB->successors())
- if (!Visited.count(Succ) && MDT.dominates(Header, Succ))
- WL.push_back(Succ);
- }
- return false;
-}
-
void WebAssemblyExceptionInfo::recalculate(
MachineFunction &MF, MachineDominatorTree &MDT,
const MachineDominanceFrontier &MDF) {
@@ -89,107 +64,6 @@ void WebAssemblyExceptionInfo::recalculate(
Exceptions.push_back(std::move(WE));
}
- // WasmEHFuncInfo contains a map of <catchpad, its next unwind destination>,
- // which means, if an exception is not caught by the catchpad, it should end
- // up in the next unwind destination stored in this data structure. (It is
- // written as catchswitch's 'unwind' destination in ll files.) The below is an
- // intuitive example of their relationship in C++ code:
- // try {
- // try {
- // } catch (int) { // catchpad
- // ... // this catch (int) { ... } is grouped as an exception
- // }
- // } catch (...) { // next unwind destination
- // }
- // (The example is try-catches for illustration purpose, but the unwind
- // destination can be also a cleanuppad generated by destructor calls.) So the
- // unwind destination is in the outside of the catchpad's exception.
- //
- // We group exceptions in this analysis simply by including all BBs dominated
- // by an EH pad. But in case the EH pad's unwind destination does not have any
- // children outside of the exception, that unwind destination ends up also
- // being dominated by the EH pad and included in the exception, which is not
- // semantically correct, because it unwinds/rethrows into an inner scope.
- //
- // Here we extract those unwind destinations from their (incorrect) parent
- // exception. Note that the unwind destinations may not be an immediate
- // children of the parent exception, so we have to traverse the parent chain.
- //
- // We should traverse BBs in the preorder of the dominator tree, because
- // otherwise the result can be incorrect. For example, when there are three
- // exceptions A, B, and C and A > B > C (> is subexception relationship here),
- // and A's unwind destination is B and B's is C. When we visit B before A, we
- // end up extracting C only out of B but not out of A.
- const auto *EHInfo = MF.getWasmEHFuncInfo();
- assert(EHInfo);
- SmallVector<std::pair<WebAssemblyException *, WebAssemblyException *>>
- UnwindWEVec;
- for (auto *DomNode : depth_first(&MDT)) {
- MachineBasicBlock *EHPad = DomNode->getBlock();
- if (!EHPad->isEHPad())
- continue;
- if (!EHInfo->hasUnwindDest(EHPad))
- continue;
- auto *UnwindDest = EHInfo->getUnwindDest(EHPad);
- auto *SrcWE = getExceptionFor(EHPad);
- auto *DstWE = getExceptionFor(UnwindDest);
- if (SrcWE->contains(DstWE)) {
- UnwindWEVec.push_back(std::make_pair(SrcWE, DstWE));
- LLVM_DEBUG(dbgs() << "Unwind destination ExceptionInfo fix:\n "
- << DstWE->getEHPad()->getNumber() << "."
- << DstWE->getEHPad()->getName()
- << "'s exception is taken out of "
- << SrcWE->getEHPad()->getNumber() << "."
- << SrcWE->getEHPad()->getName() << "'s exception\n");
- DstWE->setParentException(SrcWE->getParentException());
- }
- }
-
- // After fixing subexception relationship between unwind destinations above,
- // there can still be remaining discrepancies.
- //
- // For example, suppose Exception A is dominated by EHPad A and Exception B is
- // dominated by EHPad B. EHPad A's unwind destination is EHPad B, but because
- // EHPad B is dominated by EHPad A, the initial grouping makes Exception B a
- // subexception of Exception A, and we fix it by taking Exception B out of
- // Exception A above. But there can still be remaining BBs within Exception A
- // that are reachable from Exception B. These BBs semantically don't belong
- // to Exception A and were not a part of this 'catch' clause or cleanup code
- // in the original code, but they just happened to be grouped within Exception
- // A because they were dominated by EHPad A. We fix this case by taking those
- // BBs out of the incorrect exception and all its subexceptions that it
- // belongs to.
- //
- // 1. First, we take out remaining incorrect subexceptions. This part is
- // easier, because we haven't added BBs to exceptions yet, we only need to
- // change parent exception pointer.
- for (auto *DomNode : depth_first(&MDT)) {
- MachineBasicBlock *EHPad = DomNode->getBlock();
- if (!EHPad->isEHPad())
- continue;
- auto *WE = getExceptionFor(EHPad);
-
- // For each source EHPad -> unwind destination EHPad
- for (auto &P : UnwindWEVec) {
- auto *SrcWE = P.first;
- auto *DstWE = P.second;
- // If WE (the current EH pad's exception) is still contained in SrcWE but
- // reachable from DstWE that was taken out of SrcWE above, we have to take
- // out WE out of SrcWE too.
- if (WE != SrcWE && SrcWE->contains(WE) && !DstWE->contains(WE) &&
- isReachableAmongDominated(DstWE->getEHPad(), EHPad, SrcWE->getEHPad(),
- MDT)) {
- LLVM_DEBUG(dbgs() << "Remaining reachable ExceptionInfo fix:\n "
- << WE->getEHPad()->getNumber() << "."
- << WE->getEHPad()->getName()
- << "'s exception is taken out of "
- << SrcWE->getEHPad()->getNumber() << "."
- << SrcWE->getEHPad()->getName() << "'s exception\n");
- WE->setParentException(SrcWE->getParentException());
- }
- }
- }
-
// Add BBs to exceptions' block set. This is a preparation to take out
// remaining incorect BBs from exceptions, because we need to iterate over BBs
// for each exception.
@@ -200,45 +74,6 @@ void WebAssemblyExceptionInfo::recalculate(
WE->addToBlocksSet(MBB);
}
- // 2. We take out remaining individual BBs out. Now we have added BBs to each
- // exceptions' BlockSet, when we take a BB out of an exception, we need to fix
- // those sets too.
- for (auto &P : UnwindWEVec) {
- auto *SrcWE = P.first;
- auto *DstWE = P.second;
-
- SrcWE->getBlocksSet().remove_if([&](MachineBasicBlock *MBB){
- if (MBB->isEHPad()) {
- assert(!isReachableAmongDominated(DstWE->getEHPad(), MBB,
- SrcWE->getEHPad(), MDT) &&
- "We already handled EH pads above");
- return false;
- }
- if (isReachableAmongDominated(DstWE->getEHPad(), MBB, SrcWE->getEHPad(),
- MDT)) {
- LLVM_DEBUG(dbgs() << "Remainder BB: " << MBB->getNumber() << "."
- << MBB->getName() << " is\n");
- WebAssemblyException *InnerWE = getExceptionFor(MBB);
- while (InnerWE != SrcWE) {
- LLVM_DEBUG(dbgs()
- << " removed from " << InnerWE->getEHPad()->getNumber()
- << "." << InnerWE->getEHPad()->getName()
- << "'s exception\n");
- InnerWE->removeFromBlocksSet(MBB);
- InnerWE = InnerWE->getParentException();
- }
- LLVM_DEBUG(dbgs() << " removed from " << SrcWE->getEHPad()->getNumber()
- << "." << SrcWE->getEHPad()->getName()
- << "'s exception\n");
- changeExceptionFor(MBB, SrcWE->getParentException());
- if (SrcWE->getParentException())
- SrcWE->getParentException()->addToBlocksSet(MBB);
- return true;
- }
- return false;
- });
- }
-
// Add BBs to exceptions' block vector
for (auto *DomNode : post_order(&MDT)) {
MachineBasicBlock *MBB = DomNode->getBlock();
diff --git a/llvm/test/CodeGen/WebAssembly/function-info.mir b/llvm/test/CodeGen/WebAssembly/function-info.mir
index ae7c15efe872f..0a8740927834e 100644
--- a/llvm/test/CodeGen/WebAssembly/function-info.mir
+++ b/llvm/test/CodeGen/WebAssembly/function-info.mir
@@ -53,7 +53,7 @@ machineFunctionInfo:
2: 3
body: |
bb.0:
- successors: %bb.1, %bb.2
+ successors: %bb.1, %bb.2, %bb.3
CALL @foo, implicit-def dead $arguments, implicit $sp32, implicit $sp64, implicit-def dead $arguments, implicit $sp32, implicit $sp64
bb.1:
``````````
</details>
https://github.com/llvm/llvm-project/pull/191495
More information about the llvm-commits
mailing list