[all-commits] [llvm/llvm-project] 494fe0: [WebAssembly] Remove wasm-specific findWasmUnwindD...

[Heejin Ahn via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 494fe0b4145810d4e4e7b6003cabd194f76cb5d4
      https://github.com/llvm/llvm-project/commit/494fe0b4145810d4e4e7b6003cabd194f76cb5d4
  Author: Heejin Ahn <aheejin at gmail.com>
  Date:   2025-03-10 (Mon, 10 Mar 2025)

  Changed paths:
    M llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
    M llvm/lib/CodeGen/WinEHPrepare.cpp
    M llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
    M llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
    M llvm/test/CodeGen/WebAssembly/exception-legacy.ll
    M llvm/test/CodeGen/WebAssembly/exception.ll

  Log Message:
  -----------
  [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.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications