[SimplifyCFG] Reland transform for redirecting phis between unmergeable BB and SuccBB (#68473)

Reland #67275 with #68953 resolved.

Author: XChy

llvm/lib/Transforms/Utils/Local.cpp

@@ -850,17 +850,17 @@ static bool CanMergeValues(Value *First, Value *Second) {
 /// branch to Succ, into Succ.
 ///
 /// Assumption: Succ is the single successor for BB.
-static bool CanPropagatePredecessorsForPHIs(BasicBlock *BB, BasicBlock *Succ) {
+static bool
+CanPropagatePredecessorsForPHIs(BasicBlock *BB, BasicBlock *Succ,
+                                const SmallPtrSetImpl<BasicBlock *> &BBPreds) {
   assert(*succ_begin(BB) == Succ && "Succ is not successor of BB!");
 
   LLVM_DEBUG(dbgs() << "Looking to fold " << BB->getName() << " into "
                     << Succ->getName() << "\n");
   // Shortcut, if there is only a single predecessor it must be BB and merging
   // is always safe
-  if (Succ->getSinglePredecessor()) return true;
-
-  // Make a list of the predecessors of BB
-  SmallPtrSet<BasicBlock*, 16> BBPreds(pred_begin(BB), pred_end(BB));
+  if (Succ->getSinglePredecessor())
+    return true;
 
   // Look at all the phi nodes in Succ, to see if they present a conflict when
   // merging these blocks
@@ -1000,16 +1000,47 @@ static void replaceUndefValuesInPhi(PHINode *PN,
   }
 }
 
+// Only when they shares a single common predecessor, return true.
+// Only handles cases when BB can't be merged while its predecessors can be
+// redirected.
+static bool
+CanRedirectPredsOfEmptyBBToSucc(BasicBlock *BB, BasicBlock *Succ,
+                                const SmallPtrSetImpl<BasicBlock *> &BBPreds,
+                                const SmallPtrSetImpl<BasicBlock *> &SuccPreds,
+                                BasicBlock *&CommonPred) {
+
+  // There must be phis in BB, otherwise BB will be merged into Succ directly
+  if (BB->phis().empty() || Succ->phis().empty())
+    return false;
+
+  // BB must have predecessors not shared that can be redirected to Succ
+  if (!BB->hasNPredecessorsOrMore(2))
+    return false;
+
+  // Get single common predecessors of both BB and Succ
+  for (BasicBlock *SuccPred : SuccPreds) {
+    if (BBPreds.count(SuccPred)) {
+      if (CommonPred)
+        return false;
+      CommonPred = SuccPred;
+    }
+  }
+
+  return true;
+}
+
 /// Replace a value flowing from a block to a phi with
 /// potentially multiple instances of that value flowing from the
 /// block's predecessors to the phi.
 ///
 /// \param BB The block with the value flowing into the phi.
 /// \param BBPreds The predecessors of BB.
 /// \param PN The phi that we are updating.
+/// \param CommonPred The common predecessor of BB and PN's BasicBlock
 static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
                                                 const PredBlockVector &BBPreds,
-                                                PHINode *PN) {
+                                                PHINode *PN,
+                                                BasicBlock *CommonPred) {
   Value *OldVal = PN->removeIncomingValue(BB, false);
   assert(OldVal && "No entry in PHI for Pred BB!");
 
@@ -1037,26 +1068,39 @@ static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
       // will trigger asserts if we try to clean it up now, without also
       // simplifying the corresponding conditional branch).
       BasicBlock *PredBB = OldValPN->getIncomingBlock(i);
+
+      if (PredBB == CommonPred)
+        continue;
+
       Value *PredVal = OldValPN->getIncomingValue(i);
-      Value *Selected = selectIncomingValueForBlock(PredVal, PredBB,
-                                                    IncomingValues);
+      Value *Selected =
+          selectIncomingValueForBlock(PredVal, PredBB, IncomingValues);
 
       // And add a new incoming value for this predecessor for the
       // newly retargeted branch.
       PN->addIncoming(Selected, PredBB);
     }
+    if (CommonPred)
+      PN->addIncoming(OldValPN->getIncomingValueForBlock(CommonPred), BB);
+
   } else {
     for (unsigned i = 0, e = BBPreds.size(); i != e; ++i) {
       // Update existing incoming values in PN for this
       // predecessor of BB.
       BasicBlock *PredBB = BBPreds[i];
-      Value *Selected = selectIncomingValueForBlock(OldVal, PredBB,
-                                                    IncomingValues);
+
+      if (PredBB == CommonPred)
+        continue;
+
+      Value *Selected =
+          selectIncomingValueForBlock(OldVal, PredBB, IncomingValues);
 
       // And add a new incoming value for this predecessor for the
       // newly retargeted branch.
       PN->addIncoming(Selected, PredBB);
     }
+    if (CommonPred)
+      PN->addIncoming(OldVal, BB);
   }
 
   replaceUndefValuesInPhi(PN, IncomingValues);
@@ -1067,13 +1111,30 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
   assert(BB != &BB->getParent()->getEntryBlock() &&
          "TryToSimplifyUncondBranchFromEmptyBlock called on entry block!");
 
-  // We can't eliminate infinite loops.
+  // We can't simplify infinite loops.
   BasicBlock *Succ = cast<BranchInst>(BB->getTerminator())->getSuccessor(0);
-  if (BB == Succ) return false;
+  if (BB == Succ)
+    return false;
+
+  SmallPtrSet<BasicBlock *, 16> BBPreds(pred_begin(BB), pred_end(BB));
+  SmallPtrSet<BasicBlock *, 16> SuccPreds(pred_begin(Succ), pred_end(Succ));
 
-  // Check to see if merging these blocks would cause conflicts for any of the
-  // phi nodes in BB or Succ. If not, we can safely merge.
-  if (!CanPropagatePredecessorsForPHIs(BB, Succ)) return false;
+  // The single common predecessor of BB and Succ when BB cannot be killed
+  BasicBlock *CommonPred = nullptr;
+
+  bool BBKillable = CanPropagatePredecessorsForPHIs(BB, Succ, BBPreds);
+
+  // Even if we can not fold bB into Succ, we may be able to redirect the
+  // predecessors of BB to Succ.
+  bool BBPhisMergeable =
+      BBKillable ||
+      CanRedirectPredsOfEmptyBBToSucc(BB, Succ, BBPreds, SuccPreds, CommonPred);
+
+  if (!BBKillable && !BBPhisMergeable)
+    return false;
+
+  // Check to see if merging these blocks/phis would cause conflicts for any of
+  // the phi nodes in BB or Succ. If not, we can safely merge.
 
   // Check for cases where Succ has multiple predecessors and a PHI node in BB
   // has uses which will not disappear when the PHI nodes are merged.  It is
@@ -1102,6 +1163,11 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
     }
   }
 
