Skip to content

Commit 21f04b1

Browse files
apaszkejpienaar
andauthored
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 <[email protected]>
1 parent 9325a61 commit 21f04b1

File tree

1 file changed

+15
-8
lines changed

1 file changed

+15
-8
lines changed

mlir/lib/Interfaces/SideEffectInterfaces.cpp

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include "mlir/IR/SymbolTable.h"
1212
#include "llvm/ADT/SmallPtrSet.h"
13+
#include <utility>
1314

1415
using namespace mlir;
1516

@@ -41,10 +42,18 @@ bool mlir::isOpTriviallyDead(Operation *op) {
4142
/// allows for marking region operations as trivially dead without always being
4243
/// conservative of terminators.
4344
static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) {
44-
// The set of operations to consider when checking for side effects.
45-
SmallVector<Operation *, 1> effectingOps(1, rootOp);
45+
// The set of operation intervals (end-exclusive) to consider when checking
46+
// for side effects.
47+
SmallVector<std::pair<Block::iterator, Block::iterator>, 1> effectingOps = {
48+
std::make_pair(Block::iterator(rootOp), ++Block::iterator(rootOp))};
4649
while (!effectingOps.empty()) {
47-
Operation *op = effectingOps.pop_back_val();
50+
Block::iterator &it = effectingOps.back().first;
51+
Block::iterator end = effectingOps.back().second;
52+
if (it == end) {
53+
effectingOps.pop_back();
54+
continue;
55+
}
56+
mlir::Operation *op = &*(it++);
4857

4958
// If the operation has recursive effects, push all of the nested operations
5059
// on to the stack to consider.
@@ -53,8 +62,7 @@ static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) {
5362
if (hasRecursiveEffects) {
5463
for (Region &region : op->getRegions()) {
5564
for (auto &block : region) {
56-
for (auto &nestedOp : block)
57-
effectingOps.push_back(&nestedOp);
65+
effectingOps.push_back(std::make_pair(block.begin(), block.end()));
5866
}
5967
}
6068
}
@@ -86,10 +94,9 @@ static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) {
8694
return false;
8795
}
8896
continue;
89-
90-
// Otherwise, if the op has recursive side effects we can treat the
91-
// operation itself as having no effects.
9297
}
98+
// Otherwise, if the op only has recursive side effects we can treat the
99+
// operation itself as having no effects. We will visit its children next.
93100
if (hasRecursiveEffects)
94101
continue;
95102

0 commit comments

Comments
 (0)