@@ -276,6 +276,7 @@ class SimplifyCFGOpt {
276276 bool simplifyCleanupReturn (CleanupReturnInst *RI);
277277 bool simplifyUnreachable (UnreachableInst *UI);
278278 bool simplifySwitch (SwitchInst *SI, IRBuilder<> &Builder);
279+ bool simplifyDuplicateSwitchArms (SwitchInst *SI, DomTreeUpdater *DTU);
279280 bool simplifyIndirectBr (IndirectBrInst *IBI);
280281 bool simplifyBranch (BranchInst *Branch, IRBuilder<> &Builder);
281282 bool simplifyUncondBranch (BranchInst *BI, IRBuilder<> &Builder);
@@ -7436,6 +7437,179 @@ static bool simplifySwitchOfCmpIntrinsic(SwitchInst *SI, IRBuilderBase &Builder,
74367437 return true ;
74377438}
74387439
7440+ // / Checking whether two cases of SI are equal depends on the contents of the
7441+ // / BasicBlock and the incoming values of their successor PHINodes.
7442+ // / PHINode::getIncomingValueForBlock is O(|Preds|), so we'd like to avoid
7443+ // / calling this function on each BasicBlock every time isEqual is called,
7444+ // / especially since the same BasicBlock may be passed as an argument multiple
7445+ // / times. To do this, we can precompute a map of PHINode -> Pred BasicBlock ->
7446+ // / IncomingValue and add it in the Wrapper so isEqual can do O(1) checking
7447+ // / of the incoming values.
7448+ struct SwitchSuccWrapper {
7449+ // Keep so we can use SwitchInst::setSuccessor to do the replacement. It won't
7450+ // be important to equality though.
7451+ unsigned SuccNum;
7452+ BasicBlock *Dest;
7453+ DenseMap<PHINode *, SmallDenseMap<BasicBlock *, Value *, 8 >> *PhiPredIVs;
7454+ };
7455+
7456+ namespace llvm {
7457+ template <> struct DenseMapInfo <const SwitchSuccWrapper *> {
7458+ static const SwitchSuccWrapper *getEmptyKey () {
7459+ return static_cast <SwitchSuccWrapper *>(
7460+ DenseMapInfo<void *>::getEmptyKey ());
7461+ }
7462+ static const SwitchSuccWrapper *getTombstoneKey () {
7463+ return static_cast <SwitchSuccWrapper *>(
7464+ DenseMapInfo<void *>::getTombstoneKey ());
7465+ }
7466+ static unsigned getHashValue (const SwitchSuccWrapper *SSW) {
7467+ BasicBlock *Succ = SSW->Dest ;
7468+ BranchInst *BI = cast<BranchInst>(Succ->getTerminator ());
7469+ assert (BI->isUnconditional () &&
7470+ " Only supporting unconditional branches for now" );
7471+ assert (BI->getNumSuccessors () == 1 &&
7472+ " Expected unconditional branches to have one successor" );
7473+ assert (Succ->size () == 1 && " Expected just a single branch in the BB" );
7474+
7475+ // Since we assume the BB is just a single BranchInst with a single
7476+ // successor, we hash as the BB and the incoming Values of its successor
7477+ // PHIs. Initially, we tried to just use the successor BB as the hash, but
7478+ // including the incoming PHI values leads to better performance.
7479+ // We also tried to build a map from BB -> Succs.IncomingValues ahead of
7480+ // time and passing it in SwitchSuccWrapper, but this slowed down the
7481+ // average compile time without having any impact on the worst case compile
7482+ // time.
7483+ BasicBlock *BB = BI->getSuccessor (0 );
7484+ SmallVector<Value *> PhiValsForBB;
7485+ for (PHINode &Phi : BB->phis ())
7486+ PhiValsForBB.emplace_back ((*SSW->PhiPredIVs )[&Phi][BB]);
7487+
7488+ return hash_combine (
7489+ BB, hash_combine_range (PhiValsForBB.begin (), PhiValsForBB.end ()));
7490+ }
7491+ static bool isEqual (const SwitchSuccWrapper *LHS,
7492+ const SwitchSuccWrapper *RHS) {
7493+ auto EKey = DenseMapInfo<SwitchSuccWrapper *>::getEmptyKey ();
7494+ auto TKey = DenseMapInfo<SwitchSuccWrapper *>::getTombstoneKey ();
7495+ if (LHS == EKey || RHS == EKey || LHS == TKey || RHS == TKey)
7496+ return LHS == RHS;
7497+
7498+ BasicBlock *A = LHS->Dest ;
7499+ BasicBlock *B = RHS->Dest ;
7500+
7501+ // FIXME: we checked that the size of A and B are both 1 in
7502+ // simplifyDuplicateSwitchArms to make the Case list smaller to
7503+ // improve performance. If we decide to support BasicBlocks with more
7504+ // than just a single instruction, we need to check that A.size() ==
7505+ // B.size() here, and we need to check more than just the BranchInsts
7506+ // for equality.
7507+
7508+ BranchInst *ABI = cast<BranchInst>(A->getTerminator ());
7509+ BranchInst *BBI = cast<BranchInst>(B->getTerminator ());
7510+ assert (ABI->isUnconditional () && BBI->isUnconditional () &&
7511+ " Only supporting unconditional branches for now" );
7512+ if (ABI->getSuccessor (0 ) != BBI->getSuccessor (0 ))
7513+ return false ;
7514+
7515+ // Need to check that PHIs in successor have matching values
7516+ BasicBlock *Succ = ABI->getSuccessor (0 );
7517+ for (PHINode &Phi : Succ->phis ()) {
7518+ auto &PredIVs = (*LHS->PhiPredIVs )[&Phi];
7519+ if (PredIVs[A] != PredIVs[B])
7520+ return false ;
7521+ }
7522+
7523+ return true ;
7524+ }
7525+ };
7526+ } // namespace llvm
7527+
7528+ bool SimplifyCFGOpt::simplifyDuplicateSwitchArms (SwitchInst *SI,
7529+ DomTreeUpdater *DTU) {
7530+ // Build Cases. Skip BBs that are not candidates for simplification. Mark
7531+ // PHINodes which need to be processed into PhiPredIVs. We decide to process
7532+ // an entire PHI at once after the loop, opposed to calling
7533+ // getIncomingValueForBlock inside this loop, since each call to
7534+ // getIncomingValueForBlock is O(|Preds|).
7535+ SmallPtrSet<PHINode *, 8 > Phis;
7536+ SmallPtrSet<BasicBlock *, 8 > Seen;
7537+ DenseMap<PHINode *, SmallDenseMap<BasicBlock *, Value *, 8 >> PhiPredIVs;
7538+ SmallVector<SwitchSuccWrapper> Cases;
7539+ Cases.reserve (SI->getNumSuccessors ());
7540+
7541+ for (unsigned I = 0 ; I < SI->getNumSuccessors (); ++I) {
7542+ BasicBlock *BB = SI->getSuccessor (I);
7543+
7544+ // FIXME: Support more than just a single BranchInst. One way we could do
7545+ // this is by taking a hashing approach of all insts in BB.
7546+ if (BB->size () != 1 )
7547+ continue ;
7548+
7549+ // FIXME: This case needs some extra care because the terminators other than
7550+ // SI need to be updated.
7551+ if (BB->hasNPredecessorsOrMore (2 ))
7552+ continue ;
7553+
7554+ // FIXME: Relax that the terminator is a BranchInst by checking for equality
7555+ // on other kinds of terminators. We decide to only support unconditional
7556+ // branches for now for compile time reasons.
7557+ auto *BI = dyn_cast<BranchInst>(BB->getTerminator ());
7558+ if (!BI || BI->isConditional ())
7559+ continue ;
7560+
7561+ if (Seen.insert (BB).second ) {
7562+ // Keep track of which PHIs we need as keys in PhiPredIVs below.
7563+ for (BasicBlock *Succ : BI->successors ())
7564+ for (PHINode &Phi : Succ->phis ())
7565+ Phis.insert (&Phi);
7566+ }
7567+ Cases.emplace_back (SwitchSuccWrapper{I, BB, &PhiPredIVs});
7568+ }
7569+
7570+ // Precompute a data structure to improve performance of isEqual for
7571+ // SwitchSuccWrapper.
7572+ PhiPredIVs.reserve (Phis.size ());
7573+ for (PHINode *Phi : Phis) {
7574+ PhiPredIVs[Phi] =
7575+ SmallDenseMap<BasicBlock *, Value *, 8 >(Phi->getNumIncomingValues ());
7576+ for (auto &IV : Phi->incoming_values ())
7577+ PhiPredIVs[Phi].insert ({Phi->getIncomingBlock (IV), IV.get ()});
7578+ }
7579+
7580+ // Build a set such that if the SwitchSuccWrapper exists in the set and
7581+ // another SwitchSuccWrapper isEqual, then the equivalent SwitchSuccWrapper
7582+ // which is not in the set should be replaced with the one in the set. If the
7583+ // SwitchSuccWrapper is not in the set, then it should be added to the set so
7584+ // other SwitchSuccWrappers can check against it in the same manner. We use
7585+ // SwitchSuccWrapper instead of just BasicBlock because we'd like to pass
7586+ // around information to isEquality, getHashValue, and when doing the
7587+ // replacement with better performance.
7588+ DenseSet<const SwitchSuccWrapper *> ReplaceWith;
7589+ ReplaceWith.reserve (Cases.size ());
7590+
7591+ SmallVector<DominatorTree::UpdateType> Updates;
7592+ Updates.reserve (ReplaceWith.size ());
7593+ bool MadeChange = false ;
7594+ for (auto &SSW : Cases) {
7595+ // SSW is a candidate for simplification. If we find a duplicate BB,
7596+ // replace it.
7597+ const auto [It, Inserted] = ReplaceWith.insert (&SSW);
7598+ if (!Inserted) {
7599+ // We know that SI's parent BB no longer dominates the old case successor
7600+ // since we are making it dead.
7601+ Updates.push_back ({DominatorTree::Delete, SI->getParent (), SSW.Dest });
7602+ SI->setSuccessor (SSW.SuccNum , (*It)->Dest );
7603+ MadeChange = true ;
7604+ }
7605+ }
7606+
7607+ if (DTU)
7608+ DTU->applyUpdates (Updates);
7609+
7610+ return MadeChange;
7611+ }
7612+
74397613bool SimplifyCFGOpt::simplifySwitch (SwitchInst *SI, IRBuilder<> &Builder) {
74407614 BasicBlock *BB = SI->getParent ();
74417615
@@ -7496,6 +7670,9 @@ bool SimplifyCFGOpt::simplifySwitch(SwitchInst *SI, IRBuilder<> &Builder) {
74967670 hoistCommonCodeFromSuccessors (SI, !Options.HoistCommonInsts ))
74977671 return requestResimplify ();
74987672
7673+ if (simplifyDuplicateSwitchArms (SI, DTU))
7674+ return requestResimplify ();
7675+
74997676 return false ;
75007677}
75017678
0 commit comments