+  if (BBPhisMergeable && CommonPred)
+    LLVM_DEBUG(dbgs() << "Found Common Predecessor between: " << BB->getName()
+                      << " and " << Succ->getName() << " : "
+                      << CommonPred->getName() << "\n");
+
   // 'BB' and 'BB->Pred' are loop latches, bail out to presrve inner loop
   // metadata.
   //
@@ -1174,25 +1240,37 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
           if (PredTI->hasMetadata(LLVMContext::MD_loop))
             return false;
 
-  LLVM_DEBUG(dbgs() << "Killing Trivial BB: \n" << *BB);
+  if (BBKillable)
+    LLVM_DEBUG(dbgs() << "Killing Trivial BB: \n" << *BB);
+  else if (BBPhisMergeable)
+    LLVM_DEBUG(dbgs() << "Merge Phis in Trivial BB: \n" << *BB);
 
   SmallVector<DominatorTree::UpdateType, 32> Updates;
+
   if (DTU) {
     // To avoid processing the same predecessor more than once.
     SmallPtrSet<BasicBlock *, 8> SeenPreds;
-    // All predecessors of BB will be moved to Succ.
-    SmallPtrSet<BasicBlock *, 8> PredsOfSucc(pred_begin(Succ), pred_end(Succ));
+    // All predecessors of BB (except the common predecessor) will be moved to
+    // Succ.
     Updates.reserve(Updates.size() + 2 * pred_size(BB) + 1);
-    for (auto *PredOfBB : predecessors(BB))
-      // This predecessor of BB may already have Succ as a successor.
-      if (!PredsOfSucc.contains(PredOfBB))
+
+    for (auto *PredOfBB : predecessors(BB)) {
+      // Do not modify those common predecessors of BB and Succ
+      if (!SuccPreds.contains(PredOfBB))
         if (SeenPreds.insert(PredOfBB).second)
           Updates.push_back({DominatorTree::Insert, PredOfBB, Succ});
+    }
+
     SeenPreds.clear();
+
     for (auto *PredOfBB : predecessors(BB))
-      if (SeenPreds.insert(PredOfBB).second)
+      // When BB cannot be killed, do not remove the edge between BB and
+      // CommonPred.
+      if (SeenPreds.insert(PredOfBB).second && PredOfBB != CommonPred)
         Updates.push_back({DominatorTree::Delete, PredOfBB, BB});
-    Updates.push_back({DominatorTree::Delete, BB, Succ});
+
+    if (BBKillable)
+      Updates.push_back({DominatorTree::Delete, BB, Succ});
   }
 
   if (isa<PHINode>(Succ->begin())) {
@@ -1204,21 +1282,19 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
     // Loop over all of the PHI nodes in the successor of BB.
     for (BasicBlock::iterator I = Succ->begin(); isa<PHINode>(I); ++I) {
       PHINode *PN = cast<PHINode>(I);
-
-      redirectValuesFromPredecessorsToPhi(BB, BBPreds, PN);
+      redirectValuesFromPredecessorsToPhi(BB, BBPreds, PN, CommonPred);
     }
   }
 
   if (Succ->getSinglePredecessor()) {
     // BB is the only predecessor of Succ, so Succ will end up with exactly
     // the same predecessors BB had.
-
     // Copy over any phi, debug or lifetime instruction.
     BB->getTerminator()->eraseFromParent();
     Succ->splice(Succ->getFirstNonPHI()->getIterator(), BB);
   } else {
     while (PHINode *PN = dyn_cast<PHINode>(&BB->front())) {
-      // We explicitly check for such uses in CanPropagatePredecessorsForPHIs.
+      // We explicitly check for such uses for merging phis.
       assert(PN->use_empty() && "There shouldn't be any uses here!");
       PN->eraseFromParent();
     }
@@ -1231,21 +1307,35 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
       for (BasicBlock *Pred : predecessors(BB))
         Pred->getTerminator()->setMetadata(LLVMContext::MD_loop, LoopMD);
 
-  // Everything that jumped to BB now goes to Succ.
-  BB->replaceAllUsesWith(Succ);
-  if (!Succ->hasName()) Succ->takeName(BB);
-
-  // Clear the successor list of BB to match updates applying to DTU later.
-  if (BB->getTerminator())
-    BB->back().eraseFromParent();
-  new UnreachableInst(BB->getContext(), BB);
-  assert(succ_empty(BB) && "The successor list of BB isn't empty before "
-                           "applying corresponding DTU updates.");
+  if (BBKillable) {
+    // Everything that jumped to BB now goes to Succ.
+    BB->replaceAllUsesWith(Succ);
+
+    if (!Succ->hasName())
+      Succ->takeName(BB);
+
+    // Clear the successor list of BB to match updates applying to DTU later.
+    if (BB->getTerminator())
+      BB->back().eraseFromParent();
+
+    new UnreachableInst(BB->getContext(), BB);
+    assert(succ_empty(BB) && "The successor list of BB isn't empty before "
+                             "applying corresponding DTU updates.");
+  } else if (BBPhisMergeable) {
+    //  Everything except CommonPred that jumped to BB now goes to Succ.
+    BB->replaceUsesWithIf(Succ, [BBPreds, CommonPred](Use &U) -> bool {
+      if (Instruction *UseInst = dyn_cast<Instruction>(U.getUser()))
+        return UseInst->getParent() != CommonPred &&
+               BBPreds.contains(UseInst->getParent());
+      return false;
+    });
+  }
 
   if (DTU)
     DTU->applyUpdates(Updates);
 
-  DeleteDeadBlock(BB, DTU);
+  if (BBKillable)
+    DeleteDeadBlock(BB, DTU);
 
   return true;
 }

