[llvm] c5fff90 - [NFC][SimplifyCFG] Merge `FoldTwoEntryPHINode()` into it's only callee

Roman Lebedev via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 2 06:54:30 PST 2022


Author: Roman Lebedev
Date: 2022-02-02T17:53:56+03:00
New Revision: c5fff9095342a792bf4b9a077fe3c3a83c4e566c

URL: https://github.com/llvm/llvm-project/commit/c5fff9095342a792bf4b9a077fe3c3a83c4e566c
DIFF: https://github.com/llvm/llvm-project/commit/c5fff9095342a792bf4b9a077fe3c3a83c4e566c.diff

LOG: [NFC][SimplifyCFG] Merge `FoldTwoEntryPHINode()` into it's only callee

Added: 
    

Modified: 
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 593d4d8dae6f..8e1fdad8ac5b 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2765,17 +2765,84 @@ static bool FoldCondBranchOnPHI(BranchInst *BI, DomTreeUpdater *DTU,
   return EverChanged;
 }
 
-/// Given a BB that starts with the specified two-entry PHI node,
-/// see if we can eliminate it.
-static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
-                                DomTreeUpdater *DTU, const DataLayout &DL) {
+static bool SpeculativelyExecuteThenElseCode(BranchInst *BI,
+                                             const TargetTransformInfo &TTI,
+                                             DomTreeUpdater *DTU,
+                                             const DataLayout &DL) {
+  assert(BI->isConditional() && !isa<ConstantInt>(BI->getCondition()) &&
+         BI->getSuccessor(0) != BI->getSuccessor(1) &&
+         "Only for truly conditional branches.");
+  BasicBlock *BB = BI->getParent();
+
+  // Which ones of our successors end up with an unconditional branch?
+  SmallVector<BasicBlock *, 2> UncondSuccessors;
+  SmallVector<BasicBlock *, 2> OtherSuccessors;
+  for (BasicBlock *Succ : successors(BI)) {
+    auto *SuccBI = dyn_cast<BranchInst>(Succ->getTerminator());
+    if (SuccBI && SuccBI->isUnconditional())
+      UncondSuccessors.emplace_back(Succ);
+    else
+      OtherSuccessors.emplace_back(Succ);
+  }
+  assert(UncondSuccessors.size() + OtherSuccessors.size() == 2 &&
+         "Can not have more than two successors!");
+
+  // If none do, then we can't do anything.
+  if (UncondSuccessors.empty())
+    return false;
+
+  // We want to hoist code from the unconditional block[s] and eliminate them,
+  // but if they have their address taken, then we essentially can't do this.
+  for (BasicBlock *UncondSucc : UncondSuccessors)
+    if (UncondSucc->hasAddressTaken())
+      return false;
+
+  // All unconditional successors must have a single (and the same) predecessor.
+  // FIXME: lift this restriction.
+  for (BasicBlock *UncondSucc : UncondSuccessors)
+    if (!UncondSucc->getSinglePredecessor())
+      return false;
+
+  // Now, what is the merge point?
+  BasicBlock *MergeBB = nullptr;
+  // If there was only a single unconditional successor,
+  // then the other successor *must* be the merge point.
+  if (UncondSuccessors.size() == 1)
+    MergeBB = OtherSuccessors.front();
+
+  // All unconditional successors must have the same successor themselves.
+  for (BasicBlock *UncondSucc : UncondSuccessors) {
+    auto *SuccBI = cast<BranchInst>(UncondSucc->getTerminator());
+    assert(SuccBI->isUnconditional() && "Should be an unconditional branch.");
+    BasicBlock *SuccOfSucc = SuccBI->getSuccessor(0);
+    if (!MergeBB) // First unconditional successor, record it's successor.
+      MergeBB = SuccOfSucc;
+    else if (SuccOfSucc != MergeBB) // Do all succs have the same successor?
+      return false;
+  }
+
+  assert(MergeBB && "Should have found the merge point.");
+  assert(all_of(UncondSuccessors,
+                [MergeBB](BasicBlock *UncondSucc) {
+                  return is_contained(predecessors(MergeBB), UncondSucc);
+                }) &&
+         "All unconditional successors must be predecessors of merge block.");
+  assert((UncondSuccessors.size() != 1 ||
+          is_contained(predecessors(MergeBB), BB)) &&
+         "If there is only a single unconditional successor, then the dispatch "
+         "block must also be merge block's predecessor.");
+
+  auto *PN = dyn_cast<PHINode>(MergeBB->begin());
+  if (!PN || PN->getNumIncomingValues() != 2)
+    return false;
+
   // Ok, this is a two entry PHI node.  Check to see if this is a simple "if
   // statement", which has a very simple dominance structure.  Basically, we
   // are trying to find the condition that is being branched on, which
   // subsequently causes this merge to happen.  We really want control
   // dependence information for this check, but simplifycfg can't keep it up
   // to date, and this catches most of the cases we care about anyway.
-  BasicBlock *MergeBB = PN->getParent();
+  MergeBB = PN->getParent();
 
   BasicBlock *IfTrue, *IfFalse;
   BranchInst *DomBI = GetIfCondition(MergeBB, IfTrue, IfFalse);
@@ -2961,81 +3028,6 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
   return true;
 }
 
-static bool SpeculativelyExecuteThenElseCode(BranchInst *BI,
-                                             const TargetTransformInfo &TTI,
-                                             DomTreeUpdater *DTU,
-                                             const DataLayout &DL) {
-  assert(BI->isConditional() && !isa<ConstantInt>(BI->getCondition()) &&
-         BI->getSuccessor(0) != BI->getSuccessor(1) &&
-         "Only for truly conditional branches.");
-  BasicBlock *BB = BI->getParent();
-
-  // Which ones of our successors end up with an unconditional branch?
-  SmallVector<BasicBlock *, 2> UncondSuccessors;
-  SmallVector<BasicBlock *, 2> OtherSuccessors;
-  for (BasicBlock *Succ : successors(BI)) {
-    auto *SuccBI = dyn_cast<BranchInst>(Succ->getTerminator());
-    if (SuccBI && SuccBI->isUnconditional())
-      UncondSuccessors.emplace_back(Succ);
-    else
-      OtherSuccessors.emplace_back(Succ);
-  }
-  assert(UncondSuccessors.size() + OtherSuccessors.size() == 2 &&
-         "Can not have more than two successors!");
-
-  // If none do, then we can't do anything.
-  if (UncondSuccessors.empty())
-    return false;
-
-  // We want to hoist code from the unconditional block[s] and eliminate them,
-  // but if they have their address taken, then we essentially can't do this.
-  for (BasicBlock *UncondSucc : UncondSuccessors)
-    if (UncondSucc->hasAddressTaken())
-      return false;
-
-  // All unconditional successors must have a single (and the same) predecessor.
-  // FIXME: lift this restriction.
-  for (BasicBlock *UncondSucc : UncondSuccessors)
-    if (!UncondSucc->getSinglePredecessor())
-      return false;
-
-  // Now, what is the merge point?
-  BasicBlock *MergeBB = nullptr;
-  // If there was only a single unconditional successor,
-  // then the other successor *must* be the merge point.
-  if (UncondSuccessors.size() == 1)
-    MergeBB = OtherSuccessors.front();
-
-  // All unconditional successors must have the same successor themselves.
-  for (BasicBlock *UncondSucc : UncondSuccessors) {
-    auto *SuccBI = cast<BranchInst>(UncondSucc->getTerminator());
-    assert(SuccBI->isUnconditional() && "Should be an unconditional branch.");
-    BasicBlock *SuccOfSucc = SuccBI->getSuccessor(0);
-    if (!MergeBB) // First unconditional successor, record it's successor.
-      MergeBB = SuccOfSucc;
-    else if (SuccOfSucc != MergeBB) // Do all succs have the same successor?
-      return false;
-  }
-
-  assert(MergeBB && "Should have found the merge point.");
-  assert(all_of(UncondSuccessors,
-                [MergeBB](BasicBlock *UncondSucc) {
-                  return is_contained(predecessors(MergeBB), UncondSucc);
-                }) &&
-         "All unconditional successors must be predecessors of merge block.");
-  assert((UncondSuccessors.size() != 1 ||
-          is_contained(predecessors(MergeBB), BB)) &&
-         "If there is only a single unconditional successor, then the dispatch "
-         "block must also be merge block's predecessor.");
-
-  if (auto *PN = dyn_cast<PHINode>(MergeBB->begin()))
-    // FIXME: lift this restriction.
-    if (PN->getNumIncomingValues() == 2)
-      return FoldTwoEntryPHINode(PN, TTI, DTU, DL);
-
-  return false;
-}
-
 static Value *createLogicalOp(IRBuilderBase &Builder,
                               Instruction::BinaryOps Opc, Value *LHS,
                               Value *RHS, const Twine &Name = "") {


        


More information about the llvm-commits mailing list