[llvm] 48fc781 - [UnifyFunctionExitNodes] Fix Modified status for unreachable blocks
David Stenberg via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 9 04:36:24 PDT 2020
Author: David Stenberg
Date: 2020-09-09T13:36:03+02:00
New Revision: 48fc781438767bd8337facf2e232c695b0426fb4
URL: https://github.com/llvm/llvm-project/commit/48fc781438767bd8337facf2e232c695b0426fb4
DIFF: https://github.com/llvm/llvm-project/commit/48fc781438767bd8337facf2e232c695b0426fb4.diff
LOG: [UnifyFunctionExitNodes] Fix Modified status for unreachable blocks
If a function had at most one return block, the pass would return false
regardless if an unified unreachable block was created.
This patch fixes that by refactoring runOnFunction into two separate
helper functions for handling the unreachable blocks respectively the
return blocks, as suggested by @bjope in a review comment.
This was caught using the check introduced by D80916.
Reviewed By: serge-sans-paille
Differential Revision: https://reviews.llvm.org/D85818
Added:
llvm/test/Transforms/UnifyFunctionExitNodes/unreachable-blocks-status.ll
Modified:
llvm/include/llvm/Transforms/Utils/UnifyFunctionExitNodes.h
llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/UnifyFunctionExitNodes.h b/llvm/include/llvm/Transforms/Utils/UnifyFunctionExitNodes.h
index ce7cb16b3886..a9fe808cb455 100644
--- a/llvm/include/llvm/Transforms/Utils/UnifyFunctionExitNodes.h
+++ b/llvm/include/llvm/Transforms/Utils/UnifyFunctionExitNodes.h
@@ -20,7 +20,10 @@ namespace llvm {
class BasicBlock;
-struct UnifyFunctionExitNodes : public FunctionPass {
+class UnifyFunctionExitNodes : public FunctionPass {
+ bool unifyUnreachableBlocks(Function &F);
+ bool unifyReturnBlocks(Function &F);
+
public:
static char ID; // Pass identification, replacement for typeid
UnifyFunctionExitNodes();
diff --git a/llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp b/llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp
index b124d0536254..621e944741b1 100644
--- a/llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp
+++ b/llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp
@@ -40,44 +40,41 @@ void UnifyFunctionExitNodes::getAnalysisUsage(AnalysisUsage &AU) const{
AU.addPreservedID(LowerSwitchID);
}
-// UnifyAllExitNodes - Unify all exit nodes of the CFG by creating a new
-// BasicBlock, and converting all returns to unconditional branches to this
-// new basic block. The singular exit node is returned.
-//
-// If there are no return stmts in the Function, a null pointer is returned.
-//
-bool UnifyFunctionExitNodes::runOnFunction(Function &F) {
- // Loop over all of the blocks in a function, tracking all of the blocks that
- // return.
- //
- std::vector<BasicBlock*> ReturningBlocks;
+bool UnifyFunctionExitNodes::unifyUnreachableBlocks(Function &F) {
std::vector<BasicBlock*> UnreachableBlocks;
+
for (BasicBlock &I : F)
- if (isa<ReturnInst>(I.getTerminator()))
- ReturningBlocks.push_back(&I);
- else if (isa<UnreachableInst>(I.getTerminator()))
+ if (isa<UnreachableInst>(I.getTerminator()))
UnreachableBlocks.push_back(&I);
- // Then unreachable blocks.
- if (UnreachableBlocks.size() > 1) {
- BasicBlock *UnreachableBlock = BasicBlock::Create(F.getContext(),
- "UnifiedUnreachableBlock", &F);
- new UnreachableInst(F.getContext(), UnreachableBlock);
+ if (UnreachableBlocks.size() <= 1)
+ return false;
+
+ BasicBlock *UnreachableBlock =
+ BasicBlock::Create(F.getContext(), "UnifiedUnreachableBlock", &F);
+ new UnreachableInst(F.getContext(), UnreachableBlock);
- for (BasicBlock *BB : UnreachableBlocks) {
- BB->getInstList().pop_back(); // Remove the unreachable inst.
- BranchInst::Create(UnreachableBlock, BB);
- }
+ for (BasicBlock *BB : UnreachableBlocks) {
+ BB->getInstList().pop_back(); // Remove the unreachable inst.
+ BranchInst::Create(UnreachableBlock, BB);
}
- // There is nothing more to do if we do not have multiple return blocks.
+ return true;
+}
+
+bool UnifyFunctionExitNodes::unifyReturnBlocks(Function &F) {
+ std::vector<BasicBlock *> ReturningBlocks;
+
+ for (BasicBlock &I : F)
+ if (isa<ReturnInst>(I.getTerminator()))
+ ReturningBlocks.push_back(&I);
+
if (ReturningBlocks.size() <= 1)
return false;
- // Otherwise, we need to insert a new basic block into the function, add a PHI
- // nodes (if the function returns values), and convert all of the return
- // instructions into unconditional branches.
- //
+ // Insert a new basic block into the function, add PHI nodes (if the function
+ // returns values), and convert all of the return instructions into
+ // unconditional branches.
BasicBlock *NewRetBlock = BasicBlock::Create(F.getContext(),
"UnifiedReturnBlock", &F);
@@ -94,7 +91,6 @@ bool UnifyFunctionExitNodes::runOnFunction(Function &F) {
// Loop over all of the blocks, replacing the return instruction with an
// unconditional branch.
- //
for (BasicBlock *BB : ReturningBlocks) {
// Add an incoming element to the PHI node for every return instruction that
// is merging into this new block...
@@ -104,5 +100,16 @@ bool UnifyFunctionExitNodes::runOnFunction(Function &F) {
BB->getInstList().pop_back(); // Remove the return insn
BranchInst::Create(NewRetBlock, BB);
}
+
return true;
}
+
+// Unify all exit nodes of the CFG by creating a new BasicBlock, and converting
+// all returns to unconditional branches to this new basic block. Also, unify
+// all unreachable blocks.
+bool UnifyFunctionExitNodes::runOnFunction(Function &F) {
+ bool Changed = false;
+ Changed |= unifyUnreachableBlocks(F);
+ Changed |= unifyReturnBlocks(F);
+ return Changed;
+}
diff --git a/llvm/test/Transforms/UnifyFunctionExitNodes/unreachable-blocks-status.ll b/llvm/test/Transforms/UnifyFunctionExitNodes/unreachable-blocks-status.ll
new file mode 100644
index 000000000000..a9169e9ff15e
--- /dev/null
+++ b/llvm/test/Transforms/UnifyFunctionExitNodes/unreachable-blocks-status.ll
@@ -0,0 +1,67 @@
+; RUN: opt -mergereturn -S < %s | FileCheck %s
+
+; The pass did previously not report the correct Modified status in the case
+; where a function had at most one return block, and an unified unreachable
+; block was created. This was caught by the pass return status check that is
+; hidden under EXPENSIVE_CHECKS.
+
+; CHECK: for.foo.body2:
+; CHECK-NEXT: br label %UnifiedUnreachableBlock
+
+; CHECK: for.foo.end:
+; CHECK-NEXT: br label %UnifiedUnreachableBlock
+
+; CHECK: UnifiedUnreachableBlock:
+; CHECK-NEXT: unreachable
+
+define i32 @foo() {
+entry:
+ br label %for.foo.cond
+
+for.foo.cond: ; preds = %entry
+ br i1 false, label %for.foo.body, label %for.foo.end3
+
+for.foo.body: ; preds = %for.foo.cond
+ br label %for.foo.cond1
+
+for.foo.cond1: ; preds = %for.foo.body
+ br i1 false, label %for.foo.body2, label %for.foo.end
+
+for.foo.body2: ; preds = %for.foo.cond1
+ unreachable
+
+for.foo.end: ; preds = %for.foo.cond1
+ unreachable
+
+for.foo.end3: ; preds = %for.foo.cond
+ ret i32 undef
+}
+
+; CHECK: for.bar.body2:
+; CHECK-NEXT: br label %UnifiedUnreachableBlock
+
+; CHECK: for.bar.end:
+; CHECK-NEXT: br label %UnifiedUnreachableBlock
+
+; CHECK: UnifiedUnreachableBlock:
+; CHECK-NEXT: unreachable
+
+define void @bar() {
+entry:
+ br label %for.bar.cond
+
+for.bar.cond: ; preds = %entry
+ br i1 false, label %for.bar.body, label %for.bar.end
+
+for.bar.body: ; preds = %for.bar.cond
+ br label %for.bar.cond1
+
+for.bar.cond1: ; preds = %for.bar.body
+ br i1 false, label %for.bar.body2, label %for.bar.end
+
+for.bar.body2: ; preds = %for.bar.cond1
+ unreachable
+
+for.bar.end: ; preds = %for.bar.cond1
+ unreachable
+}
More information about the llvm-commits
mailing list