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

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 31 01:31:29 PDT 2024


================
@@ -710,10 +714,107 @@ 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) {
+  MapVector<BasicBlock *, Value *> UnionSet;
+  for (auto &V : IncomingA)
+    UnionSet.insert(V);
+
+  for (auto &V : IncomingB) {
+    if (UnionSet.contains(V.first) && UnionSet[V.first] != V.second)
+      return false;
+    // Either IncomingA does not have this value or IncomingA has the same
+    // value.
+    UnionSet.insert(V);
+  }
+
+  Merged.clear();
+  Merged.append(UnionSet.takeVector());
+  return true;
+}
+
 /// Add the real PHI value as soon as everything is set up
 void StructurizeCFG::setPhiValues() {
   SmallVector<PHINode *, 8> InsertedPhis;
   SSAUpdater Updater(&InsertedPhis);
+
+  DenseMap<PHINode *, std::shared_ptr<BBValueVector>> 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 &AddedPhi : AddedPhis) {
+    BasicBlock *To = AddedPhi.first;
+    if (!DeletedPhis.contains(To))
+      continue;
+    PhiMap &OldPhi = DeletedPhis[To];
+    for (const auto &PI : OldPhi) {
+      SmallVector<PHINode *> IncomingPHIs;
+      PHINode *Phi = PI.first;
+      for (const auto &VI : PI.second) {
+        // First, for each phi, check whether it has incoming value which is
+        // another phi.
+        if (PHINode *P = dyn_cast<PHINode>(VI.second))
+          IncomingPHIs.push_back(P);
+      }
+
+      auto GetUpdatedIncoming = [&](PHINode *Phi) {
+        return MergedPHIMap.contains(Phi) ? *MergedPHIMap[Phi]
+                                          : DeletedPhis[Phi->getParent()][Phi];
+      };
+      for (auto *OtherPhi : IncomingPHIs) {
+        // Skip phis that are not unrelated to the phi reconstruction for now.
+        if (!DeletedPhis.contains(OtherPhi->getParent()))
+          continue;
+
+        // Skip phis that were already merged with others.
+        if (MergedPHIMap.contains(Phi) && MergedPHIMap.contains(OtherPhi))
+          continue;
+
+        std::shared_ptr<BBValueVector> MergedIncomings;
+        if (MergedPHIMap.contains(Phi))
+          MergedIncomings = MergedPHIMap[Phi];
+        else if (MergedPHIMap.contains(OtherPhi))
+          MergedIncomings = MergedPHIMap[OtherPhi];
----------------
arsenm wrote:

You are doing the same map lookup twice. Use find? 

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


More information about the llvm-commits mailing list