diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp index 989c3da10911..bc1c336f52ba 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp @@ -277,11 +277,19 @@ void WebAssemblyCFGStackify::placeBlockMarker(MachineBasicBlock &MBB) { #endif } - // All previously inserted BLOCK/TRY markers should be after the BLOCK - // because they are all nested blocks. + // If there is a previously placed BLOCK/TRY marker and its corresponding + // END marker is before the current BLOCK's END marker, that should be + // placed after this BLOCK. Otherwise it should be placed before this BLOCK + // marker. if (MI.getOpcode() == WebAssembly::BLOCK || - MI.getOpcode() == WebAssembly::TRY) - AfterSet.insert(&MI); + MI.getOpcode() == WebAssembly::TRY) { + if (BeginToEnd[&MI]->getParent()->getNumber() <= MBB.getNumber()) + AfterSet.insert(&MI); +#ifndef NDEBUG + else + BeforeSet.insert(&MI); +#endif + } #ifndef NDEBUG // All END_(BLOCK|LOOP|TRY) markers should be before the BLOCK. @@ -866,6 +874,10 @@ bool WebAssemblyCFGStackify::fixUnwindMismatches(MachineFunction &MF) { // In new CFG, DenseMap BrDestToExnReg; + // Destinations for branches that will be newly added, for which a new + // BLOCK/END_BLOCK markers are necessary. + SmallVector BrDests; + // Gather possibly throwing calls (i.e., previously invokes) whose current // unwind destination is not the same as the original CFG. for (auto &MBB : reverse(MF)) { @@ -1075,6 +1087,7 @@ bool WebAssemblyCFGStackify::fixUnwindMismatches(MachineFunction &MF) { ? DebugLoc() : EHPadLayoutPred->rbegin()->getDebugLoc(); BuildMI(EHPadLayoutPred, DL, TII.get(WebAssembly::BR)).addMBB(Cont); + BrDests.push_back(Cont); } } @@ -1178,8 +1191,16 @@ bool WebAssemblyCFGStackify::fixUnwindMismatches(MachineFunction &MF) { // Fix predecessor-successor relationship. NestedCont->transferSuccessors(MBB); - if (EHPad) + if (EHPad) { NestedCont->removeSuccessor(EHPad); + // If EHPad does not have any predecessors left after removing + // NextedCont predecessor, remove its successor too, because this EHPad + // is not reachable from the entry BB anyway. We can't remove EHPad BB + // itself because it can contain 'catch' or 'end', which are necessary + // for keeping try-catch-end structure. + if (EHPad->pred_empty()) + EHPad->removeSuccessor(BrDest); + } MBB->addSuccessor(NestedEHPad); MBB->addSuccessor(NestedCont); NestedEHPad->addSuccessor(BrDest); @@ -1211,10 +1232,14 @@ bool WebAssemblyCFGStackify::fixUnwindMismatches(MachineFunction &MF) { // Recompute the dominator tree. getAnalysis().runOnMachineFunction(MF); - // Place block markers for newly added branches. - SmallVector BrDests; - for (auto &P : BrDestToTryRanges) - BrDests.push_back(P.first); + // Place block markers for newly added branches, if necessary. + + // If we've created an appendix BB and a branch to it, place a block/end_block + // marker for that. For some new branches, those branch destination BBs start + // with a hoisted end_try marker, so we don't need a new marker there. + if (AppendixBB) + BrDests.push_back(AppendixBB); + llvm::sort(BrDests, [&](const MachineBasicBlock *A, const MachineBasicBlock *B) { auto ANum = A->getNumber(); diff --git a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll index 5cbff5d0bb62..188ad22c89fc 100644 --- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll +++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll @@ -850,8 +850,51 @@ terminate7: ; preds = %ehcleanup unreachable } +; We don't need to call placeBlockMarker after fixUnwindMismatches unless the +; destination is the appendix BB at the very end. This should not crash. +define void @test16(i32* %p, i32 %a, i32 %b) personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) { +entry: + br label %loop + +loop: + invoke void @foo() + to label %bb0 unwind label %catch.dispatch0 + +bb0: + %cmp = icmp ne i32 %a, %b + br i1 %cmp, label %bb1, label %last + +bb1: ; preds = %bb0 + invoke void @bar() + to label %try.cont unwind label %catch.dispatch1 + +catch.dispatch0: ; preds = %loop + %0 = catchswitch within none [label %catch.start0] unwind to caller + +catch.start0: ; preds = %catch.dispatch0 + %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 + +catch.dispatch1: ; preds = %bb1 + %4 = catchswitch within none [label %catch.start1] unwind to caller + +catch.start1: ; preds = %catch.dispatch1 + %5 = catchpad within %4 [i8* null] + %6 = call i8* @llvm.wasm.get.exception(token %5) + %7 = call i32 @llvm.wasm.get.ehselector(token %5) + catchret from %5 to label %try.cont + +try.cont: ; preds = %catch.start, %loop + br label %loop + +last: + ret void +} + ; Check if the unwind destination mismatch stats are correct -; NOSORT-STAT: 15 wasm-cfg-stackify - Number of EH pad unwind mismatches found +; NOSORT-STAT: 16 wasm-cfg-stackify - Number of EH pad unwind mismatches found declare void @foo() declare void @bar()