Reland "[BasicBlockUtils] Handle funclets when detaching EH pad blocks"

llvm/lib/Transforms/Utils/BasicBlockUtils.cpp

@@ -58,37 +58,77 @@ static cl::opt<unsigned> MaxDeoptOrUnreachableSuccessorCheckDepth(
              "is followed by a block that either has a terminating "
              "deoptimizing call or is terminated with an unreachable"));
 
-void llvm::detachDeadBlocks(
-    ArrayRef<BasicBlock *> BBs,
-    SmallVectorImpl<DominatorTree::UpdateType> *Updates,
+static void zapAllInstructionInDeadBasicBlock(BasicBlock *BB) {
+  while (!BB->empty()) {
+    Instruction &I = BB->back();
+    // If this instruction is used, replace uses with an arbitrary value.
+    // Because control flow can't get here, we don't care what we replace the
+    // value with. Note that since this block is unreachable, and all values
+    // contained within it must dominate their uses, that all uses will
+    // eventually be removed (they are themselves dead).
+    if (!I.use_empty())
+      I.replaceAllUsesWith(PoisonValue::get(I.getType()));
+    BB->back().eraseFromParent();
+  }
+  new UnreachableInst(BB->getContext(), BB);
+  assert(BB->size() == 1 && isa<UnreachableInst>(BB->getTerminator()) &&
+         "The successor list of BB isn't empty before "
+         "applying corresponding DTU updates.");
+}
+
+static void deleteBasicBlockFromSuccessor(
+    BasicBlock *BB, SmallVectorImpl<DominatorTree::UpdateType> *Updates,
     bool KeepOneInputPHIs) {
+  SmallPtrSet<BasicBlock *, 4> UniqueSuccessors;
+  for (BasicBlock *Succ : successors(BB)) {
+    Succ->removePredecessor(BB, KeepOneInputPHIs);
+    if (Updates && UniqueSuccessors.insert(Succ).second)
+      Updates->push_back({DominatorTree::Delete, BB, Succ});
+  }
+}
+
+static void replaceFuncletPadsRetWithUnreachable(
+    Instruction &I, SmallVectorImpl<DominatorTree::UpdateType> *Updates,
+    bool KeepOneInputPHIs) {
+  assert(isa<FuncletPadInst>(I) && "Instruction must be a funclet pad!");
+  for (User *User : make_early_inc_range(I.users())) {
+    Instruction *ReturnInstr = dyn_cast<Instruction>(User);
+    // If we have a cleanupret or catchret block, replace it with just an
+    // unreachable.
+    if (isa<CatchReturnInst>(ReturnInstr) ||
+        isa<CleanupReturnInst>(ReturnInstr)) {
+      BasicBlock *ReturnInstrBB = ReturnInstr->getParent();
+      // This catchret or catchpad basic block is detached now. Let the
+      // successors know it.
+      deleteBasicBlockFromSuccessor(ReturnInstrBB, Updates, KeepOneInputPHIs);
+      zapAllInstructionInDeadBasicBlock(ReturnInstrBB);
+    }
+  }
+}
+
+void llvm::detachDeadBlocks(ArrayRef<BasicBlock *> BBs,
+                            SmallVectorImpl<DominatorTree::UpdateType> *Updates,
+                            bool KeepOneInputPHIs) {
   for (auto *BB : BBs) {
     // Loop through all of our successors and make sure they know that one
     // of their predecessors is going away.
     SmallPtrSet<BasicBlock *, 4> UniqueSuccessors;
-    for (BasicBlock *Succ : successors(BB)) {
-      Succ->removePredecessor(BB, KeepOneInputPHIs);
-      if (Updates && UniqueSuccessors.insert(Succ).second)
-        Updates->push_back({DominatorTree::Delete, BB, Succ});
+    auto NonFirstPhiIt = BB->getFirstNonPHIIt();
+    if (NonFirstPhiIt != BB->end()) {
+      Instruction &I = *NonFirstPhiIt;
+      // Exception handling funclets need to be explicitly addressed.
+      // These funclets must begin with cleanuppad or catchpad and end with
+      // cleanupred or catchret. The return instructions can be in different
+      // basic blocks than the pad instruction. If we would only delete the
+      // first block, the we would have possible cleanupret and catchret
+      // instructions with poison arguments, which wouldn't be valid.
+      if (isa<FuncletPadInst>(I))
+        replaceFuncletPadsRetWithUnreachable(I, Updates, KeepOneInputPHIs);
     }
 
-    // Zap all the instructions in the block.
-    while (!BB->empty()) {
-      Instruction &I = BB->back();
-      // If this instruction is used, replace uses with an arbitrary value.
-      // Because control flow can't get here, we don't care what we replace the
-      // value with.  Note that since this block is unreachable, and all values
-      // contained within it must dominate their uses, that all uses will
-      // eventually be removed (they are themselves dead).
-      if (!I.use_empty())
-        I.replaceAllUsesWith(PoisonValue::get(I.getType()));
-      BB->back().eraseFromParent();
-    }
-    new UnreachableInst(BB->getContext(), BB);
-    assert(BB->size() == 1 &&
-           isa<UnreachableInst>(BB->getTerminator()) &&
-           "The successor list of BB isn't empty before "
-           "applying corresponding DTU updates.");
+    // Detaching and emptying the current basic block.
+    deleteBasicBlockFromSuccessor(BB, Updates, KeepOneInputPHIs);
+    zapAllInstructionInDeadBasicBlock(BB);
   }
 }
 

llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll

@@ -0,0 +1,206 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes=simplifycfg -S < %s | FileCheck %s
+
+; cleanuppad/cleanupret
+
+define void @unreachable_cleanuppad_linear(i64 %shapes.1) personality ptr null {
+; CHECK-LABEL: define void @unreachable_cleanuppad_linear(
+; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
+; CHECK-NEXT:  [[START:.*:]]
+; CHECK-NEXT:    [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
+; CHECK-NEXT:    ret void
+;
+start:
+  %_7 = icmp ult i64 0, %shapes.1
+  ret void
+
+funclet:
+  %cleanuppad = cleanuppad within none []
+  br label %funclet_end
+
+funclet_end:
+  cleanupret from %cleanuppad unwind to caller
+}
+
+define void @unreachable_cleanuppad_multiple_predecessors(i64 %shapes.1) personality ptr null {
+; CHECK-LABEL: define void @unreachable_cleanuppad_multiple_predecessors(
+; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
+; CHECK-NEXT:  [[START:.*:]]
+; CHECK-NEXT:    [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
+; CHECK-NEXT:    ret void
+;
+start:
+  %_7 = icmp ult i64 0, %shapes.1
+  ret void
+
+funclet:
+  %cleanuppad = cleanuppad within none []
+  switch i64 %shapes.1, label %otherwise [ i64 0, label %one
+  i64 1, label %two
+  i64 42, label %three ]
+one:
+  br label %funclet_end
+
+two:
+  br label %funclet_end
+
+three:
+  br label %funclet_end
+
+otherwise:
+  br label %funclet_end
+
+funclet_end:
+  cleanupret from %cleanuppad unwind to caller
+}
+
+; catchpad/catchret
+
+define void @unreachable_catchpad_linear(i64 %shapes.1) personality ptr null {
+; CHECK-LABEL: define void @unreachable_catchpad_linear(
+; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
+; CHECK-NEXT:  [[START:.*:]]
+; CHECK-NEXT:    [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
+; CHECK-NEXT:    ret void
+;
+start:
+  %_7 = icmp ult i64 0, %shapes.1
+  ret void
+
+dispatch:
+  %cs = catchswitch within none [label %funclet] unwind to caller
+
+funclet:
+  %cleanuppad = catchpad within %cs []
+  br label %funclet_end
+
+
+funclet_end:
+  catchret from %cleanuppad to label %unreachable
+
+unreachable:
+  unreachable
+}
+
+define void @unreachable_catchpad_multiple_predecessors(i64 %shapes.1) personality ptr null {
+; CHECK-LABEL: define void @unreachable_catchpad_multiple_predecessors(
+; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
+; CHECK-NEXT:  [[START:.*:]]
+; CHECK-NEXT:    [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
+; CHECK-NEXT:    ret void
+;
+start:
+  %_7 = icmp ult i64 0, %shapes.1
+  ret void
+
+dispatch:
+  %cs = catchswitch within none [label %funclet] unwind to caller
+
+funclet:
+  %cleanuppad = catchpad within %cs []
+  switch i64 %shapes.1, label %otherwise [ i64 0, label %one
+  i64 1, label %two
+  i64 42, label %three ]
+one:
+  br label %funclet_end
+
+two:
+  br label %funclet_end
+
+three:
+  br label %funclet_end
+
+otherwise:
+  br label %funclet_end
+
+funclet_end:
+  catchret from %cleanuppad to label %unreachable
+
+unreachable:
+  unreachable
+}
+
+; Issue reproducer
+
+define void @gh148052(i64 %shapes.1) personality ptr null {
+; CHECK-LABEL: define void @gh148052(
+; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
+; CHECK-NEXT:  [[START:.*:]]
+; CHECK-NEXT:    [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
+; CHECK-NEXT:    call void @llvm.assume(i1 [[_7]])
+; CHECK-NEXT:    ret void
+;
+start:
+  %_7 = icmp ult i64 0, %shapes.1
+  br i1 %_7, label %bb1, label %panic
+
+bb1:
+  %_11 = icmp ult i64 0, %shapes.1
+  br i1 %_11, label %bb3, label %panic1
+
+panic:
+  unreachable
+
+bb3:
+  ret void
+
+panic1:
+  invoke void @func(i64 0, i64 0, ptr null)
+  to label %unreachable unwind label %funclet_bb14
+
+funclet_bb14:
+  %cleanuppad = cleanuppad within none []
+  br label %bb13
+
+unreachable:
+  unreachable
+
+bb10:
+  cleanupret from %cleanuppad5 unwind to caller
+
+funclet_bb10:
+  %cleanuppad5 = cleanuppad within none []
+  br label %bb10
+
+bb13:
+  cleanupret from %cleanuppad unwind label %funclet_bb10
+}
+
+%struct.foo = type { ptr, %struct.eggs, ptr }
+%struct.eggs = type { ptr, ptr, ptr }
+
+declare x86_thiscallcc ptr @quux(ptr, ptr, i32)
+
+define x86_thiscallcc ptr @baz(ptr %arg, ptr %arg1, ptr %arg2, i1 %arg3, ptr %arg4) personality ptr null {
+bb:
+  %alloca = alloca [2 x %struct.foo], align 4
+  %invoke = invoke x86_thiscallcc ptr @quux(ptr null, ptr null, i32 0)
+          to label %bb5 unwind label %bb10
+
+bb5:                                              ; preds = %bb
+  %getelementptr = getelementptr i8, ptr %arg, i32 20
+  %call = call x86_thiscallcc ptr null(ptr null, ptr null, i32 0)
+  br label %bb6
+
+bb6:                                              ; preds = %bb10, %bb5
+  %phi = phi ptr [ null, %bb10 ], [ null, %bb5 ]
+  ret ptr %phi
+
+bb7:                                              ; No predecessors!
+  %cleanuppad = cleanuppad within none []
+  %getelementptr8 = getelementptr i8, ptr %arg2, i32 -20
+  %icmp = icmp eq ptr %arg, null
+  br label %bb9
+
+bb9:                                              ; preds = %bb7
+  cleanupret from %cleanuppad unwind label %bb10
+
+bb10:                                             ; preds = %bb9, %bb
+  %phi11 = phi ptr [ %arg, %bb9 ], [ null, %bb ]
+  %cleanuppad12 = cleanuppad within none []
+  %getelementptr13 = getelementptr i8, ptr %phi11, i32 -20
+  store i32 0, ptr %phi11, align 4
+  br label %bb6
+}
+
+declare void @func(i64, i64, ptr)