[llvm] [SimplifyCFG] Simplify switch instruction that has duplicate arms (PR #114262)
Michael Maitland via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 14 08:01:45 PST 2024
================
@@ -7436,6 +7437,187 @@ static bool simplifySwitchOfCmpIntrinsic(SwitchInst *SI, IRBuilderBase &Builder,
return true;
}
+/// Checking whether two cases of SI are equal depends on the contents of the
+/// BasicBlock and the incoming values of their successor PHINodes.
+/// PHINode::getIncomingValueForBlock is O(|Preds|), so we'd like to avoid
+/// calling this function on each BasicBlock every time isEqual is called,
+/// especially since the same BasicBlock may be passed as an argument multiple
+/// times. To do this, we can precompute a map of PHINode -> Pred BasicBlock ->
+/// IncomingValue and add it in the Wrapper so isEqual can do O(1) checking
+/// of the incoming values.
+struct SwitchSuccWrapper {
+ // Keep so we can use SwitchInst::setSuccessor to do the replacement. It won't
+ // be important to equality though.
+ unsigned SuccNum;
+ BasicBlock *Dest;
+ DenseMap<PHINode *, DenseMap<BasicBlock *, Value *>> *PhiPredIVs;
+};
+
+namespace llvm {
+template <> struct DenseMapInfo<const SwitchSuccWrapper *> {
+ static const SwitchSuccWrapper *getEmptyKey() {
+ return static_cast<SwitchSuccWrapper *>(
+ DenseMapInfo<void *>::getEmptyKey());
+ }
+ static const SwitchSuccWrapper *getTombstoneKey() {
+ return static_cast<SwitchSuccWrapper *>(
+ DenseMapInfo<void *>::getTombstoneKey());
+ }
+ static unsigned getHashValue(const SwitchSuccWrapper *SSW) {
+ BasicBlock *Succ = SSW->Dest;
+ BranchInst *BI = cast<BranchInst>(Succ->getTerminator());
+ assert(BI->isUnconditional() &&
+ "Only supporting unconditional branches for now");
+ assert(BI->getNumSuccessors() == 1 &&
+ "Expected unconditional branches to have one successor");
+ assert(Succ->size() == 1 && "Expected just a single branch in the BB");
+
+ // Since we assume the BB is just a single BranchInst with a single
+ // succsessor, we hash as the BB and the incoming Values of its successor
+ // PHIs. Initially, we tried to just use the successor BB as the hash, but
+ // this had poor performance. We find that the extra computation of getting
+ // the incoming PHI values here leads to better performance on overall Set
+ // performance. We also tried to build a map from BB -> Succs.IncomingValues
+ // ahead of time and passing it in SwitchSuccWrapper, but this slowed down
+ // the average compile time without having any impact on the worst case
+ // compile time.
+ BasicBlock *BB = BI->getSuccessor(0);
+ SmallVector<Value *> PhiValsForBB;
+ for (PHINode &Phi : BB->phis())
+ PhiValsForBB.emplace_back((*SSW->PhiPredIVs)[&Phi][BB]);
+
+ return hash_combine(
+ BB, hash_combine_range(PhiValsForBB.begin(), PhiValsForBB.end()));
+ }
+ static bool isEqual(const SwitchSuccWrapper *LHS,
+ const SwitchSuccWrapper *RHS) {
+ auto EKey = DenseMapInfo<SwitchSuccWrapper *>::getEmptyKey();
+ auto TKey = DenseMapInfo<SwitchSuccWrapper *>::getTombstoneKey();
+ if (LHS == EKey || RHS == EKey || LHS == TKey || RHS == TKey)
+ return LHS == RHS;
+
+ BasicBlock *A = LHS->Dest;
+ BasicBlock *B = RHS->Dest;
+
+ // FIXME: we checked that the size of A and B are both 1 in
+ // simplifyDuplicateSwitchArms to make the Case list smaller to
+ // improve performance. If we decide to support BasicBlocks with more
+ // than just a single instruction, we need to check that A.size() ==
+ // B.size() here, and we need to check more than just the BranchInsts
+ // for equality.
+
+ BranchInst *ABI = cast<BranchInst>(A->getTerminator());
+ BranchInst *BBI = cast<BranchInst>(B->getTerminator());
+ assert(ABI->isUnconditional() && BBI->isUnconditional() &&
+ "Only supporting unconditional branches for now");
+ assert(ABI->getNumSuccessors() == 1 &&
+ "Expected unconditional branches to have one successor");
+ if (ABI->getSuccessor(0) != BBI->getSuccessor(0))
+ return false;
+
+ // 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;
+ }
+};
+} // namespace llvm
+
+bool SimplifyCFGOpt::simplifyDuplicateSwitchArms(SwitchInst *SI,
+ DomTreeUpdater *DTU) {
+ // 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 *, DenseMap<BasicBlock *, Value *>> PhiPredIVs;
+ SmallVector<SwitchSuccWrapper> Cases;
+ Cases.reserve(SI->getNumCases());
----------------
michaelmaitland wrote:
updated
https://github.com/llvm/llvm-project/pull/114262
More information about the llvm-commits
mailing list