aboutsummaryrefslogtreecommitdiff
path: root/clang/lib/CodeGen/ModuleBuilder.cpp
diff options
context:
space:
mode:
authorHeejin Ahn <aheejin@gmail.com>2025-03-10 20:56:38 -0700
committerGitHub <noreply@github.com>2025-03-10 20:56:38 -0700
commit494fe0b4145810d4e4e7b6003cabd194f76cb5d4 (patch)
tree954e5deca5f21c5213d7c01aa04d5831940e3ca8 /clang/lib/CodeGen/ModuleBuilder.cpp
parent95e38baf24ed7509decf1b9ee362cf3e23e68e54 (diff)
downloadllvm-494fe0b4145810d4e4e7b6003cabd194f76cb5d4.zip
llvm-494fe0b4145810d4e4e7b6003cabd194f76cb5d4.tar.gz
llvm-494fe0b4145810d4e4e7b6003cabd194f76cb5d4.tar.bz2
[WebAssembly] Remove wasm-specific findWasmUnwindDestinations (#130374)
Unlike in Itanium EH IR, WinEH IR's unwinding instructions (e.g. `invoke`s) can have multiple possible unwind destinations. For example: ```ll entry: invoke void @foo() to label %cont unwind label %catch.dispatch catch.dispatch: ; preds = %entry %0 = catchswitch within none [label %catch.start] unwind label %terminate catch.start: ; preds = %catch.dispatch %1 = catchpad within %0 [ptr null] ... terminate: ; preds = %catch.dispatch %2 = catchpad within none [] ... ... ``` In this case, if an exception is not caught by `catch.dispatch` (and thus `catch.start`), it should next unwind to `terminate`. `findUnwindDestination` in ISel gathers the list of this unwind destinations traversing the unwind edges: https://github.com/llvm/llvm-project/blob/ae42f071032b29821beef6a33771258086bbbb1c/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp#L2089-L2150 But we don't use that, and instead use our custom `findWasmUnwindDestinations` that only adds the first unwind destination, `catch.start`, to the successor list of `entry`, and not `terminate`: https://github.com/llvm/llvm-project/blob/ae42f071032b29821beef6a33771258086bbbb1c/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp#L2037-L2087 The reason behind it was, as described in the comment block in the code, it was assumed that there always would be an `invoke` that connects `catch.start` and `terminate`. In case of `catch (type)`, there will be `call void @llvm.wasm.rethrow()` in `catch.start`'s predecessor that unwinds to the next destination. For example: https://github.com/llvm/llvm-project/blob/0db702ac8e06911478615ac537f75ac778817c04/llvm/test/CodeGen/WebAssembly/exception.ll#L429-L430 In case of `catch (...)`, `__cxa_end_catch` can throw, so it becomes an `invoke` that unwinds to the next destination. For example: https://github.com/llvm/llvm-project/blob/0db702ac8e06911478615ac537f75ac778817c04/llvm/test/CodeGen/WebAssembly/exception.ll#L537-L538 So the unwind ordering relationship between `catch.start` and `terminate` here would be preserved. But turns out this assumption does not always hold. For example: ```ll entry: invoke void @foo() to label %cont unwind label %catch.dispatch catch.dispatch: ; preds = %entry %0 = catchswitch within none [label %catch.start] unwind label %terminate catch.start: ; preds = %catch.dispatch %1 = catchpad within %0 [ptr null] ... call void @_ZSt9terminatev() unreachable terminate: ; preds = %catch.dispatch %2 = catchpad within none [] call void @_ZSt9terminatev() unreachable ... ``` In this case there is no `invoke` that connects `catch.start` to `terminate`. So after `catch.dispatch` BB is removed in ISel, `terminate` is considered unreachable and incorrectly removed in DCE. This makes Wasm just use the general `findUnwindDestination`. In that case `entry`'s successor is going to be [`catch.start`, `terminate`]. We can get the first unwind destination by just traversing the list from the front. --- This required another change in WinEHPrepare. WinEHPrepare demotes all PHIs in EH pads because they are funclets in Windows and funclets can't have PHIs. When used in Wasm they are not funclets so we don't need to do that wholesale but we still need to demote PHIs in `catchswitch` BBs because they are deleted during ISel. (So we created [`-demote-catchswitch-only`](https://github.com/llvm/llvm-project/blob/a5588b6d20590a10db0f1a2046fba4d9f205ed68/llvm/lib/CodeGen/WinEHPrepare.cpp#L57-L59) option for that) But turns out we need to remove PHIs that have a `catchswitch` BB as an incoming block too: ```ll ... catch.dispatch: %0 = catchswitch within none [label %catch.start] unwind label %terminate catch.start: ... somebb: ... ehcleanup ; preds = %catch.dispatch, %somebb %1 = phi i32 [ 10, %catch.dispatch ], [ 20, %somebb ] ... ``` In this case the `phi` in `ehcleanup` BB should be demoted too because `catch.dispatch` BB will be removed in ISel so one if its incoming block will be gone. This pattern didn't manifest before presumably due to how `findWasmUnwindDestinations` worked. (In this example, in our `findWasmUnwindDestinations`, `catch.dispatch` would have had only one successor, `catch.start`. But now `catch.dispatch` has both `catch.start` and `ehcleanup` as successors, revealing this bug. This case is [represented](https://github.com/llvm/llvm-project/blob/ab87206c4b95aa0b5047facffb5f78f7fe6ac269/llvm/test/CodeGen/WebAssembly/exception.ll#L445) by `rethrow_terminator` function in `exception.ll` (or `exception-legacy.ll`) and without the WinEHPrepare fix it will crash. --- Discovered by the reproducer provided in #126916, even though the bug reported there was not this one.
Diffstat (limited to 'clang/lib/CodeGen/ModuleBuilder.cpp')
0 files changed, 0 insertions, 0 deletions