fixup! another attempt at fixing hashing

Author: michaelmaitland

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

@@ -7444,10 +7444,14 @@ static bool simplifySwitchOfCmpIntrinsic(SwitchInst *SI, IRBuilderBase &Builder,
 /// especially since the same BasicBlock may be passed as an argument multiple
 /// times. To do this, we can precompute a map, PhiPredIVs, of PHINode -> Pred
 /// BasicBlock -> IncomingValue and add it in the Wrapper so isEqual can do O(1)
-/// checking of the incoming values.
+/// checking of the incoming values. We also want to use the incoming Phi
+/// values of a getCaseSuccessor to calculate the hash value. In order to avoid
+/// iterating all of the successor Phis on evry call to getHashValue, we
+/// precompute a list of incoming values from getCaseSucessor, PhiVals.
 struct CaseHandleWrapper {
   const SwitchInst::CaseHandle Case;
   DenseMap<PHINode *, DenseMap<BasicBlock *, Value *>> *PhiPredIVs;
+  SmallVector<Value *> *PhiVals;
 };
 
 namespace llvm {
@@ -7468,8 +7472,15 @@ template <> struct DenseMapInfo<const CaseHandleWrapper *> {
     assert(BI->getNumSuccessors() == 1 &&
            "Expected unconditional branches to have one successor");
     assert(Succ->size() == 1 && "Expected just a single branch in the BB");
+
+    // Since the BB is just a single BranchInst with a single successor, we hash
+    // the BB and the incoming Values of its successor incoming PHI values.
+    // Initially, we tried to just use the successor BB as the hash, but this
+    // has better performance.
     BasicBlock *BB = BI->getSuccessor(0);
-    return hash_value(BB);
+    return hash_combine(
+        hash_value(BB),
+        hash_combine_range(CHW->PhiVals->begin(), CHW->PhiVals->end()));
   }
   static bool isEqual(const CaseHandleWrapper *LHS,
                       const CaseHandleWrapper *RHS) {
@@ -7519,6 +7530,7 @@ bool SimplifyCFGOpt::simplifyDuplicateSwitchArms(SwitchInst *SI) {
   SmallPtrSet<PHINode *, 8> Phis;
   SmallPtrSet<BasicBlock *, 8> Seen;
   DenseMap<PHINode *, DenseMap<BasicBlock *, Value *>> PhiPredIVs;
+  DenseMap<BasicBlock *, SmallVector<Value *>> PhiVals;
   std::vector<CaseHandleWrapper> Cases;
   Cases.reserve(SI->getNumCases());
   for (auto &Case : SI->cases()) {
@@ -7551,17 +7563,24 @@ bool SimplifyCFGOpt::simplifyDuplicateSwitchArms(SwitchInst *SI) {
         for (PHINode &Phi : Succ->phis())
           Phis.insert(&Phi);
 
-    Cases.emplace_back(CaseHandleWrapper{Case, &PhiPredIVs});
+    PhiVals[BB] = SmallVector<Value *>();
+    Cases.emplace_back(CaseHandleWrapper{Case, &PhiPredIVs, &PhiVals[BB]});
   }
 
-  // Precompute the data structures to improve performance of isEqual for
-  // CaseHandleWrapper.
+  // Precompute the data structures to improve performance of isEqual and
+  // getHashValue for CaseHandleWrapper.
   PhiPredIVs.reserve(Phis.size());
   for (PHINode *Phi : Phis) {
     PhiPredIVs[Phi] =
         DenseMap<BasicBlock *, Value *>(Phi->getNumIncomingValues());
-    for (Use &IV : Phi->incoming_values())
-      PhiPredIVs[Phi].insert({Phi->getIncomingBlock(IV), IV.get()});
+    for (Use &IV : Phi->incoming_values()) {
+      BasicBlock *BB = Phi->getIncomingBlock(IV);
+      PhiPredIVs[Phi].insert({BB, IV.get()});
+      // Only need to track PhiVals for BBs we've added as keys in the loop
+      // above.
+      if (PhiVals.contains(BB))
+        PhiVals[BB].emplace_back(IV.get());
+    }
   }
 
   // Build a set such that if the CaseHandleWrapper exists in the set and