[Mlir-commits] [mlir] 21f04b1 - Hold a queue of iterator ranges (not operations) in wouldOpBeTriviallyDead (#123642)

[llvmlistbot at llvm.org]

Author: Adam Paszke
Date: 2025-01-25T07:28:21-08:00
New Revision: 21f04b1458c52ba875a23b58b02cf6b1f8db0661

URL: https://github.com/llvm/llvm-project/commit/21f04b1458c52ba875a23b58b02cf6b1f8db0661
DIFF: https://github.com/llvm/llvm-project/commit/21f04b1458c52ba875a23b58b02cf6b1f8db0661.diff

LOG: Hold a queue of iterator ranges (not operations) in wouldOpBeTriviallyDead (#123642)

Ranges let us push the whole blocks onto the queue in constant time. If
one of the first ops in the block is side-effecting we'll be able to
provide the answer quickly. The previous implementation had to walk the
block and queue all the operations only to start traversing them again,
which was a considerable slowdown for compile times of large MLIR
programs in our benchmarks.

---------

Co-authored-by: Jacques Pienaar <jpienaar at google.com>

Added: 
    

Modified: 
    mlir/lib/Interfaces/SideEffectInterfaces.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Interfaces/SideEffectInterfaces.cpp b/mlir/lib/Interfaces/SideEffectInterfaces.cpp
index c9feb001a19844..59fd19310cea54 100644
--- a/mlir/lib/Interfaces/SideEffectInterfaces.cpp
+++ b/mlir/lib/Interfaces/SideEffectInterfaces.cpp
@@ -10,6 +10,7 @@
 
 #include "mlir/IR/SymbolTable.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include <utility>
 
 using namespace mlir;
 
@@ -41,10 +42,18 @@ bool mlir::isOpTriviallyDead(Operation *op) {
 /// allows for marking region operations as trivially dead without always being
 /// conservative of terminators.
 static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) {
-  // The set of operations to consider when checking for side effects.
-  SmallVector<Operation *, 1> effectingOps(1, rootOp);
+  // The set of operation intervals (end-exclusive) to consider when checking
+  // for side effects.
+  SmallVector<std::pair<Block::iterator, Block::iterator>, 1> effectingOps = {
+      std::make_pair(Block::iterator(rootOp), ++Block::iterator(rootOp))};
   while (!effectingOps.empty()) {
-    Operation *op = effectingOps.pop_back_val();
+    Block::iterator &it = effectingOps.back().first;
+    Block::iterator end = effectingOps.back().second;
+    if (it == end) {
+      effectingOps.pop_back();
+      continue;
+    }
+    mlir::Operation *op = &*(it++);
 
     // If the operation has recursive effects, push all of the nested operations
     // on to the stack to consider.
@@ -53,8 +62,7 @@ static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) {
     if (hasRecursiveEffects) {
       for (Region &region : op->getRegions()) {
         for (auto &block : region) {
-          for (auto &nestedOp : block)
-            effectingOps.push_back(&nestedOp);
+          effectingOps.push_back(std::make_pair(block.begin(), block.end()));
         }
       }
     }
@@ -86,10 +94,9 @@ static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) {
         return false;
       }
       continue;
-
-      // Otherwise, if the op has recursive side effects we can treat the
-      // operation itself as having no effects.
     }
+    // Otherwise, if the op only has recursive side effects we can treat the
+    // operation itself as having no effects. We will visit its children next.
     if (hasRecursiveEffects)
       continue;