@@ -276,6 +276,7 @@ class SimplifyCFGOpt {
276
276
bool simplifyCleanupReturn (CleanupReturnInst *RI);
277
277
bool simplifyUnreachable (UnreachableInst *UI);
278
278
bool simplifySwitch (SwitchInst *SI, IRBuilder<> &Builder);
279
+ bool simplifyDuplicateSwitchArms (SwitchInst *SI, DomTreeUpdater *DTU);
279
280
bool simplifyIndirectBr (IndirectBrInst *IBI);
280
281
bool simplifyBranch (BranchInst *Branch, IRBuilder<> &Builder);
281
282
bool simplifyUncondBranch (BranchInst *BI, IRBuilder<> &Builder);
@@ -7436,6 +7437,179 @@ static bool simplifySwitchOfCmpIntrinsic(SwitchInst *SI, IRBuilderBase &Builder,
7436
7437
return true ;
7437
7438
}
7438
7439
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
+
7439
7613
bool SimplifyCFGOpt::simplifySwitch (SwitchInst *SI, IRBuilder<> &Builder) {
7440
7614
BasicBlock *BB = SI->getParent ();
7441
7615
@@ -7496,6 +7670,9 @@ bool SimplifyCFGOpt::simplifySwitch(SwitchInst *SI, IRBuilder<> &Builder) {
7496
7670
hoistCommonCodeFromSuccessors (SI, !Options.HoistCommonInsts ))
7497
7671
return requestResimplify ();
7498
7672
7673
+ if (simplifyDuplicateSwitchArms (SI, DTU))
7674
+ return requestResimplify ();
7675
+
7499
7676
return false ;
7500
7677
}
7501
7678
0 commit comments