[llvm] 3c8f3b9 - [WebAssembly] Treat 'rethrow' as terminator in custom isel (#95967)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 18 21:56:46 PDT 2024
Author: Heejin Ahn
Date: 2024-06-18T21:56:41-07:00
New Revision: 3c8f3b91d898cb3f76e1e430da98972cdf8a4a1c
URL: https://github.com/llvm/llvm-project/commit/3c8f3b91d898cb3f76e1e430da98972cdf8a4a1c
DIFF: https://github.com/llvm/llvm-project/commit/3c8f3b91d898cb3f76e1e430da98972cdf8a4a1c.diff
LOG: [WebAssembly] Treat 'rethrow' as terminator in custom isel (#95967)
`rethrow` instruction is a terminator, but when when its DAG is built in
`SelectionDAGBuilder` in a custom routine, it was NOT treated as such.
```ll
rethrow: ; preds = %catch.start
invoke void @llvm.wasm.rethrow() #1 [ "funclet"(token %1) ]
to label %unreachable unwind label %ehcleanup
ehcleanup: ; preds = %rethrow, %catch.dispatch
%tmp = phi i32 [ 10, %catch.dispatch ], [ 20, %rethrow ]
...
```
In this bitcode, because of the `phi`, a `CONST_I32` will be created in
the `rethrow` BB. Without this patch, the DAG for the `rethrow` BB looks
like this:
```
t0: ch,glue = EntryToken
t3: ch = CopyToReg t0, Register:i32 %9, Constant:i32<20>
t5: ch = llvm.wasm.rethrow t0, TargetConstant:i32<12161>
t6: ch = TokenFactor t3, t5
t8: ch = br t6, BasicBlock:ch<unreachable 0x562532e43c50>
```
Note that `CopyToReg` and `llvm.wasm.rethrow` don't have dependence so
either can come first in the selected code, which can result in the code
like
```mir
bb.3.rethrow:
RETHROW 0, implicit-def dead $arguments
%9:i32 = CONST_I32 20, implicit-def dead $arguments
BR %bb.6, implicit-def dead $arguments
```
After this patch, `llvm.wasm.rethrow` is treated as a terminator, and
the DAG will look like
```
t0: ch,glue = EntryToken
t3: ch = CopyToReg t0, Register:i32 %9, Constant:i32<20>
t5: ch = llvm.wasm.rethrow t3, TargetConstant:i32<12161>
t7: ch = br t5, BasicBlock:ch<unreachable 0x5555e3d32c70>
```
Note that now `rethrow` takes a token from `CopyToReg`, so `rethrow` has
to come after `CopyToReg`. And the resulting code will be
```mir
bb.3.rethrow:
%9:i32 = CONST_I32 20, implicit-def dead $arguments
RETHROW 0, implicit-def dead $arguments
BR %bb.6, implicit-def dead $arguments
```
I'm not very familiar with the internals of `getRoot` vs.
`getControlRoot`, but other terminator instructions seem to use the
latter, and using it for `rethrow` too worked.
Added:
Modified:
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/test/CodeGen/WebAssembly/exception-legacy.ll
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 3f5d1a5047a42..296b06187ec0f 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -3355,7 +3355,7 @@ void SelectionDAGBuilder::visitInvoke(const InvokeInst &I) {
// special because it can be invoked, so we manually lower it to a DAG
// node here.
SmallVector<SDValue, 8> Ops;
- Ops.push_back(getRoot()); // inchain
+ Ops.push_back(getControlRoot()); // inchain for the terminator node
const TargetLowering &TLI = DAG.getTargetLoweringInfo();
Ops.push_back(
DAG.getTargetConstant(Intrinsic::wasm_rethrow, getCurSDLoc(),
diff --git a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
index dfa33f95f37be..3537baa425164 100644
--- a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
+++ b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
@@ -350,8 +350,60 @@ ehcleanupret: ; preds = %catch.start, %ehcle
cleanupret from %0 unwind to caller
}
+; Regression test for the bug that 'rethrow' was not treated correctly as a
+; terminator in isel.
+define void @test_rethrow_terminator() personality ptr @__gxx_wasm_personality_v0 {
+entry:
+ invoke void @foo()
+ to label %try.cont unwind label %catch.dispatch
+
+catch.dispatch: ; preds = %entry
+ %0 = catchswitch within none [label %catch.start] unwind label %ehcleanup
+
+catch.start: ; preds = %catch.dispatch
+ %1 = catchpad within %0 [ptr @_ZTIi]
+ %2 = call ptr @llvm.wasm.get.exception(token %1)
+ %3 = call i32 @llvm.wasm.get.ehselector(token %1)
+ %4 = call i32 @llvm.eh.typeid.for.p0(ptr @_ZTIi)
+ %matches = icmp eq i32 %3, %4
+ br i1 %matches, label %catch, label %rethrow
+
+catch: ; preds = %catch.start
+ %5 = call ptr @__cxa_begin_catch(ptr %2) [ "funclet"(token %1) ]
+ %6 = load i32, ptr %5, align 4
+ call void @__cxa_end_catch() [ "funclet"(token %1) ]
+ catchret from %1 to label %try.cont
+
+rethrow: ; preds = %catch.start
+ invoke void @llvm.wasm.rethrow() #1 [ "funclet"(token %1) ]
+ to label %unreachable unwind label %ehcleanup
+
+try.cont: ; preds = %entry, %catch
+ ret void
+
+ehcleanup: ; preds = %rethrow, %catch.dispatch
+ ; 'rethrow' BB is this BB's predecessor, and its
+ ; 'invoke void @llvm.wasm.rethrow()' is lowered down to a 'RETHROW' in Wasm
+ ; MIR. And this 'phi' creates 'CONST_I32' instruction in the predecessor
+ ; 'rethrow' BB. If 'RETHROW' is not treated correctly as a terminator, it can
+ ; create a BB like
+ ; bb.3.rethrow:
+ ; RETHROW 0
+ ; %0 = CONST_I32 20
+ ; BR ...
+ %tmp = phi i32 [ 10, %catch.dispatch ], [ 20, %rethrow ]
+ %7 = cleanuppad within none []
+ call void @take_i32(i32 %tmp) [ "funclet"(token %7) ]
+ cleanupret from %7 unwind to caller
+
+unreachable: ; preds = %rethrow
+ unreachable
+}
+
+
declare void @foo()
declare void @bar(ptr)
+declare void @take_i32(i32)
declare i32 @__gxx_wasm_personality_v0(...)
; Function Attrs: noreturn
declare void @llvm.wasm.throw(i32, ptr) #1
More information about the llvm-commits
mailing list