[llvm] [SimplifyCFG] Simplify switch instruction that has duplicate arms (PR #114262)

Yingwei Zheng via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 2 00:44:13 PDT 2024


================
@@ -7436,6 +7437,185 @@ static bool simplifySwitchOfCmpIntrinsic(SwitchInst *SI, IRBuilderBase &Builder,
   return true;
 }
 
+/// Checking whether two CaseHandle.getCaseSuccessor() are equal depends on
+/// the incoming values of their successor PHINodes.
+/// PHINode::getIncomingValueForBlock has 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 CaseHandleWrapper {
+  const SwitchInst::CaseHandle Case;
+  DenseMap<PHINode *, DenseMap<BasicBlock *, Value *>> &PhiPredIVs;
+};
+
+namespace llvm {
+template <> struct DenseMapInfo<const CaseHandleWrapper *> {
+  static const CaseHandleWrapper *getEmptyKey() {
+    return reinterpret_cast<CaseHandleWrapper *>(
+        DenseMapInfo<void *>::getEmptyKey());
+  }
+  static const CaseHandleWrapper *getTombstoneKey() {
+    return reinterpret_cast<CaseHandleWrapper *>(
+        DenseMapInfo<void *>::getTombstoneKey());
+  }
+  static unsigned getHashValue(const CaseHandleWrapper *CT) {
+    BasicBlock *Succ = CT->Case.getCaseSuccessor();
+    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 PHIs.
+    // Initially, we tried to just use the sucessor 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.
+    BasicBlock *BB = BI->getSuccessor(0);
+    SmallVector<Value *> PhiValsForBB;
+    for (PHINode &Phi : BB->phis())
+      PhiValsForBB.emplace_back(CT->PhiPredIVs[&Phi][BB]);
+
+    return hash_combine(
+        BB, hash_combine_range(PhiValsForBB.begin(), PhiValsForBB.end()));
+  }
+  static bool isEqual(const CaseHandleWrapper *LHS,
+                      const CaseHandleWrapper *RHS) {
+    auto EKey = DenseMapInfo<CaseHandleWrapper *>::getEmptyKey();
+    auto TKey = DenseMapInfo<CaseHandleWrapper *>::getTombstoneKey();
+    if (LHS == EKey || RHS == EKey || LHS == TKey || RHS == TKey)
+      return LHS == RHS;
+
+    auto IsBranchEq = [](BranchInst *A, BranchInst *B) {
+      assert(A->isUnconditional() && B->isUnconditional() &&
+             "Only supporting unconditional branches for now");
+      assert(A->getNumSuccessors() == 1 && B->getNumSuccessors() == 1 &&
+             "Expected unconditional branches to have one successor");
+
+      if (A->getSuccessor(0) != B->getSuccessor(0))
+        return false;
+
+      return true;
+    };
+
+    auto IsBBEq =
+        [&IsBranchEq](
+            BasicBlock *A, BasicBlock *B,
+            DenseMap<PHINode *, DenseMap<BasicBlock *, Value *>> &PhiPredIVs) {
+          // 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());
+          if (!IsBranchEq(ABI, cast<BranchInst>(B->getTerminator())))
+            return false;
+
+          // Need to check that PHIs in successor have matching values
+          assert(ABI->getNumSuccessors() == 1 &&
+                 "Expected unconditional branches to have one successor");
+          BasicBlock *Succ = ABI->getSuccessor(0);
+          for (PHINode &Phi : Succ->phis()) {
+            auto PredIVs = PhiPredIVs[&Phi];
----------------
dtcxzyw wrote:

```suggestion
            auto &PredIVs = PhiPredIVs[&Phi];
```
Unnecessary copy.


https://github.com/llvm/llvm-project/pull/114262


More information about the llvm-commits mailing list