From: Heejin Ahn Date: Tue, 8 Oct 2019 16:15:39 +0000 (+0000) Subject: [WebAssembly] Fix a bug in 'try' placement X-Git-Tag: llvmorg-11-init~7192 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6a37c5d6fcae2182856051f558aab6cb8ba7233c;p=platform%2Fupstream%2Fllvm.git [WebAssembly] Fix a bug in 'try' placement Summary: When searching for local expression tree created by stackified registers, for 'block' placement, we start the search from the previous instruction of a BB's terminator. But in 'try''s case, we should start from the previous instruction of a call that can throw, or a EH_LABEL that precedes the call, because the return values of the call's previous instructions can be stackified and consumed by the throwing call. For example, ``` i32.call @foo call @bar ; may throw br $label0 ``` In this case, if we start the search from the previous instruction of the terminator (`br` here), we end up stopping at `call @bar` and place a 'try' between `i32.call @foo` and `call @bar`, because `call @bar` does not have a return value so it is not a local expression tree of `br`. But in this case, unlike when placing 'block's, we should start the search from `call @bar`, because the return value of `i32.call @foo` is stackified and used by `call @bar`. Reviewers: dschuff Subscribers: sbc100, jgravelle-google, hiraditya, sunfish, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D68619 llvm-svn: 374073 --- diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp index d2a3557..1032709 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp @@ -526,40 +526,50 @@ void WebAssemblyCFGStackify::placeTryMarker(MachineBasicBlock &MBB) { AfterSet.insert(&MI); } - // Local expression tree should go after the TRY. - for (auto I = Header->getFirstTerminator(), E = Header->begin(); I != E; - --I) { - if (std::prev(I)->isDebugInstr() || std::prev(I)->isPosition()) - continue; - if (WebAssembly::isChild(*std::prev(I), MFI)) - AfterSet.insert(&*std::prev(I)); - else - break; - } - // If Header unwinds to MBB (= Header contains 'invoke'), the try block should // contain the call within it. So the call should go after the TRY. The // exception is when the header's terminator is a rethrow instruction, in // which case that instruction, not a call instruction before it, is gonna // throw. + MachineInstr *ThrowingCall = nullptr; if (MBB.isPredecessor(Header)) { auto TermPos = Header->getFirstTerminator(); if (TermPos == Header->end() || TermPos->getOpcode() != WebAssembly::RETHROW) { - for (const auto &MI : reverse(*Header)) { + for (auto &MI : reverse(*Header)) { if (MI.isCall()) { AfterSet.insert(&MI); + ThrowingCall = &MI; // Possibly throwing calls are usually wrapped by EH_LABEL // instructions. We don't want to split them and the call. if (MI.getIterator() != Header->begin() && - std::prev(MI.getIterator())->isEHLabel()) + std::prev(MI.getIterator())->isEHLabel()) { AfterSet.insert(&*std::prev(MI.getIterator())); + ThrowingCall = &*std::prev(MI.getIterator()); + } break; } } } } + // Local expression tree should go after the TRY. + // For BLOCK placement, we start the search from the previous instruction of a + // BB's terminator, but in TRY's case, we should start from the previous + // instruction of a call that can throw, or a EH_LABEL that precedes the call, + // because the return values of the call's previous instructions can be + // stackified and consumed by the throwing call. + auto SearchStartPt = ThrowingCall ? MachineBasicBlock::iterator(ThrowingCall) + : Header->getFirstTerminator(); + for (auto I = SearchStartPt, E = Header->begin(); I != E; --I) { + if (std::prev(I)->isDebugInstr() || std::prev(I)->isPosition()) + continue; + if (WebAssembly::isChild(*std::prev(I), MFI)) + AfterSet.insert(&*std::prev(I)); + else + break; + } + // Add the TRY. auto InsertPos = getLatestInsertPos(Header, BeforeSet, AfterSet); MachineInstr *Begin = diff --git a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll index 9d75aaa..858a3b7 100644 --- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll +++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll @@ -704,14 +704,41 @@ ehcleanup: ; preds = %entry cleanupret from %0 unwind to caller } +; Tests if 'try' marker is placed correctly. In this test, 'try' should be +; placed before the call to 'nothrow_i32' and not between the call to +; 'nothrow_i32' and 'fun', because the return value of 'nothrow_i32' is +; stackified and pushed onto the stack to be consumed by the call to 'fun'. + +; CHECK-LABEL: test11 +; CHECK: try +; CHECK: i32.call $push{{.*}}=, nothrow_i32 +; CHECK: call fun, $pop{{.*}} +define void @test11() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) { +entry: + %call = call i32 @nothrow_i32() + invoke void @fun(i32 %call) + to label %invoke.cont unwind label %terminate + +invoke.cont: ; preds = %entry + ret void + +terminate: ; preds = %entry + %0 = cleanuppad within none [] + %1 = tail call i8* @llvm.wasm.get.exception(token %0) + call void @__clang_call_terminate(i8* %1) [ "funclet"(token %0) ] + unreachable +} + ; Check if the unwind destination mismatch stats are correct ; NOSORT-STAT: 11 wasm-cfg-stackify - Number of EH pad unwind mismatches found declare void @foo() declare void @bar() declare i32 @baz() +declare void @fun(i32) ; Function Attrs: nounwind declare void @nothrow(i32) #0 +declare i32 @nothrow_i32() #0 ; Function Attrs: nounwind declare %class.Object* @_ZN6ObjectD2Ev(%class.Object* returned) #0 declare i32 @__gxx_wasm_personality_v0(...)