[llvm] StructurizeCFG: Optimize phi insertion during ssa reconstruction (PR #101301)

[Sameer Sahasrabuddhe via llvm-commits]

================
@@ -710,10 +714,114 @@ void StructurizeCFG::findUndefBlocks(
   }
 }
 
+// Return true if two PHI nodes have compatible incoming values (for each
+// incoming block, either they have the same incoming value or only one PHI
+// node has a incoming value). And return the union of the incoming values
+// through \p Merged.
+bool StructurizeCFG::isCompatible(const BBValueVector &IncomingA,
+                                  const BBValueVector &IncomingB,
+                                  BBValueVector &Merged) {
+  DenseMap<BasicBlock *, Value *> UnionSet;
+  for (auto &V : IncomingA)
+    UnionSet.insert(V);
+
+  for (auto &[BB, V] : IncomingB) {
+    if (UnionSet.contains(BB) && UnionSet[BB] != V)
+      return false;
+    // Either IncomingA does not have this value or IncomingA has the same
+    // value.
+    UnionSet.insert({BB, V});
+  }
+
+  Merged.clear();
+  for (auto &[BB, V] : UnionSet)
+    Merged.push_back({BB, V});
+  return true;
+}
+
 /// Add the real PHI value as soon as everything is set up
 void StructurizeCFG::setPhiValues() {
   SmallVector<PHINode *, 8> InsertedPhis;
   SSAUpdater Updater(&InsertedPhis);
+
+  SmallVector<BBValueVector> BBValuesPool;
+  // Map PHINode to the index of the merged incoming values in BBValuesPool
+  DenseMap<PHINode *, unsigned> MergedPHIMap;
+  // Find out phi nodes that have compatible incoming values (either they have
+  // the same value for the same block or one have undefined value, see example
+  // below). We only search again the phi's that are referenced by another phi,
+  // which is the cases we care.
+  //
+  // For example (-- means no incoming value):
+  // phi1 : BB1:phi2   BB2:v  BB3:--
+  // phi2:  BB1:--     BB2:v  BB3:w
+  //
+  // Then we can merge these incoming values and let phi1, phi2 use the
+  // same set of incoming values:
+  //
+  // phi1&phi2: BB1:phi2  BB2:v  BB3:w
+  //
+  // By doing this, phi1 and phi2 would share more intermediate phi nodes.
+  // This would help reducing number of phi nodes during SSA reconstruction and
+  // get less COPY instructions finally.
+  //
+  // This should be correct, because if a phi node does not have incoming
+  // value from certain block, this means the block is not the predecessor
+  // of the parent block, so we actually don't care its incoming value.
+  for (const auto &[To, From] : AddedPhis) {
+    if (!DeletedPhis.contains(To))
+      continue;
+    PhiMap &OldPhi = DeletedPhis[To];
+    for (const auto &[Phi, Incomings] : OldPhi) {
+      SmallVector<PHINode *> IncomingPHIs;
+      for (const auto &[BB, V] : Incomings) {
+        // First, for each phi, check whether it has incoming value which is
+        // another phi.
+        if (PHINode *P = dyn_cast<PHINode>(V))
+          IncomingPHIs.push_back(P);
+      }
+
+      const auto GetUpdatedIncoming = [&](PHINode *Phi) {
+        if (auto It = MergedPHIMap.find(Phi); It != MergedPHIMap.end())
+          return BBValuesPool[It->second];
+        return DeletedPhis[Phi->getParent()][Phi];
+      };
+
+      for (auto *OtherPhi : IncomingPHIs) {
+        // Skip phis that are unrelated to the phi reconstruction for now.
+        if (!DeletedPhis.contains(OtherPhi->getParent()))
+          continue;
+
+        auto PhiIt = MergedPHIMap.find(Phi);
+        auto OtherPhiIt = MergedPHIMap.find(OtherPhi);
+        // Skip phis that were both already merged with others.
+        if (PhiIt != MergedPHIMap.end() && OtherPhiIt != MergedPHIMap.end())
+          continue;
+
+        unsigned PoolIndex;
+        if (PhiIt != MergedPHIMap.end()) {
+          PoolIndex = PhiIt->second;
+        } else if (OtherPhiIt != MergedPHIMap.end()) {
+          PoolIndex = OtherPhiIt->second;
+        } else {
+          PoolIndex = BBValuesPool.size();
+          BBValuesPool.push_back(BBValueVector());
+        }
+
+        const auto &Incoming = GetUpdatedIncoming(Phi);
+        const auto &OtherIncoming = GetUpdatedIncoming(OtherPhi);
+        if (isCompatible(Incoming, OtherIncoming, BBValuesPool[PoolIndex])) {
+          // union the incoming values
+          if (!MergedPHIMap.contains(Phi))
----------------
ssahasra wrote:

Kinda lost here. What does it mean if `MergedPHIMap` already contains `Phi`?

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