diff --git a/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp index 65cc088cd27..b1a47955726 100644 --- a/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp +++ b/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp @@ -193,10 +193,7 @@ void WebAssemblyCFGStackify::unregisterScope(MachineInstr *Begin) { /// Insert a BLOCK marker for branches to MBB (if needed). void WebAssemblyCFGStackify::placeBlockMarker(MachineBasicBlock &MBB) { - // This should have been handled in placeTryMarker. - if (MBB.isEHPad()) - return; - + assert(!MBB.isEHPad()); MachineFunction &MF = *MBB.getParent(); auto &MDT = getAnalysis(); const auto &TII = *MF.getSubtarget().getInstrInfo(); @@ -253,14 +250,12 @@ void WebAssemblyCFGStackify::placeBlockMarker(MachineBasicBlock &MBB) { // Instructions that should go after the BLOCK. SmallPtrSet AfterSet; for (const auto &MI : *Header) { - // If there is a previously placed LOOP/TRY marker and the bottom block of - // the loop/exception is above MBB, it should be after the BLOCK, because - // the loop/exception is nested in this block. Otherwise it should be before - // the BLOCK. - if (MI.getOpcode() == WebAssembly::LOOP || - MI.getOpcode() == WebAssembly::TRY) { - auto *BottomBB = BeginToEnd[&MI]->getParent()->getPrevNode(); - if (MBB.getNumber() > BottomBB->getNumber()) + // If there is a previously placed LOOP marker and the bottom block of the + // loop is above MBB, it should be after the BLOCK, because the loop is + // nested in this BLOCK. Otherwise it should be before the BLOCK. + if (MI.getOpcode() == WebAssembly::LOOP) { + auto *LoopBottom = BeginToEnd[&MI]->getParent()->getPrevNode(); + if (MBB.getNumber() > LoopBottom->getNumber()) AfterSet.insert(&MI); #ifndef NDEBUG else @@ -268,9 +263,10 @@ void WebAssemblyCFGStackify::placeBlockMarker(MachineBasicBlock &MBB) { #endif } - // All previously inserted BLOCK markers should be after the BLOCK because - // they are all nested blocks. - if (MI.getOpcode() == WebAssembly::BLOCK) + // All previously inserted BLOCK/TRY markers should be after the BLOCK + // because they are all nested blocks. + if (MI.getOpcode() == WebAssembly::BLOCK || + MI.getOpcode() == WebAssembly::TRY) AfterSet.insert(&MI); #ifndef NDEBUG @@ -428,9 +424,7 @@ void WebAssemblyCFGStackify::placeLoopMarker(MachineBasicBlock &MBB) { } void WebAssemblyCFGStackify::placeTryMarker(MachineBasicBlock &MBB) { - if (!MBB.isEHPad()) - return; - + assert(MBB.isEHPad()); MachineFunction &MF = *MBB.getParent(); auto &MDT = getAnalysis(); const auto &TII = *MF.getSubtarget().getInstrInfo(); @@ -486,16 +480,17 @@ void WebAssemblyCFGStackify::placeTryMarker(MachineBasicBlock &MBB) { // Decide where in Header to put the TRY. - // Instructions that should go before the BLOCK. + // Instructions that should go before the TRY. SmallPtrSet BeforeSet; - // Instructions that should go after the BLOCK. + // Instructions that should go after the TRY. SmallPtrSet AfterSet; for (const auto &MI : *Header) { - // If there is a previously placed LOOP marker and the bottom block of - // the loop is above MBB, the LOOP should be after the TRY, because the - // loop is nested in this try. Otherwise it should be before the TRY. + // If there is a previously placed LOOP marker and the bottom block of the + // loop is above MBB, it should be after the TRY, because the loop is nested + // in this TRY. Otherwise it should be before the TRY. if (MI.getOpcode() == WebAssembly::LOOP) { - if (MBB.getNumber() > Bottom->getNumber()) + auto *LoopBottom = BeginToEnd[&MI]->getParent()->getPrevNode(); + if (MBB.getNumber() > LoopBottom->getNumber()) AfterSet.insert(&MI); #ifndef NDEBUG else @@ -503,14 +498,16 @@ void WebAssemblyCFGStackify::placeTryMarker(MachineBasicBlock &MBB) { #endif } - // All previously inserted TRY markers should be after the TRY because they - // are all nested trys. - if (MI.getOpcode() == WebAssembly::TRY) + // All previously inserted BLOCK/TRY markers should be after the TRY because + // they are all nested trys. + if (MI.getOpcode() == WebAssembly::BLOCK || + MI.getOpcode() == WebAssembly::TRY) AfterSet.insert(&MI); #ifndef NDEBUG - // All END_(LOOP/TRY) markers should be before the TRY. - if (MI.getOpcode() == WebAssembly::END_LOOP || + // All END_(BLOCK/LOOP/TRY) markers should be before the TRY. + if (MI.getOpcode() == WebAssembly::END_BLOCK || + MI.getOpcode() == WebAssembly::END_LOOP || MI.getOpcode() == WebAssembly::END_TRY) BeforeSet.insert(&MI); #endif @@ -566,8 +563,9 @@ void WebAssemblyCFGStackify::placeTryMarker(MachineBasicBlock &MBB) { AfterSet.clear(); for (const auto &MI : *Cont) { #ifndef NDEBUG - // END_TRY should precede existing LOOP markers. - if (MI.getOpcode() == WebAssembly::LOOP) + // END_TRY should precede existing LOOP and BLOCK markers. + if (MI.getOpcode() == WebAssembly::LOOP || + MI.getOpcode() == WebAssembly::BLOCK) AfterSet.insert(&MI); // All END_TRY markers placed earlier belong to exceptions that contains @@ -588,6 +586,8 @@ void WebAssemblyCFGStackify::placeTryMarker(MachineBasicBlock &MBB) { AfterSet.insert(&MI); #endif } + + // It is not possible for an END_BLOCK to be already in this block. } // Mark the end of the TRY. @@ -774,15 +774,19 @@ void WebAssemblyCFGStackify::placeMarkers(MachineFunction &MF) { // Place the LOOP for MBB if MBB is the header of a loop. for (auto &MBB : MF) placeLoopMarker(MBB); - // Place the TRY for MBB if MBB is the EH pad of an exception. + const MCAsmInfo *MCAI = MF.getTarget().getMCAsmInfo(); - if (MCAI->getExceptionHandlingType() == ExceptionHandling::Wasm && - MF.getFunction().hasPersonalityFn()) - for (auto &MBB : MF) - placeTryMarker(MBB); - // Place the BLOCK for MBB if MBB is branched to from above. - for (auto &MBB : MF) - placeBlockMarker(MBB); + for (auto &MBB : MF) { + if (MBB.isEHPad()) { + // Place the TRY for MBB if MBB is the EH pad of an exception. + if (MCAI->getExceptionHandlingType() == ExceptionHandling::Wasm && + MF.getFunction().hasPersonalityFn()) + placeTryMarker(MBB); + } else { + // Place the BLOCK for MBB if MBB is branched to from above. + placeBlockMarker(MBB); + } + } } void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) { diff --git a/test/CodeGen/WebAssembly/cfg-stackify-eh.ll b/test/CodeGen/WebAssembly/cfg-stackify-eh.ll index 5b06a0baa35..f0f49344cfc 100644 --- a/test/CodeGen/WebAssembly/cfg-stackify-eh.ll +++ b/test/CodeGen/WebAssembly/cfg-stackify-eh.ll @@ -1,4 +1,5 @@ ; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -exception-model=wasm -mattr=+exception-handling | FileCheck %s +; RUN: llc < %s -O0 -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -verify-machineinstrs -exception-model=wasm -mattr=+exception-handling | FileCheck %s --check-prefix=NOOPT target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128" target triple = "wasm32-unknown-unknown" @@ -70,7 +71,7 @@ rethrow: ; preds = %catch.fallthrough call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ] unreachable -try.cont: ; preds = %entry, %catch, %catch2 +try.cont: ; preds = %catch, %catch2, %entry ret void } @@ -173,7 +174,7 @@ rethrow5: ; preds = %catch.start3 invoke void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %9) ] to label %unreachable unwind label %ehcleanup9 -try.cont: ; preds = %catch, %invoke.cont8 +try.cont: ; preds = %invoke.cont8, %catch call void @__cxa_end_catch() [ "funclet"(token %1) ] catchret from %1 to label %try.cont11 @@ -181,7 +182,7 @@ rethrow: ; preds = %catch.start call void @llvm.wasm.rethrow.in.catch() [ "funclet"(token %1) ] unreachable -try.cont11: ; preds = %entry, %try.cont +try.cont11: ; preds = %try.cont, %entry ret void ehcleanup: ; preds = %catch6 @@ -286,6 +287,54 @@ terminate: ; preds = %ehcleanup unreachable } +; Tests if block and try markers are correctly placed. Even if two predecessors +; of the EH pad are bb2 and bb3 and their nearest common dominator is bb1, the +; TRY marker should be placed at bb0 because there's a branch from bb0 to bb2, +; and scopes cannot be interleaved. + +; NOOPT-LABEL: test3 +; NOOPT: try +; NOOPT: block +; NOOPT: block +; NOOPT: block +; NOOPT: end_block +; NOOPT: end_block +; NOOPT: call foo +; NOOPT: end_block +; NOOPT: call bar +; NOOPT: catch {{.*}} +; NOOPT: end_try +define void @test3() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) { +bb0: + br i1 undef, label %bb1, label %bb2 + +bb1: ; preds = %bb0 + br i1 undef, label %bb3, label %bb4 + +bb2: ; preds = %bb0 + br label %try.cont + +bb3: ; preds = %bb1 + invoke void @foo() + to label %try.cont unwind label %catch.dispatch + +bb4: ; preds = %bb1 + invoke void @bar() + to label %try.cont unwind label %catch.dispatch + +catch.dispatch: ; preds = %bb4, %bb3 + %0 = catchswitch within none [label %catch.start] unwind to caller + +catch.start: ; preds = %catch.dispatch + %1 = catchpad within %0 [i8* null] + %2 = call i8* @llvm.wasm.get.exception(token %1) + %3 = call i32 @llvm.wasm.get.ehselector(token %1) + catchret from %1 to label %try.cont + +try.cont: ; preds = %catch.start, %bb4, %bb3, %bb2 + ret void +} + declare void @foo() declare void @bar() declare i32 @__gxx_wasm_personality_v0(...)