[llvm] 2c37334 - [Transforms] Speed up SSAUpdater::FindExistingPHI (#100281)

[via llvm-commits]

Author: William Junda Huang
Date: 2024-07-29T12:15:45-04:00
New Revision: 2c37334d8dce7ef7d3ecc8c80522fe73010b12cc

URL: https://github.com/llvm/llvm-project/commit/2c37334d8dce7ef7d3ecc8c80522fe73010b12cc
DIFF: https://github.com/llvm/llvm-project/commit/2c37334d8dce7ef7d3ecc8c80522fe73010b12cc.diff

LOG: [Transforms] Speed up SSAUpdater::FindExistingPHI (#100281)

In SSAUpdater::FindExistingPHI, the cleanup function is inefficient for
large function with many blocks because it clears the Phi value
reference for every block if not matched for every phi value, even if
most blocks are not modified by CheckIfPHIMatches. This behavior is
particularly slow for large functions because the complexity is Θ(# PHI
* # BBs).


Updated the behavior to only clear modified blocks, which in practice
has a much less complexity because of early exit on PHI mismatch.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h b/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
index 28ff6c4c7927d..010d6b0de9f6a 100644
--- a/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
+++ b/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
@@ -15,6 +15,7 @@
 #define LLVM_TRANSFORMS_UTILS_SSAUPDATERIMPL_H
 
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Debug.h"
@@ -413,26 +414,33 @@ class SSAUpdaterImpl {
   /// FindExistingPHI - Look through the PHI nodes in a block to see if any of
   /// them match what is needed.
   void FindExistingPHI(BlkT *BB, BlockListTy *BlockList) {
+    SmallVector<BBInfo *, 20> TaggedBlocks;
     for (auto &SomePHI : BB->phis()) {
-      if (CheckIfPHIMatches(&SomePHI)) {
+      if (CheckIfPHIMatches(&SomePHI, TaggedBlocks)) {
         RecordMatchingPHIs(BlockList);
         break;
       }
-      // Match failed: clear all the PHITag values.
-      for (typename BlockListTy::iterator I = BlockList->begin(),
-             E = BlockList->end(); I != E; ++I)
-        (*I)->PHITag = nullptr;
     }
   }
 
   /// CheckIfPHIMatches - Check if a PHI node matches the placement and values
   /// in the BBMap.
-  bool CheckIfPHIMatches(PhiT *PHI) {
+  bool CheckIfPHIMatches(PhiT *PHI, SmallVectorImpl<BBInfo *> &TaggedBlocks) {
+    // Match failed: clear all the PHITag values. Only need to clear visited
+    // blocks.
+    auto Cleanup = make_scope_exit([&]() {
+      for (BBInfo *TaggedBlock : TaggedBlocks)
+        TaggedBlock->PHITag = nullptr;
+      TaggedBlocks.clear();
+    });
+
     SmallVector<PhiT *, 20> WorkList;
     WorkList.push_back(PHI);
 
     // Mark that the block containing this PHI has been visited.
-    BBMap[PHI->getParent()]->PHITag = PHI;
+    BBInfo *PHIBlock = BBMap[PHI->getParent()];
+    PHIBlock->PHITag = PHI;
+    TaggedBlocks.push_back(PHIBlock);
 
     while (!WorkList.empty()) {
       PHI = WorkList.pop_back_val();
@@ -465,10 +473,13 @@ class SSAUpdaterImpl {
           return false;
         }
         PredInfo->PHITag = IncomingPHIVal;
+        TaggedBlocks.push_back(PredInfo);
 
         WorkList.push_back(IncomingPHIVal);
       }
     }
+    // Match found, keep PHITags.
+    Cleanup.release();
     return true;
   }