From 3c8f3b91d898cb3f76e1e430da98972cdf8a4a1c Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Tue, 18 Jun 2024 21:56:41 -0700 Subject: [PATCH] [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 ``` 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 ``` 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. --- .../SelectionDAG/SelectionDAGBuilder.cpp | 2 +- .../CodeGen/WebAssembly/exception-legacy.ll | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 3f5d1a5047a423..296b06187ec0fd 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 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 dfa33f95f37bec..3537baa425164c 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