llvm/test/CodeGen/ARM/jump-table-islands.ll

@@ -2,6 +2,8 @@
 
 %BigInt = type i8500
 
+declare void @use(%BigInt)
+
 define %BigInt @test_moved_jumptable(i1 %tst, i32 %sw, %BigInt %l) {
 ; CHECK-LABEL: test_moved_jumptable:
 
@@ -34,6 +36,8 @@ other:
 
 end:
   %val = phi %BigInt [ %l, %complex ], [ -1, %simple ]
+; Prevent SimplifyCFG from simplifying the phi node above.
+  call void @use(%BigInt %val)
   ret %BigInt %val
 }
 

llvm/test/Transforms/JumpThreading/codesize-loop.ll

@@ -42,16 +42,14 @@ define i32 @test_minsize(i32 %argc, ptr nocapture readonly %argv) local_unnamed_
 ; OVERIDE-NEXT:    [[TMP3:%.*]] = mul i32 [[CALL]], [[TMP2]]
 ; OVERIDE-NEXT:    [[TMP4:%.*]] = icmp sgt i32 [[CALL]], 0
 ; OVERIDE-NEXT:    [[COND_FR:%.*]] = freeze i1 [[TMP4]]
-; OVERIDE-NEXT:    br i1 [[COND_FR]], label [[COND_END_THREAD]], label [[TMP6:%.*]]
+; OVERIDE-NEXT:    br i1 [[COND_FR]], label [[TMP5:%.*]], label [[COND_END_THREAD]]
+; OVERIDE:       5:
+; OVERIDE-NEXT:    br label [[COND_END_THREAD]]
 ; OVERIDE:       cond.end.thread:
