[llvm] [SimplifyCFG] Simplify identical predecessors (PR #173022)
Antonio Frighetto via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 5 01:26:10 PST 2026
================
@@ -8053,113 +8088,158 @@ template <> struct llvm::DenseMapInfo<const SwitchSuccWrapper *> {
if (ABI->getSuccessor(0) != BBI->getSuccessor(0))
return false;
- // Need to check that PHIs in successor have matching values
+ // Need to check that PHIs in successor have matching values.
BasicBlock *Succ = ABI->getSuccessor(0);
- for (PHINode &Phi : Succ->phis()) {
- auto &PredIVs = (*LHS->PhiPredIVs)[&Phi];
- if (PredIVs[A] != PredIVs[B])
- return false;
- }
-
- return true;
+ auto IfPhiIVMatch = [A, B, &PhiPredIVs = *LHS->PhiPredIVs](PHINode &Phi) {
+ // Replace O(|Pred|) Phi.getIncomingValueForBlock with this O(1) hashmap
+ // query.
+ auto &PredIVs = PhiPredIVs[&Phi];
+ return PredIVs[A] == PredIVs[B];
+ };
+ return all_of(Succ->phis(), IfPhiIVMatch);
}
};
-bool SimplifyCFGOpt::simplifyDuplicateSwitchArms(SwitchInst *SI,
- DomTreeUpdater *DTU) {
+// Merge identical BBs into one of them.
+static bool mergeIdenticalBBs(ArrayRef<BasicBlock *> Candidates,
+ DomTreeUpdater *DTU) {
+ if (Candidates.size() < 2)
+ return false;
+
// Build Cases. Skip BBs that are not candidates for simplification. Mark
// PHINodes which need to be processed into PhiPredIVs. We decide to process
// an entire PHI at once after the loop, opposed to calling
// getIncomingValueForBlock inside this loop, since each call to
// getIncomingValueForBlock is O(|Preds|).
- SmallPtrSet<PHINode *, 8> Phis;
- SmallPtrSet<BasicBlock *, 8> Seen;
- DenseMap<PHINode *, SmallDenseMap<BasicBlock *, Value *, 8>> PhiPredIVs;
- DenseMap<BasicBlock *, SmallVector<unsigned, 32>> BBToSuccessorIndexes;
- SmallVector<SwitchSuccWrapper> Cases;
- Cases.reserve(SI->getNumSuccessors());
-
- for (unsigned I = 0; I < SI->getNumSuccessors(); ++I) {
- BasicBlock *BB = SI->getSuccessor(I);
-
- // FIXME: Support more than just a single BranchInst. One way we could do
- // this is by taking a hashing approach of all insts in BB.
- if (BB->size() != 1)
- continue;
-
- // FIXME: Relax that the terminator is a BranchInst by checking for equality
- // on other kinds of terminators. We decide to only support unconditional
- // branches for now for compile time reasons.
- auto *BI = dyn_cast<BranchInst>(BB->getTerminator());
- if (!BI || BI->isConditional())
- continue;
-
- if (!Seen.insert(BB).second) {
- auto It = BBToSuccessorIndexes.find(BB);
- if (It != BBToSuccessorIndexes.end())
- It->second.emplace_back(I);
- continue;
- }
-
- // FIXME: This case needs some extra care because the terminators other than
- // SI need to be updated. For now, consider only backedges to the SI.
- if (BB->getUniquePredecessor() != SI->getParent())
- continue;
-
- // Keep track of which PHIs we need as keys in PhiPredIVs below.
- for (BasicBlock *Succ : BI->successors())
- Phis.insert_range(llvm::make_pointer_range(Succ->phis()));
+ EqualBBWrapper::Phi2IVsMap PhiPredIVs;
+ SmallVector<EqualBBWrapper> BBs2Merge;
+ BBs2Merge.reserve(Candidates.size());
+ SmallSetVector<PHINode *, 8> Phis;
- // Add the successor only if not previously visited.
- Cases.emplace_back(SwitchSuccWrapper{BB, &PhiPredIVs});
- BBToSuccessorIndexes[BB].emplace_back(I);
+ for (BasicBlock *BB : Candidates) {
+ BasicBlock *Succ = BB->getSingleSuccessor();
+ assert(Succ && "Expected unconditional BB");
+ BBs2Merge.emplace_back(EqualBBWrapper{BB, &PhiPredIVs});
+ Phis.insert_range(make_pointer_range(Succ->phis()));
}
// Precompute a data structure to improve performance of isEqual for
- // SwitchSuccWrapper.
+ // EqualBBWrapper.
PhiPredIVs.reserve(Phis.size());
for (PHINode *Phi : Phis) {
auto &IVs =
PhiPredIVs.try_emplace(Phi, Phi->getNumIncomingValues()).first->second;
+ // Pre-fill all incoming for O(1) lookup as Phi.getIncomingValueForBlock is
+ // O(|Pred|).
for (auto &IV : Phi->incoming_values())
IVs.insert({Phi->getIncomingBlock(IV), IV.get()});
}
- // Build a set such that if the SwitchSuccWrapper exists in the set and
- // another SwitchSuccWrapper isEqual, then the equivalent SwitchSuccWrapper
- // which is not in the set should be replaced with the one in the set. If the
- // SwitchSuccWrapper is not in the set, then it should be added to the set so
- // other SwitchSuccWrappers can check against it in the same manner. We use
- // SwitchSuccWrapper instead of just BasicBlock because we'd like to pass
- // around information to isEquality, getHashValue, and when doing the
- // replacement with better performance.
- DenseSet<const SwitchSuccWrapper *> ReplaceWith;
- ReplaceWith.reserve(Cases.size());
+ // Group duplicates using DenseSet with custom equality/hashing.
+ // Build a set such that if the EqualBBWrapper exists in the set and another
+ // EqualBBWrapper isEqual, then the equivalent EqualBBWrapper which is not in
+ // the set should be replaced with the one in the set. If the EqualBBWrapper
+ // is not in the set, then it should be added to the set so other
+ // EqualBBWrapper can check against it in the same manner. We use
+ // EqualBBWrapper instead of just BasicBlock because we'd like to pass around
+ // information to isEquality, getHashValue, and when doing the replacement
+ // with better performance.
+ DenseSet<const EqualBBWrapper *> Keep;
+ Keep.reserve(BBs2Merge.size());
SmallVector<DominatorTree::UpdateType> Updates;
- Updates.reserve(ReplaceWith.size());
+ Updates.reserve(BBs2Merge.size() * 2);
+
bool MadeChange = false;
- for (auto &SSW : Cases) {
- // SSW is a candidate for simplification. If we find a duplicate BB,
- // replace it.
- const auto [It, Inserted] = ReplaceWith.insert(&SSW);
- if (!Inserted) {
- // We know that SI's parent BB no longer dominates the old case successor
- // since we are making it dead.
- Updates.push_back({DominatorTree::Delete, SI->getParent(), SSW.Dest});
- const auto &Successors = BBToSuccessorIndexes.at(SSW.Dest);
- for (unsigned Idx : Successors)
- SI->setSuccessor(Idx, (*It)->Dest);
- MadeChange = true;
+
+ // Helper: redirect all edges X -> DeadPred to X -> LivePred.
+ auto RedirectIncomingEdges = [&](BasicBlock *Dead, BasicBlock *Live) {
+ SmallSetVector<BasicBlock *, 8> DeadPreds(llvm::from_range,
+ predecessors(Dead));
+ if (DTU) {
+ // All predecessors of DeadPred (except the common predecessor) will be
+ // moved to LivePred.
+ Updates.reserve(Updates.size() + DeadPreds.size() * 2);
+ SmallPtrSet<BasicBlock *, 16> LivePreds(llvm::from_range,
+ predecessors(Live));
+ for (BasicBlock *PredOfDead : DeadPreds) {
+ // Do not modify those common predecessors of DeadPred and LivePred.
+ if (!LivePreds.contains(PredOfDead))
+ Updates.push_back({DominatorTree::Insert, PredOfDead, Live});
+ Updates.push_back({DominatorTree::Delete, PredOfDead, Dead});
+ }
+ }
+ LLVM_DEBUG(dbgs() << "Replacing duplicate pred BB ";
+ Dead->printAsOperand(dbgs()); dbgs() << " with pred ";
+ Live->printAsOperand(dbgs()); dbgs() << " for ";
+ Live->getSingleSuccessor()->printAsOperand(dbgs());
+ dbgs() << "\n");
+ // Replace successors in all predecessors of DeadPred.
+ for (BasicBlock *PredOfDead : DeadPreds) {
+ Instruction *T = PredOfDead->getTerminator();
+ T->replaceSuccessorWith(Dead, Live);
}
+ };
+
+ // Try to eliminate duplicate predecessors.
+ for (const auto &EBW : BBs2Merge) {
+ // Pred is a candidate for simplification. If we find a duplicate BB,
+ // replace it.
+ const auto &[It, Inserted] = Keep.insert(&EBW);
+ if (Inserted)
+ continue;
+
+ // Found duplicate: merge P into canonical predecessor It->Pred.
+ BasicBlock *KeepBB = (*It)->BB;
+ BasicBlock *DeadBB = EBW.BB;
+
+ // Avoid merging if either is the other's predecessor in weird ways.
+ if (KeepBB == DeadBB)
+ continue;
+
+ // Redirect all edges into DeadPred to KeepPred.
+ RedirectIncomingEdges(DeadBB, KeepBB);
+
+ // Now DeadBB should become unreachable; leave DCE to later,
+ // but we can try to simplify it if it only branches to Succ.
+ // (We won't erase here to keep the routine simple and DT-safe.)
+ assert(pred_empty(DeadBB) && "DeadBB shoud be unreachable.");
----------------
antoniofrighetto wrote:
```suggestion
assert(pred_empty(DeadBB) && "DeadBB should be unreachable.");
```
https://github.com/llvm/llvm-project/pull/173022
More information about the llvm-commits
mailing list