Revert "[SimplifyCFG] Start redesigning FoldTwoEntryPHINode()."

Unfortunately, it seems we really do need to take the long route;
start from the "merge" block, find (all the) "dispatch" blocks,
and deal with each "dispatch" block separately, instead of simply
starting from each "dispatch" block like it would logically make sense,
otherwise we run into a number of other missing folds around
`switch` formation, missing sinking/hoisting and phase ordering.

This reverts commit 85628ce.
This reverts commit c5fff90.
This reverts commit 34a98e1.
This reverts commit 1e353f0.

Author: LebedevRI

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

@@ -2765,86 +2765,20 @@ static bool FoldCondBranchOnPHI(BranchInst *BI, DomTreeUpdater *DTU,
   return EverChanged;
 }
 
-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.");
-
-  // 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), BI->getParent())) &&
-         "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;
-
+/// 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) {
   // 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.
-  MergeBB = PN->getParent();
+  BasicBlock *BB = PN->getParent();
 
   BasicBlock *IfTrue, *IfFalse;
-  BranchInst *DomBI = GetIfCondition(MergeBB, IfTrue, IfFalse);
+  BranchInst *DomBI = GetIfCondition(BB, IfTrue, IfFalse);
   if (!DomBI)
     return false;
   Value *IfCond = DomBI->getCondition();