-; OVERIDE-NEXT:    [[TMP5:%.*]] = phi i32 [ [[TMP3]], [[COND_END]] ], [ 205962976, [[ENTRY:%.*]] ]
-; OVERIDE-NEXT:    [[COND3:%.*]] = phi i32 [ [[CALL]], [[COND_END]] ], [ 46, [[ENTRY]] ]
-; OVERIDE-NEXT:    br label [[TMP6]]
-; OVERIDE:       6:
-; OVERIDE-NEXT:    [[TMP7:%.*]] = phi i32 [ [[TMP5]], [[COND_END_THREAD]] ], [ [[TMP3]], [[COND_END]] ]
-; OVERIDE-NEXT:    [[TMP8:%.*]] = phi i32 [ [[COND3]], [[COND_END_THREAD]] ], [ 0, [[COND_END]] ]
-; OVERIDE-NEXT:    [[TMP9:%.*]] = mul i32 [[TMP7]], [[TMP8]]
-; OVERIDE-NEXT:    [[CALL33:%.*]] = tail call i32 (ptr, ...) @printf(ptr nonnull dereferenceable(1) @.str, i32 [[TMP9]])
+; OVERIDE-NEXT:    [[TMP6:%.*]] = phi i32 [ [[TMP3]], [[COND_END]] ], [ [[TMP3]], [[TMP5]] ], [ 205962976, [[ENTRY:%.*]] ]
+; OVERIDE-NEXT:    [[TMP7:%.*]] = phi i32 [ 0, [[COND_END]] ], [ [[CALL]], [[TMP5]] ], [ 46, [[ENTRY]] ]
+; OVERIDE-NEXT:    [[TMP8:%.*]] = mul i32 [[TMP6]], [[TMP7]]
+; OVERIDE-NEXT:    [[CALL33:%.*]] = tail call i32 (ptr, ...) @printf(ptr nonnull dereferenceable(1) @.str, i32 [[TMP8]])
 ; OVERIDE-NEXT:    ret i32 0
 ;
 entry:
@@ -90,16 +88,14 @@ define i32 @test_optsize(i32 %argc, ptr nocapture readonly %argv) local_unnamed_
 ; DEFAULT-NEXT:    [[TMP3:%.*]] = mul i32 [[CALL]], [[TMP2]]
 ; DEFAULT-NEXT:    [[TMP4:%.*]] = icmp sgt i32 [[CALL]], 0
 ; DEFAULT-NEXT:    [[COND_FR:%.*]] = freeze i1 [[TMP4]]
-; DEFAULT-NEXT:    br i1 [[COND_FR]], label [[COND_END_THREAD]], label [[TMP6:%.*]]
+; DEFAULT-NEXT:    br i1 [[COND_FR]], label [[TMP5:%.*]], label [[COND_END_THREAD]]
+; DEFAULT:       5:
+; DEFAULT-NEXT:    br label [[COND_END_THREAD]]
 ; DEFAULT:       cond.end.thread:
