Skip to content

Commit fe81a79

Browse files
committed
Hold a queue of iterator ranges (not operations) in wouldOpBeTriviallyDead
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.
1 parent bf17016 commit fe81a79

File tree

1 file changed

+14
-8
lines changed

1 file changed

+14
-8
lines changed

mlir/lib/Interfaces/SideEffectInterfaces.cpp

Lines changed: 14 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,17 @@ 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+
if (it == end) {
51+
effectingOps.pop_back();
52+
}
53+
Block::iterator &it = effectingOps.back().first;
54+
Block::iterator end = effectingOps.back().second;
55+
mlir::Operation *op = &*(it++);
4856

4957
// If the operation has recursive effects, push all of the nested operations
5058
// on to the stack to consider.
@@ -53,8 +61,7 @@ static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) {
5361
if (hasRecursiveEffects) {
5462
for (Region &region : op->getRegions()) {
5563
for (auto &block : region) {
56-
for (auto &nestedOp : block)
57-
effectingOps.push_back(&nestedOp);
64+
effectingOps.push_back(std::make_pair(block.begin(), block.end()));
5865
}
5966
}
6067
}
@@ -86,10 +93,9 @@ static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) {
8693
return false;
8794
}
8895
continue;
89-
90-
// Otherwise, if the op has recursive side effects we can treat the
91-
// operation itself as having no effects.
9296
}
97+
// Otherwise, if the op only has recursive side effects we can treat the
98+
// operation itself as having no effects. We will visit its children next.
9399
if (hasRecursiveEffects)
94100
continue;
95101

0 commit comments

Comments
 (0)