@@ -2876,7 +2810,7 @@ static bool SpeculativelyExecuteThenElseCode(BranchInst *BI,
       BranchProbability BIFalseProb = BITrueProb.getCompl();
       if (IfBlocks.size() == 1) {
         BranchProbability BIBBProb =
-            DomBI->getSuccessor(0) == MergeBB ? BITrueProb : BIFalseProb;
+            DomBI->getSuccessor(0) == BB ? BITrueProb : BIFalseProb;
         if (BIBBProb >= Likely)
           return false;
       } else {
@@ -2889,7 +2823,7 @@ static bool SpeculativelyExecuteThenElseCode(BranchInst *BI,
   // Don't try to fold an unreachable block. For example, the phi node itself
   // can't be the candidate if-condition for a select that we want to form.
   if (auto *IfCondPhiInst = dyn_cast<PHINode>(IfCond))
-    if (IfCondPhiInst->getParent() == MergeBB)
+    if (IfCondPhiInst->getParent() == BB)
       return false;
 
   // Okay, we found that we can merge this two-entry phi node into a select.
@@ -2898,8 +2832,7 @@ static bool SpeculativelyExecuteThenElseCode(BranchInst *BI,
   // doesn't support cmov's).  Only do this transformation if there are two or
   // fewer PHI nodes in this block.
   unsigned NumPhis = 0;
-  for (BasicBlock::iterator I = MergeBB->begin(); isa<PHINode>(I);
-       ++NumPhis, ++I)
+  for (BasicBlock::iterator I = BB->begin(); isa<PHINode>(I); ++NumPhis, ++I)
     if (NumPhis > 2)
       return false;
 
@@ -2912,7 +2845,7 @@ static bool SpeculativelyExecuteThenElseCode(BranchInst *BI,
       TwoEntryPHINodeFoldingThreshold * TargetTransformInfo::TCC_Basic;
 
   bool Changed = false;
-  for (BasicBlock::iterator II = MergeBB->begin(); isa<PHINode>(II);) {
+  for (BasicBlock::iterator II = BB->begin(); isa<PHINode>(II);) {
     PHINode *PN = cast<PHINode>(II++);
     if (Value *V = SimplifyInstruction(PN, {DL, PN})) {
       PN->replaceAllUsesWith(V);
@@ -2921,16 +2854,16 @@ static bool SpeculativelyExecuteThenElseCode(BranchInst *BI,
       continue;
     }
 
-    if (!dominatesMergePoint(PN->getIncomingValue(0), MergeBB, AggressiveInsts,
+    if (!dominatesMergePoint(PN->getIncomingValue(0), BB, AggressiveInsts,
                              Cost, Budget, TTI) ||
-        !dominatesMergePoint(PN->getIncomingValue(1), MergeBB, AggressiveInsts,
+        !dominatesMergePoint(PN->getIncomingValue(1), BB, AggressiveInsts,
                              Cost, Budget, TTI))
       return Changed;
   }
 
   // If we folded the first phi, PN dangles at this point.  Refresh it.  If
   // we ran out of PHIs then we simplified them all.
-  PN = dyn_cast<PHINode>(MergeBB->begin());
+  PN = dyn_cast<PHINode>(BB->begin());
   if (!PN)
     return true;
 
@@ -2994,7 +2927,7 @@ static bool SpeculativelyExecuteThenElseCode(BranchInst *BI,
   IRBuilder<NoFolder> Builder(DomBI);
   // Propagate fast-math-flags from phi nodes to replacement selects.
   IRBuilder<>::FastMathFlagGuard FMFGuard(Builder);
-  while (PHINode *PN = dyn_cast<PHINode>(MergeBB->begin())) {
+  while (PHINode *PN = dyn_cast<PHINode>(BB->begin())) {
     if (isa<FPMathOperator>(PN))
       Builder.setFastMathFlags(PN->getFastMathFlags());
 
@@ -3011,11 +2944,11 @@ static bool SpeculativelyExecuteThenElseCode(BranchInst *BI,
   // At this point, all IfBlocks are empty, so our if statement
   // has been flattened.  Change DomBlock to jump directly to our new block to
   // avoid other simplifycfg's kicking in on the diamond.
-  Builder.CreateBr(MergeBB);
+  Builder.CreateBr(BB);
 
   SmallVector<DominatorTree::UpdateType, 3> Updates;
   if (DTU) {
-    Updates.push_back({DominatorTree::Insert, DomBlock, MergeBB});
+    Updates.push_back({DominatorTree::Insert, DomBlock, BB});
     for (auto *Successor : successors(DomBlock))
       Updates.push_back({DominatorTree::Delete, DomBlock, Successor});
   }
@@ -6588,11 +6521,6 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
         return requestResimplify();
   }
 
-  if (Options.FoldTwoEntryPHINode) {
-    if (SpeculativelyExecuteThenElseCode(BI, TTI, DTU, DL))
-      return true;
-  }
-
   // If this is a branch on a phi node in the current block, thread control
   // through this block if any PHI node entries are constants.
   if (PHINode *PN = dyn_cast<PHINode>(BI->getCondition()))
@@ -6808,6 +6736,15 @@ bool SimplifyCFGOpt::simplifyOnce(BasicBlock *BB) {
 
   IRBuilder<> Builder(BB);
 
+  if (Options.FoldTwoEntryPHINode) {
+    // If there is a trivial two-entry PHI node in this basic block, and we can
+    // eliminate it, do so now.
+    if (auto *PN = dyn_cast<PHINode>(BB->begin()))
+      if (PN->getNumIncomingValues() == 2)
+        if (FoldTwoEntryPHINode(PN, TTI, DTU, DL))
+          return true;
+  }
+
   Instruction *Terminator = BB->getTerminator();
   Builder.SetInsertPoint(Terminator);
   switch (Terminator->getOpcode()) {

llvm/test/CodeGen/AArch64/check-sign-bit-before-extension.ll

@@ -13,12 +13,10 @@
 define i32 @f_i8_sign_extend_inreg(i8 %in, i32 %a, i32 %b) nounwind {
 ; CHECK-LABEL: f_i8_sign_extend_inreg:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    and w8, w0, #0xff
-; CHECK-NEXT:    sxtb w9, w0
-; CHECK-NEXT:    add w10, w8, w1
-; CHECK-NEXT:    add w8, w8, w2
-; CHECK-NEXT:    cmp w9, #0
-; CHECK-NEXT:    csel w0, w10, w8, ge
+; CHECK-NEXT:    sxtb w8, w0
+; CHECK-NEXT:    cmp w8, #0
+; CHECK-NEXT:    csel w8, w1, w2, ge
+; CHECK-NEXT:    add w0, w8, w0, uxtb
 ; CHECK-NEXT:    ret
 entry:
   %cmp = icmp sgt i8 %in, -1
@@ -37,12 +35,10 @@ B:
 define i32 @f_i16_sign_extend_inreg(i16 %in, i32 %a, i32 %b) nounwind {
 ; CHECK-LABEL: f_i16_sign_extend_inreg:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    and w8, w0, #0xffff
-; CHECK-NEXT:    sxth w9, w0
-; CHECK-NEXT:    add w10, w8, w1
-; CHECK-NEXT:    add w8, w8, w2
-; CHECK-NEXT:    cmp w9, #0
-; CHECK-NEXT:    csel w0, w10, w8, ge
+; CHECK-NEXT:    sxth w8, w0
+; CHECK-NEXT:    cmp w8, #0
+; CHECK-NEXT:    csel w8, w1, w2, ge
+; CHECK-NEXT:    add w0, w8, w0, uxth
 ; CHECK-NEXT:    ret
 entry:
   %cmp = icmp sgt i16 %in, -1
@@ -61,11 +57,9 @@ B:
 define i64 @f_i32_sign_extend_inreg(i32 %in, i64 %a, i64 %b) nounwind {
 ; CHECK-LABEL: f_i32_sign_extend_inreg:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    mov w8, w0
 ; CHECK-NEXT:    cmp w0, #0
-; CHECK-NEXT:    add x9, x8, x1
-; CHECK-NEXT:    add x8, x8, x2
-; CHECK-NEXT:    csel x0, x9, x8, ge
+; CHECK-NEXT:    csel x8, x1, x2, ge
+; CHECK-NEXT:    add x0, x8, w0, uxtw
 ; CHECK-NEXT:    ret
 entry:
   %cmp = icmp sgt i32 %in, -1
@@ -84,12 +78,10 @@ B:
 define i32 @g_i8_sign_extend_inreg(i8 %in, i32 %a, i32 %b) nounwind {
 ; CHECK-LABEL: g_i8_sign_extend_inreg:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    and w8, w0, #0xff
-; CHECK-NEXT:    sxtb w9, w0
-; CHECK-NEXT:    add w10, w8, w1
-; CHECK-NEXT:    add w8, w8, w2
-; CHECK-NEXT:    cmp w9, #0
-; CHECK-NEXT:    csel w0, w10, w8, lt
+; CHECK-NEXT:    sxtb w8, w0
+; CHECK-NEXT:    cmp w8, #0
+; CHECK-NEXT:    csel w8, w1, w2, lt
+; CHECK-NEXT:    add w0, w8, w0, uxtb
 ; CHECK-NEXT:    ret
 entry:
   %cmp = icmp slt i8 %in, 0
@@ -108,12 +100,10 @@ B:
 define i32 @g_i16_sign_extend_inreg(i16 %in, i32 %a, i32 %b) nounwind {
 ; CHECK-LABEL: g_i16_sign_extend_inreg:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    and w8, w0, #0xffff
-; CHECK-NEXT:    sxth w9, w0
-; CHECK-NEXT:    add w10, w8, w1
-; CHECK-NEXT:    add w8, w8, w2
-; CHECK-NEXT:    cmp w9, #0
-; CHECK-NEXT:    csel w0, w10, w8, lt
+; CHECK-NEXT:    sxth w8, w0
+; CHECK-NEXT:    cmp w8, #0
+; CHECK-NEXT:    csel w8, w1, w2, lt
+; CHECK-NEXT:    add w0, w8, w0, uxth
 ; CHECK-NEXT:    ret
 entry:
   %cmp = icmp slt i16 %in, 0
@@ -132,11 +122,9 @@ B:
 define i64 @g_i32_sign_extend_inreg(i32 %in, i64 %a, i64 %b) nounwind {
 ; CHECK-LABEL: g_i32_sign_extend_inreg:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    mov w8, w0
 ; CHECK-NEXT:    cmp w0, #0
-; CHECK-NEXT:    add x9, x8, x1
-; CHECK-NEXT:    add x8, x8, x2
-; CHECK-NEXT:    csel x0, x9, x8, lt
+; CHECK-NEXT:    csel x8, x1, x2, lt
+; CHECK-NEXT:    add x0, x8, w0, uxtw
 ; CHECK-NEXT:    ret
 entry:
   %cmp = icmp slt i32 %in, 0
@@ -156,12 +144,10 @@ define i64 @f_i32_sign_extend_i64(i32 %in, i64 %a, i64 %b) nounwind {
 ; CHECK-LABEL: f_i32_sign_extend_i64:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    // kill: def $w0 killed $w0 def $x0
-; CHECK-NEXT:    mov w8, w0
-; CHECK-NEXT:    sxtw x9, w0
-; CHECK-NEXT:    add x10, x8, x1
-; CHECK-NEXT:    add x8, x8, x2
-; CHECK-NEXT:    cmp x9, #0
-; CHECK-NEXT:    csel x0, x10, x8, ge
+; CHECK-NEXT:    sxtw x8, w0
+; CHECK-NEXT:    cmp x8, #0
+; CHECK-NEXT:    csel x8, x1, x2, ge
+; CHECK-NEXT:    add x0, x8, w0, uxtw
 ; CHECK-NEXT:    ret
 entry:
   %inext = sext i32 %in to i64
@@ -182,12 +168,10 @@ define i64 @g_i32_sign_extend_i64(i32 %in, i64 %a, i64 %b) nounwind {
 ; CHECK-LABEL: g_i32_sign_extend_i64:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    // kill: def $w0 killed $w0 def $x0
-; CHECK-NEXT:    mov w8, w0
-; CHECK-NEXT:    sxtw x9, w0
-; CHECK-NEXT:    add x10, x8, x1
-; CHECK-NEXT:    add x8, x8, x2
-; CHECK-NEXT:    cmp x9, #0
-; CHECK-NEXT:    csel x0, x10, x8, lt
+; CHECK-NEXT:    sxtw x8, w0
+; CHECK-NEXT:    cmp x8, #0
+; CHECK-NEXT:    csel x8, x1, x2, lt
+; CHECK-NEXT:    add x0, x8, w0, uxtw
 ; CHECK-NEXT:    ret
 entry:
   %inext = sext i32 %in to i64

llvm/test/CodeGen/AArch64/combine-comparisons-by-cse.ll

@@ -654,7 +654,7 @@ define i32 @fcmpri(i32 %argc, i8** nocapture readonly %argv) {
 ; CHECK-NEXT:    cbz x8, .LBB9_3
 ; CHECK-NEXT:  // %bb.2:
 ; CHECK-NEXT:    mov w0, #3
-; CHECK-NEXT:    b .LBB9_6
+; CHECK-NEXT:    b .LBB9_4
 ; CHECK-NEXT:  .LBB9_3: // %if.end
 ; CHECK-NEXT:    mov w0, #1
 ; CHECK-NEXT:    bl zoo
@@ -666,17 +666,14 @@ define i32 @fcmpri(i32 %argc, i8** nocapture readonly %argv) {
 ; CHECK-NEXT:    cinc w0, w19, gt
 ; CHECK-NEXT:    fmov d8, d0
 ; CHECK-NEXT:    bl xoo
-; CHECK-NEXT:    fcmp d8, #0.0
-; CHECK-NEXT:    b.gt .LBB9_5
-; CHECK-NEXT:  // %bb.4: // %cond.false12
 ; CHECK-NEXT:    fmov d0, #-1.00000000
-; CHECK-NEXT:    fadd d8, d8, d0
-; CHECK-NEXT:  .LBB9_5: // %cond.end14
-; CHECK-NEXT:    fmov d0, d8
+; CHECK-NEXT:    fcmp d8, #0.0
 ; CHECK-NEXT:    fmov d1, #-2.00000000
+; CHECK-NEXT:    fadd d0, d8, d0
+; CHECK-NEXT:    fcsel d0, d8, d0, gt
 ; CHECK-NEXT:    bl woo
 ; CHECK-NEXT:    mov w0, #4
-; CHECK-NEXT:  .LBB9_6: // %return
+; CHECK-NEXT:  .LBB9_4: // %return
 ; CHECK-NEXT:    ldp x30, x19, [sp, #16] // 16-byte Folded Reload
 ; CHECK-NEXT:    ldr d8, [sp], #32 // 8-byte Folded Reload
 ; CHECK-NEXT:    ret