-; DEFAULT-NEXT:    [[TMP5:%.*]] = phi i32 [ [[TMP3]], [[COND_END]] ], [ 205962976, [[ENTRY:%.*]] ]
-; DEFAULT-NEXT:    [[COND3:%.*]] = phi i32 [ [[CALL]], [[COND_END]] ], [ 46, [[ENTRY]] ]
-; DEFAULT-NEXT:    br label [[TMP6]]
-; DEFAULT:       6:
-; DEFAULT-NEXT:    [[TMP7:%.*]] = phi i32 [ [[TMP5]], [[COND_END_THREAD]] ], [ [[TMP3]], [[COND_END]] ]
-; DEFAULT-NEXT:    [[TMP8:%.*]] = phi i32 [ [[COND3]], [[COND_END_THREAD]] ], [ 0, [[COND_END]] ]
-; DEFAULT-NEXT:    [[TMP9:%.*]] = mul i32 [[TMP7]], [[TMP8]]
-; DEFAULT-NEXT:    [[CALL33:%.*]] = tail call i32 (ptr, ...) @printf(ptr nonnull dereferenceable(1) @.str, i32 [[TMP9]])
+; DEFAULT-NEXT:    [[TMP6:%.*]] = phi i32 [ [[TMP3]], [[COND_END]] ], [ [[TMP3]], [[TMP5]] ], [ 205962976, [[ENTRY:%.*]] ]
+; DEFAULT-NEXT:    [[TMP7:%.*]] = phi i32 [ 0, [[COND_END]] ], [ [[CALL]], [[TMP5]] ], [ 46, [[ENTRY]] ]
+; DEFAULT-NEXT:    [[TMP8:%.*]] = mul i32 [[TMP6]], [[TMP7]]
+; DEFAULT-NEXT:    [[CALL33:%.*]] = tail call i32 (ptr, ...) @printf(ptr nonnull dereferenceable(1) @.str, i32 [[TMP8]])
 ; DEFAULT-NEXT:    ret i32 0
 ;
 ; OVERIDE-LABEL: @test_optsize(
@@ -115,16 +111,14 @@ define i32 @test_optsize(i32 %argc, ptr nocapture readonly %argv) local_unnamed_
 ; OVERIDE-NEXT:    [[TMP3:%.*]] = mul i32 [[CALL]], [[TMP2]]
 ; OVERIDE-NEXT:    [[TMP4:%.*]] = icmp sgt i32 [[CALL]], 0
 ; OVERIDE-NEXT:    [[COND_FR:%.*]] = freeze i1 [[TMP4]]
-; OVERIDE-NEXT:    br i1 [[COND_FR]], label [[COND_END_THREAD]], label [[TMP6:%.*]]
+; OVERIDE-NEXT:    br i1 [[COND_FR]], label [[TMP5:%.*]], label [[COND_END_THREAD]]
+; OVERIDE:       5:
+; OVERIDE-NEXT:    br label [[COND_END_THREAD]]
 ; OVERIDE:       cond.end.thread:
-; OVERIDE-NEXT:    [[TMP5:%.*]] = phi i32 [ [[TMP3]], [[COND_END]] ], [ 205962976, [[ENTRY:%.*]] ]
-; OVERIDE-NEXT:    [[COND3:%.*]] = phi i32 [ [[CALL]], [[COND_END]] ], [ 46, [[ENTRY]] ]
-; OVERIDE-NEXT:    br label [[TMP6]]
-; OVERIDE:       6:
-; OVERIDE-NEXT:    [[TMP7:%.*]] = phi i32 [ [[TMP5]], [[COND_END_THREAD]] ], [ [[TMP3]], [[COND_END]] ]
-; OVERIDE-NEXT:    [[TMP8:%.*]] = phi i32 [ [[COND3]], [[COND_END_THREAD]] ], [ 0, [[COND_END]] ]
-; OVERIDE-NEXT:    [[TMP9:%.*]] = mul i32 [[TMP7]], [[TMP8]]
-; OVERIDE-NEXT:    [[CALL33:%.*]] = tail call i32 (ptr, ...) @printf(ptr nonnull dereferenceable(1) @.str, i32 [[TMP9]])
+; OVERIDE-NEXT:    [[TMP6:%.*]] = phi i32 [ [[TMP3]], [[COND_END]] ], [ [[TMP3]], [[TMP5]] ], [ 205962976, [[ENTRY:%.*]] ]
+; OVERIDE-NEXT:    [[TMP7:%.*]] = phi i32 [ 0, [[COND_END]] ], [ [[CALL]], [[TMP5]] ], [ 46, [[ENTRY]] ]
+; OVERIDE-NEXT:    [[TMP8:%.*]] = mul i32 [[TMP6]], [[TMP7]]
+; OVERIDE-NEXT:    [[CALL33:%.*]] = tail call i32 (ptr, ...) @printf(ptr nonnull dereferenceable(1) @.str, i32 [[TMP8]])
 ; OVERIDE-NEXT:    ret i32 0
 ;
 entry: