Revert "Reland "[BasicBlockUtils] Handle funclets when detaching EH p… #159292
+28
−321
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ad blocks" (llvm#158435)" This reverts commit 41cef78.
|
@llvm/pr-subscribers-llvm-transforms Author: Gábor Spaits (spaits) Changes…ad blocks" (#158435)" This reverts commit 41cef78. Full diff: https://github.com/llvm/llvm-project/pull/159292.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 91e245e5e8f54..cad0b4c12b54e 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -58,94 +58,37 @@ static cl::opt<unsigned> MaxDeoptOrUnreachableSuccessorCheckDepth(
"is followed by a block that either has a terminating "
"deoptimizing call or is terminated with an unreachable"));
-/// Zap all the instructions in the block and replace them with an unreachable
-/// instruction and notify the basic block's successors that one of their
-/// predecessors is going away.
-static void
-emptyAndDetachBlock(BasicBlock *BB,
- SmallVectorImpl<DominatorTree::UpdateType> *Updates,
- bool KeepOneInputPHIs) {
- // 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});
- }
-
- // 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.");
-}
-
-void llvm::detachDeadBlocks(ArrayRef<BasicBlock *> BBs,
- SmallVectorImpl<DominatorTree::UpdateType> *Updates,
- bool KeepOneInputPHIs) {
+void llvm::detachDeadBlocks(
+ ArrayRef<BasicBlock *> BBs,
+ SmallVectorImpl<DominatorTree::UpdateType> *Updates,
+ bool KeepOneInputPHIs) {
for (auto *BB : BBs) {
- 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)) {
- 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. The other alternative, that may use a catchpad is a
- // catchswitch. That does not need special handling for now.
- 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.
- // This basic block also may have some predecessors too. For
- // example the following LLVM-IR is valid:
- //
- // [cleanuppad_block]
- // |
- // [regular_block]
- // |
- // [cleanupret_block]
- //
- // The IR after the cleanup will look like this:
- //
- // [cleanuppad_block]
- // |
- // [regular_block]
- // |
- // [unreachable]
- //
- // So regular_block will lead to an unreachable block, which is also
- // valid. There is no need to replace regular_block with unreachable
- // in this context now.
- // On the other hand, the cleanupret/catchret block's successors
- // need to know about the deletion of their predecessors.
- emptyAndDetachBlock(ReturnInstrBB, Updates, KeepOneInputPHIs);
- }
- }
- }
+ // 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});
}
- // Detaching and emptying the current basic block.
- emptyAndDetachBlock(BB, 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.");
}
}
diff --git a/llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll b/llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll
deleted file mode 100644
index 0f0fc78ec7add..0000000000000
--- a/llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll
+++ /dev/null
@@ -1,236 +0,0 @@
-; 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_linear_middle_block(i64 %shapes.1) personality ptr null {
-; CHECK-LABEL: define void @unreachable_cleanuppad_linear_middle_block(
-; 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 %middle_block
-
-middle_block:
- %tmp1 = add i64 %shapes.1, 42
- 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 {
-; CHECK-LABEL: define x86_thiscallcc ptr @baz(
-; CHECK-SAME: ptr [[ARG:%.*]], ptr [[ARG1:%.*]], ptr [[ARG2:%.*]], i1 [[ARG3:%.*]], ptr [[ARG4:%.*]]) personality ptr null {
-; CHECK-NEXT: [[BB:.*:]]
-; CHECK-NEXT: [[ALLOCA:%.*]] = alloca [2 x %struct.foo], align 4
-; CHECK-NEXT: [[INVOKE:%.*]] = call x86_thiscallcc ptr @quux(ptr null, ptr null, i32 0) #[[ATTR1:[0-9]+]]
-; CHECK-NEXT: unreachable
-;
-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)
|
|
Tests are failing on buildbot: |
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp -- llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
View the diff from clang-format here.diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index cad0b4c12..4bb3cb2d5 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -58,10 +58,9 @@ 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,
- bool KeepOneInputPHIs) {
+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.
@@ -85,8 +84,7 @@ void llvm::detachDeadBlocks(
BB->back().eraseFromParent();
}
new UnreachableInst(BB->getContext(), BB);
- assert(BB->size() == 1 &&
- isa<UnreachableInst>(BB->getTerminator()) &&
+ assert(BB->size() == 1 && isa<UnreachableInst>(BB->getTerminator()) &&
"The successor list of BB isn't empty before "
"applying corresponding DTU updates.");
}
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/163/builds/26601 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ad blocks" (#158435)"
This reverts commit 41cef78.