[WebAssembly] Split BBs after throw instructions

Summary:
`throw` instruction is a terminator in wasm, but BBs were not splitted
after `throw` instructions, causing machine instruction verifier to
fail.

This patch
- Splits BBs after `throw` instructions in WasmEHPrepare and adding an
  unreachable instruction after `throw`, which will be deleted in
  LateEHPrepare pass
- Refactors WasmEHPrepare into two member functions
- Changes the semantics of `eraseBBsAndChildren` in LateEHPrepare pass
  to match that of WasmEHPrepare pass, which is newly added. Now
  `eraseBBsAndChildren` does not delete BBs with remaining predecessors.
- Fixes style nits, making static function names conform to clang-tidy
- Re-enables the test temporarily disabled by rL346840 && rL346845

Reviewers: dschuff

Subscribers: sbc100, jgravelle-google, sunfish, llvm-commits

Differential Revision: https://reviews.llvm.org/D54571

llvm-svn: 347003

5 files changed, 140 insertions, 26 deletions

diff --git a/llvm/lib/CodeGen/WasmEHPrepare.cpp b/llvm/lib/CodeGen/WasmEHPrepare.cpp
index 6f02a05f561..e5002eb9534 100644
--- a/llvm/lib/CodeGen/WasmEHPrepare.cpp
+++ b/llvm/lib/CodeGen/WasmEHPrepare.cpp
@@ -137,6 +137,7 @@ class WasmEHPrepare : public FunctionPass {
   Value *LSDAField = nullptr;      // lsda field
   Value *SelectorField = nullptr;  // selector
 
+  Function *ThrowF = nullptr;           // wasm.throw() intrinsic
   Function *CatchF = nullptr;           // wasm.catch.extract() intrinsic
   Function *LPadIndexF = nullptr;       // wasm.landingpad.index() intrinsic
   Function *LSDAF = nullptr;            // wasm.lsda() intrinsic
@@ -145,6 +146,9 @@ class WasmEHPrepare : public FunctionPass {
   Function *CallPersonalityF = nullptr; // _Unwind_CallPersonality() wrapper
   Function *ClangCallTermF = nullptr;   // __clang_call_terminate() function
 
+  bool prepareEHPads(Function &F);
+  bool prepareThrows(Function &F);
+
   void prepareEHPad(BasicBlock *BB, unsigned Index);
   void prepareTerminateCleanupPad(BasicBlock *BB);
 
@@ -177,7 +181,62 @@ bool WasmEHPrepare::doInitialization(Module &M) {
   return false;
 }
 
+// Erase the specified BBs if the BB does not have any remaining predecessors,
+// and also all its dead children.
+template <typename Container>
+static void eraseDeadBBsAndChildren(const Container &BBs) {
+  SmallVector<BasicBlock *, 8> WL(BBs.begin(), BBs.end());
+  while (!WL.empty()) {
+    auto *BB = WL.pop_back_val();
+    if (pred_begin(BB) != pred_end(BB))
+      continue;
+    WL.append(succ_begin(BB), succ_end(BB));
+    DeleteDeadBlock(BB);
+  }
+}
+
 bool WasmEHPrepare::runOnFunction(Function &F) {
+  bool Changed = false;
+  Changed |= prepareThrows(F);
+  Changed |= prepareEHPads(F);
+  return Changed;
+}
+
+bool WasmEHPrepare::prepareThrows(Function &F) {
+  Module &M = *F.getParent();
+  IRBuilder<> IRB(F.getContext());
+  bool Changed = false;
+
+  // wasm.throw() intinsic, which will be lowered to wasm 'throw' instruction.
+  ThrowF = Intrinsic::getDeclaration(&M, Intrinsic::wasm_throw);
+
+  // Insert an unreachable instruction after a call to @llvm.wasm.throw and
+  // delete all following instructions within the BB, and delete all the dead
+  // children of the BB as well.
+  for (User *U : ThrowF->users()) {
+    // A call to @llvm.wasm.throw() is only generated from
+    // __builtin_wasm_throw() builtin call within libcxxabi, and cannot be an
+    // InvokeInst.
+    auto *ThrowI = cast<CallInst>(U);
+    if (ThrowI->getFunction() != &F)
+      continue;
+    Changed = true;
+    auto *BB = ThrowI->getParent();
+    SmallVector<BasicBlock *, 4> Succs(succ_begin(BB), succ_end(BB));
+    auto &InstList = BB->getInstList();
+    InstList.erase(std::next(BasicBlock::iterator(ThrowI)), InstList.end());
+    IRB.SetInsertPoint(BB);
+    IRB.CreateUnreachable();
+    eraseDeadBBsAndChildren(Succs);
+  }
+
+  return Changed;
+}
+
+bool WasmEHPrepare::prepareEHPads(Function &F) {
+  Module &M = *F.getParent();
+  IRBuilder<> IRB(F.getContext());
+
   SmallVector<BasicBlock *, 16> CatchPads;
   SmallVector<BasicBlock *, 16> CleanupPads;
   for (BasicBlock &BB : F) {
@@ -194,9 +253,6 @@ bool WasmEHPrepare::runOnFunction(Function &F) {
     return false;
   assert(F.hasPersonalityFn() && "Personality function not found");
 
-  Module &M = *F.getParent();
-  IRBuilder<> IRB(F.getContext());
-
   // __wasm_lpad_context global variable
   LPadContextGV = cast<GlobalVariable>(
       M.getOrInsertGlobal("__wasm_lpad_context", LPadContextTy));
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
index 98953f09482..871e92082a4 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp
@@ -31,6 +31,7 @@ class WebAssemblyLateEHPrepare final : public MachineFunctionPass {
 
   bool runOnMachineFunction(MachineFunction &MF) override;
 
+  bool removeUnnecessaryUnreachables(MachineFunction &MF);
   bool replaceFuncletReturns(MachineFunction &MF);
   bool hoistCatches(MachineFunction &MF);
   bool addCatchAlls(MachineFunction &MF);
@@ -59,7 +60,7 @@ FunctionPass *llvm::createWebAssemblyLateEHPrepare() {
 // possible search paths should be the same.
 // Returns nullptr in case it does not find any EH pad in the search, or finds
 // multiple different EH pads.
-static MachineBasicBlock *GetMatchingEHPad(MachineInstr *MI) {
+static MachineBasicBlock *getMatchingEHPad(MachineInstr *MI) {
   MachineFunction *MF = MI->getParent()->getParent();
   SmallVector<MachineBasicBlock *, 2> WL;
   SmallPtrSet<MachineBasicBlock *, 2> Visited;
@@ -83,18 +84,15 @@ static MachineBasicBlock *GetMatchingEHPad(MachineInstr *MI) {
   return EHPad;
 }
 
-// Erases the given BBs and all their children from the function. If other BBs
-// have the BB as a successor, the successor relationships will be deleted as
-// well.
+// Erase the specified BBs if the BB does not have any remaining predecessors,
+// and also all its dead children.
 template <typename Container>
-static void EraseBBsAndChildren(const Container &MBBs) {
+static void eraseDeadBBsAndChildren(const Container &MBBs) {
   SmallVector<MachineBasicBlock *, 8> WL(MBBs.begin(), MBBs.end());
   while (!WL.empty()) {
     MachineBasicBlock *MBB = WL.pop_back_val();
-    SmallVector<MachineBasicBlock *, 4> Preds(MBB->pred_begin(),
-                                              MBB->pred_end());
-    for (auto *Pred : Preds)
-      Pred->removeSuccessor(MBB);
+    if (!MBB->pred_empty())
+      continue;
     SmallVector<MachineBasicBlock *, 4> Succs(MBB->succ_begin(),
                                               MBB->succ_end());
     WL.append(MBB->succ_begin(), MBB->succ_end());
@@ -110,6 +108,7 @@ bool WebAssemblyLateEHPrepare::runOnMachineFunction(MachineFunction &MF) {
     return false;
 
   bool Changed = false;
+  Changed |= removeUnnecessaryUnreachables(MF);
   Changed |= addRethrows(MF);
   if (!MF.getFunction().hasPersonalityFn())
     return Changed;
@@ -122,6 +121,31 @@ bool WebAssemblyLateEHPrepare::runOnMachineFunction(MachineFunction &MF) {
   return Changed;
 }
 
+bool WebAssemblyLateEHPrepare::removeUnnecessaryUnreachables(
+    MachineFunction &MF) {
+  bool Changed = false;
+  for (auto &MBB : MF) {
+    for (auto &MI : MBB) {
+      if (!WebAssembly::isThrow(MI))
+        continue;
+      Changed = true;
+
+      // The instruction after the throw should be an unreachable or a branch to
+      // another BB that should eventually lead to an unreachable. Delete it
+      // because throw itself is a terminator, and also delete successors if
+      // any.
+      MBB.erase(std::next(MachineBasicBlock::iterator(MI)), MBB.end());
+      SmallVector<MachineBasicBlock *, 8> Succs(MBB.succ_begin(),
+                                                MBB.succ_end());
+      for (auto *Succ : Succs)
+        MBB.removeSuccessor(Succ);
+      eraseDeadBBsAndChildren(Succs);
+    }
+  }
+
+  return Changed;
+}
+
 bool WebAssemblyLateEHPrepare::replaceFuncletReturns(MachineFunction &MF) {
   bool Changed = false;
   const auto &TII = *MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
@@ -183,7 +207,7 @@ bool WebAssemblyLateEHPrepare::hoistCatches(MachineFunction &MF) {
         Catches.push_back(&MI);
 
   for (auto *Catch : Catches) {
-    MachineBasicBlock *EHPad = GetMatchingEHPad(Catch);
+    MachineBasicBlock *EHPad = getMatchingEHPad(Catch);
     assert(EHPad && "No matching EH pad for catch");
     if (EHPad->begin() == Catch)
       continue;
@@ -242,7 +266,7 @@ bool WebAssemblyLateEHPrepare::addRethrows(MachineFunction &MF) {
         Rethrow = BuildMI(MBB, InsertPt, MI.getDebugLoc(),
                           TII.get(WebAssembly::RETHROW_TO_CALLER));
 
-      // Becasue __cxa_rethrow does not return, the instruction after the
+      // Because __cxa_rethrow does not return, the instruction after the
       // rethrow should be an unreachable or a branch to another BB that should
       // eventually lead to an unreachable. Delete it because rethrow itself is
       // a terminator, and also delete non-EH pad successors if any.
@@ -251,7 +275,9 @@ bool WebAssemblyLateEHPrepare::addRethrows(MachineFunction &MF) {
       for (auto *Succ : MBB.successors())
         if (!Succ->isEHPad())
           NonPadSuccessors.push_back(Succ);
-      EraseBBsAndChildren(NonPadSuccessors);
+      for (auto *Succ : NonPadSuccessors)
+        MBB.removeSuccessor(Succ);
+      eraseDeadBBsAndChildren(NonPadSuccessors);
     }
   return Changed;
 }
@@ -283,7 +309,7 @@ bool WebAssemblyLateEHPrepare::ensureSingleBBTermPads(MachineFunction &MF) {
 
   bool Changed = false;
   for (auto *Call : ClangCallTerminateCalls) {
-    MachineBasicBlock *EHPad = GetMatchingEHPad(Call);
+    MachineBasicBlock *EHPad = getMatchingEHPad(Call);
     assert(EHPad && "No matching EH pad for catch");
 
     // If it is already the form we want, skip it
@@ -308,7 +334,11 @@ bool WebAssemblyLateEHPrepare::ensureSingleBBTermPads(MachineFunction &MF) {
     BuildMI(*EHPad, InsertPos, Call->getDebugLoc(),
             TII.get(WebAssembly::UNREACHABLE));
     EHPad->erase(InsertPos, EHPad->end());
-    EraseBBsAndChildren(EHPad->successors());
+    SmallVector<MachineBasicBlock *, 8> Succs(EHPad->succ_begin(),
+                                              EHPad->succ_end());
+    for (auto *Succ : Succs)
+      EHPad->removeSuccessor(Succ);
+    eraseDeadBBsAndChildren(Succs);
   }
   return Changed;
 }
diff --git a/llvm/test/CodeGen/WebAssembly/exception.ll b/llvm/test/CodeGen/WebAssembly/exception.ll
index 0577cd43b97..b53cd0cb544 100644
--- a/llvm/test/CodeGen/WebAssembly/exception.ll
+++ b/llvm/test/CodeGen/WebAssembly/exception.ll
@@ -1,5 +1,5 @@
 ; RUN: not llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -exception-model=wasm
-; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -exception-model=wasm -mattr=+exception-handling | FileCheck -allow-deprecated-dag-overlap %s
+; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -exception-model=wasm -mattr=+exception-handling -verify-machineinstrs | FileCheck -allow-deprecated-dag-overlap %s
 ; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-keep-registers -exception-model=wasm -mattr=+exception-handling
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
@@ -12,10 +12,11 @@ target triple = "wasm32-unknown-unknown"
 declare void @llvm.wasm.throw(i32, i8*)
 
 ; CHECK-LABEL: test_throw:
-; CHECK-NEXT: i32.const $push0=, 0
+; CHECK:      get_local $push0=, 0
 ; CHECK-NEXT: throw __cpp_exception@EVENT, $pop0
-define void @test_throw() {
-  call void @llvm.wasm.throw(i32 0, i8* null)
+; CHECK-NOT:  unreachable
+define void @test_throw(i8* %p) {
+  call void @llvm.wasm.throw(i32 0, i8* %p)
   ret void
 }
 
diff --git a/llvm/test/CodeGen/WebAssembly/wasmehprepare.ll b/llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
index 67e198eb058..6df7175b1ca 100644
--- a/llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
+++ b/llvm/test/CodeGen/WebAssembly/wasmehprepare.ll
@@ -298,6 +298,34 @@ try.cont10:                                       ; preds = %invoke.cont3, %catc
   ret void
 }
 
+; Tests if instructions after a call to @llvm.wasm.throw are deleted and the
+; BB's dead children are deleted.
+
+; CHECK-LABEL: @test6
+define i32 @test6(i1 %b, i8* %p) {
+entry:
+  br i1 %b, label %bb.true, label %bb.false
+
+; CHECK:      bb.true:
+; CHECK-NEXT:   call void @llvm.wasm.throw(i32 0, i8* %p)
+; CHECK-NEXT:   unreachable
+bb.true:                                          ; preds = %entry
+  call void @llvm.wasm.throw(i32 0, i8* %p)
+  br label %bb.true.0
+
+; CHECK-NOT:  bb.true.0
+bb.true.0:                                        ; preds = %bb.true
+  br label %merge
+
+; CHECK:      bb.false
+bb.false:                                         ; preds = %entry
+  br label %merge
+
+; CHECK:      merge
+merge:                                            ; preds = %bb.true.0, %bb.false
+  ret i32 0
+}
+
 declare void @foo()
 declare void @func(i32)
 declare %struct.Cleanup* @_ZN7CleanupD1Ev(%struct.Cleanup* returned)
@@ -305,6 +333,7 @@ declare i32 @__gxx_wasm_personality_v0(...)
 declare i8* @llvm.wasm.get.exception(token)
 declare i32 @llvm.wasm.get.ehselector(token)
 declare i32 @llvm.eh.typeid.for(i8*)
+declare void @llvm.wasm.throw(i32, i8*)
 declare i8* @__cxa_begin_catch(i8*)
 declare void @__cxa_end_catch()
 declare void @__cxa_rethrow()
@@ -314,4 +343,3 @@ declare void @__clang_call_terminate(i8*)
 ; CHECK-DAG: declare void @llvm.wasm.landingpad.index(token, i32)
 ; CHECK-DAG: declare i8* @llvm.wasm.lsda()
 ; CHECK-DAG: declare i32 @_Unwind_CallPersonality(i8*)
-
diff --git a/llvm/test/MC/WebAssembly/event-section.ll b/llvm/test/MC/WebAssembly/event-section.ll
index 1257196240f..59cfe113280 100644
--- a/llvm/test/MC/WebAssembly/event-section.ll
+++ b/llvm/test/MC/WebAssembly/event-section.ll
@@ -1,6 +1,5 @@
-; XFAIL: *
-; RUN: llc -filetype=obj -exception-model=wasm -mattr=+exception-handling -verify-machineinstrs %s -o - | obj2yaml | FileCheck %s
-; RUN: llc -filetype=obj -exception-model=wasm -mattr=+exception-handling -verify-machineinstrs %s -o - | llvm-readobj -s | FileCheck -check-prefix=SEC %s
+; RUN: llc -filetype=obj -exception-model=wasm -mattr=+exception-handling %s -o - | obj2yaml | FileCheck %s
+; RUN: llc -filetype=obj -exception-model=wasm -mattr=+exception-handling %s -o - | llvm-readobj -s | FileCheck -check-prefix=SEC %s
 
 target triple = "wasm32-unknown-unknown"
 
@@ -41,7 +40,7 @@ define i32 @test_throw1(i8* %p) {
 ; CHECK-NEXT:         Offset:          0x00000006
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_EVENT_INDEX_LEB
 ; CHECK-NEXT:         Index:           1
-; CHECK-NEXT:         Offset:          0x00000013
+; CHECK-NEXT:         Offset:          0x00000011
 
 ; CHECK:        - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking