Skip to content

Commit 914e607

Browse files
[mlir][IR][NFC] Rename notify*Removed to notify*Erased (#82253)
Rename listener callback names: * `notifyOperationRemoved` -> `notifyOperationErased` * `notifyBlockRemoved` -> `notifyBlockErased` The current callback names are misnomers. The callbacks are triggered when an operation/block is erased, not when it is removed (unlinked). E.g.: ```c++ /// Notify the listener that the specified operation is about to be erased. /// At this point, the operation has zero uses. /// /// Note: This notification is not triggered when unlinking an operation. virtual void notifyOperationErased(Operation *op) {} ``` This change is in preparation of adding listener support to the dialect conversion. The dialect conversion internally unlinks IR before erasing it at a later point of time. There is an important difference between "remove" and "erase". Lister callback names should be accurate to avoid confusion.
1 parent d2a26a7 commit 914e607

File tree

10 files changed

+70
-70
lines changed

10 files changed

+70
-70
lines changed

mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1012,7 +1012,7 @@ class TrackingListener : public RewriterBase::Listener,
10121012
private:
10131013
friend class TransformRewriter;
10141014

1015-
void notifyOperationRemoved(Operation *op) override;
1015+
void notifyOperationErased(Operation *op) override;
10161016

10171017
void notifyOperationReplaced(Operation *op, ValueRange newValues) override;
10181018
using Listener::notifyOperationReplaced;

mlir/include/mlir/IR/PatternMatch.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ class RewriterBase : public OpBuilder {
404404

405405
/// Notify the listener that the specified block is about to be erased.
406406
/// At this point, the block has zero uses.
407-
virtual void notifyBlockRemoved(Block *block) {}
407+
virtual void notifyBlockErased(Block *block) {}
408408

409409
/// Notify the listener that the specified operation was modified in-place.
410410
virtual void notifyOperationModified(Operation *op) {}
@@ -430,7 +430,7 @@ class RewriterBase : public OpBuilder {
430430
/// At this point, the operation has zero uses.
431431
///
432432
/// Note: This notification is not triggered when unlinking an operation.
433-
virtual void notifyOperationRemoved(Operation *op) {}
433+
virtual void notifyOperationErased(Operation *op) {}
434434

435435
/// Notify the listener that the pattern failed to match the given
436436
/// operation, and provide a callback to populate a diagnostic with the
@@ -457,9 +457,9 @@ class RewriterBase : public OpBuilder {
457457
Region::iterator previousIt) override {
458458
listener->notifyBlockInserted(block, previous, previousIt);
459459
}
460-
void notifyBlockRemoved(Block *block) override {
460+
void notifyBlockErased(Block *block) override {
461461
if (auto *rewriteListener = dyn_cast<RewriterBase::Listener>(listener))
462-
rewriteListener->notifyBlockRemoved(block);
462+
rewriteListener->notifyBlockErased(block);
463463
}
464464
void notifyOperationModified(Operation *op) override {
465465
if (auto *rewriteListener = dyn_cast<RewriterBase::Listener>(listener))
@@ -474,9 +474,9 @@ class RewriterBase : public OpBuilder {
474474
if (auto *rewriteListener = dyn_cast<RewriterBase::Listener>(listener))
475475
rewriteListener->notifyOperationReplaced(op, replacement);
476476
}
477-
void notifyOperationRemoved(Operation *op) override {
477+
void notifyOperationErased(Operation *op) override {
478478
if (auto *rewriteListener = dyn_cast<RewriterBase::Listener>(listener))
479-
rewriteListener->notifyOperationRemoved(op);
479+
rewriteListener->notifyOperationErased(op);
480480
}
481481
void notifyMatchFailure(
482482
Location loc,

mlir/lib/Dialect/Bufferization/Transforms/Bufferize.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ class BufferizationRewriter : public IRRewriter, public RewriterBase::Listener {
369369
}
370370

371371
protected:
372-
void notifyOperationRemoved(Operation *op) override {
372+
void notifyOperationErased(Operation *op) override {
373373
erasedOps.insert(op);
374374
// Erase if present.
375375
toMemrefOps.erase(op);

mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,8 @@ class NewOpsListener : public RewriterBase::ForwardingListener {
250250
assert(inserted.second && "expected newly created op");
251251
}
252252

253-
void notifyOperationRemoved(Operation *op) override {
254-
ForwardingListener::notifyOperationRemoved(op);
253+
void notifyOperationErased(Operation *op) override {
254+
ForwardingListener::notifyOperationErased(op);
255255
op->walk([&](Operation *op) { newOps.erase(op); });
256256
}
257257

mlir/lib/Dialect/Transform/IR/TransformInterfaces.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1274,7 +1274,7 @@ void transform::TrackingListener::notifyMatchFailure(
12741274
});
12751275
}
12761276

1277-
void transform::TrackingListener::notifyOperationRemoved(Operation *op) {
1277+
void transform::TrackingListener::notifyOperationErased(Operation *op) {
12781278
// TODO: Walk can be removed when D144193 has landed.
12791279
op->walk([&](Operation *op) {
12801280
// Remove mappings for result values.

mlir/lib/IR/PatternMatch.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ void RewriterBase::eraseOp(Operation *op) {
209209
assert(mayBeGraphRegion(*op->getParentRegion()) &&
210210
"expected that op has no uses");
211211
#endif // NDEBUG
212-
rewriteListener->notifyOperationRemoved(op);
212+
rewriteListener->notifyOperationErased(op);
213213

214214
// Explicitly drop all uses in case the op is in a graph region.
215215
op->dropAllUses();
@@ -265,7 +265,7 @@ void RewriterBase::eraseBlock(Block *block) {
265265

266266
// Notify the listener that the block is about to be removed.
267267
if (auto *rewriteListener = dyn_cast_if_present<Listener>(listener))
268-
rewriteListener->notifyBlockRemoved(block);
268+
rewriteListener->notifyBlockErased(block);
269269

270270
block->erase();
271271
}

mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ struct ExpensiveChecks : public RewriterBase::ForwardingListener {
130130
/// Invalidate the finger print of the given op, i.e., remove it from the map.
131131
void invalidateFingerPrint(Operation *op) { fingerprints.erase(op); }
132132

133-
void notifyBlockRemoved(Block *block) override {
134-
RewriterBase::ForwardingListener::notifyBlockRemoved(block);
133+
void notifyBlockErased(Block *block) override {
134+
RewriterBase::ForwardingListener::notifyBlockErased(block);
135135

136136
// The block structure (number of blocks, types of block arguments, etc.)
137137
// is part of the fingerprint of the parent op.
@@ -152,8 +152,8 @@ struct ExpensiveChecks : public RewriterBase::ForwardingListener {
152152
invalidateFingerPrint(op);
153153
}
154154

155-
void notifyOperationRemoved(Operation *op) override {
156-
RewriterBase::ForwardingListener::notifyOperationRemoved(op);
155+
void notifyOperationErased(Operation *op) override {
156+
RewriterBase::ForwardingListener::notifyOperationErased(op);
157157
op->walk([this](Operation *op) { invalidateFingerPrint(op); });
158158
}
159159

@@ -345,7 +345,7 @@ class GreedyPatternRewriteDriver : public PatternRewriter,
345345
/// Notify the driver that the specified operation was removed. Update the
346346
/// worklist as needed: The operation and its children are removed from the
347347
/// worklist.
348-
void notifyOperationRemoved(Operation *op) override;
348+
void notifyOperationErased(Operation *op) override;
349349

350350
/// Notify the driver that the specified operation was replaced. Update the
351351
/// worklist as needed: New users are added enqueued.
@@ -384,7 +384,7 @@ class GreedyPatternRewriteDriver : public PatternRewriter,
384384
Region::iterator previousIt) override;
385385

386386
/// Notify the driver that the given block is about to be removed.
387-
void notifyBlockRemoved(Block *block) override;
387+
void notifyBlockErased(Block *block) override;
388388

389389
/// For debugging only: Notify the driver of a pattern match failure.
390390
void
@@ -647,9 +647,9 @@ void GreedyPatternRewriteDriver::notifyBlockInserted(
647647
config.listener->notifyBlockInserted(block, previous, previousIt);
648648
}
649649

650-
void GreedyPatternRewriteDriver::notifyBlockRemoved(Block *block) {
650+
void GreedyPatternRewriteDriver::notifyBlockErased(Block *block) {
651651
if (config.listener)
652-
config.listener->notifyBlockRemoved(block);
652+
config.listener->notifyBlockErased(block);
653653
}
654654

655655
void GreedyPatternRewriteDriver::notifyOperationInserted(Operation *op,
@@ -689,7 +689,7 @@ void GreedyPatternRewriteDriver::addOperandsToWorklist(ValueRange operands) {
689689
}
690690
}
691691

692-
void GreedyPatternRewriteDriver::notifyOperationRemoved(Operation *op) {
692+
void GreedyPatternRewriteDriver::notifyOperationErased(Operation *op) {
693693
LLVM_DEBUG({
694694
logger.startLine() << "** Erase : '" << op->getName() << "'(" << op
695695
<< ")\n";
@@ -707,7 +707,7 @@ void GreedyPatternRewriteDriver::notifyOperationRemoved(Operation *op) {
707707
#endif // NDEBUG
708708

709709
if (config.listener)
710-
config.listener->notifyOperationRemoved(op);
710+
config.listener->notifyOperationErased(op);
711711

712712
addOperandsToWorklist(op->getOperands());
713713
worklist.remove(op);
@@ -901,8 +901,8 @@ class MultiOpPatternRewriteDriver : public GreedyPatternRewriteDriver {
901901
LogicalResult simplify(ArrayRef<Operation *> ops, bool *changed = nullptr) &&;
902902

903903
private:
904-
void notifyOperationRemoved(Operation *op) override {
905-
GreedyPatternRewriteDriver::notifyOperationRemoved(op);
904+
void notifyOperationErased(Operation *op) override {
905+
GreedyPatternRewriteDriver::notifyOperationErased(op);
906906
if (survivingOps)
907907
survivingOps->erase(op);
908908
}

mlir/test/Transforms/test-strict-pattern-driver.mlir

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ func.func @test_replace_with_new_op() {
5252
// -----
5353

5454
// CHECK-EN: notifyOperationInserted: test.erase_op, was unlinked
55-
// CHECK-EN: notifyOperationRemoved: test.replace_with_new_op
56-
// CHECK-EN: notifyOperationRemoved: test.erase_op
55+
// CHECK-EN: notifyOperationErased: test.replace_with_new_op
56+
// CHECK-EN: notifyOperationErased: test.erase_op
5757
// CHECK-EN-LABEL: func @test_replace_with_erase_op
5858
// CHECK-EN-SAME: {pattern_driver_all_erased = true, pattern_driver_changed = true}
5959
// CHECK-EN-NOT: "test.replace_with_new_op"
@@ -91,10 +91,10 @@ func.func @test_trigger_rewrite_through_block() {
9191

9292
// -----
9393

94-
// CHECK-AN: notifyOperationRemoved: test.foo_b
95-
// CHECK-AN: notifyOperationRemoved: test.foo_a
96-
// CHECK-AN: notifyOperationRemoved: test.graph_region
97-
// CHECK-AN: notifyOperationRemoved: test.erase_op
94+
// CHECK-AN: notifyOperationErased: test.foo_b
95+
// CHECK-AN: notifyOperationErased: test.foo_a
96+
// CHECK-AN: notifyOperationErased: test.graph_region
97+
// CHECK-AN: notifyOperationErased: test.erase_op
9898
// CHECK-AN-LABEL: func @test_remove_graph_region()
9999
// CHECK-AN-NEXT: return
100100
func.func @test_remove_graph_region() {
@@ -109,13 +109,13 @@ func.func @test_remove_graph_region() {
109109

110110
// -----
111111

112-
// CHECK-AN: notifyOperationRemoved: cf.br
113-
// CHECK-AN: notifyOperationRemoved: test.bar
114-
// CHECK-AN: notifyOperationRemoved: cf.br
115-
// CHECK-AN: notifyOperationRemoved: test.foo
116-
// CHECK-AN: notifyOperationRemoved: cf.br
117-
// CHECK-AN: notifyOperationRemoved: test.dummy_op
118-
// CHECK-AN: notifyOperationRemoved: test.erase_op
112+
// CHECK-AN: notifyOperationErased: cf.br
113+
// CHECK-AN: notifyOperationErased: test.bar
114+
// CHECK-AN: notifyOperationErased: cf.br
115+
// CHECK-AN: notifyOperationErased: test.foo
116+
// CHECK-AN: notifyOperationErased: cf.br
117+
// CHECK-AN: notifyOperationErased: test.dummy_op
118+
// CHECK-AN: notifyOperationErased: test.erase_op
119119
// CHECK-AN-LABEL: func @test_remove_cyclic_blocks()
120120
// CHECK-AN-NEXT: return
121121
func.func @test_remove_cyclic_blocks() {
@@ -134,14 +134,14 @@ func.func @test_remove_cyclic_blocks() {
134134

135135
// -----
136136

137-
// CHECK-AN: notifyOperationRemoved: test.dummy_op
138-
// CHECK-AN: notifyOperationRemoved: test.bar
139-
// CHECK-AN: notifyOperationRemoved: test.qux
140-
// CHECK-AN: notifyOperationRemoved: test.qux_unreachable
141-
// CHECK-AN: notifyOperationRemoved: test.nested_dummy
142-
// CHECK-AN: notifyOperationRemoved: cf.br
143-
// CHECK-AN: notifyOperationRemoved: test.foo
144-
// CHECK-AN: notifyOperationRemoved: test.erase_op
137+
// CHECK-AN: notifyOperationErased: test.dummy_op
138+
// CHECK-AN: notifyOperationErased: test.bar
139+
// CHECK-AN: notifyOperationErased: test.qux
140+
// CHECK-AN: notifyOperationErased: test.qux_unreachable
141+
// CHECK-AN: notifyOperationErased: test.nested_dummy
142+
// CHECK-AN: notifyOperationErased: cf.br
143+
// CHECK-AN: notifyOperationErased: test.foo
144+
// CHECK-AN: notifyOperationErased: test.erase_op
145145
// CHECK-AN-LABEL: func @test_remove_dead_blocks()
146146
// CHECK-AN-NEXT: return
147147
func.func @test_remove_dead_blocks() {
@@ -169,20 +169,20 @@ func.func @test_remove_dead_blocks() {
169169
// test.nested_* must be deleted before test.foo.
170170
// test.bar must be deleted before test.foo.
171171

172-
// CHECK-AN: notifyOperationRemoved: cf.br
173-
// CHECK-AN: notifyOperationRemoved: test.bar
174-
// CHECK-AN: notifyOperationRemoved: cf.br
175-
// CHECK-AN: notifyOperationRemoved: test.nested_b
176-
// CHECK-AN: notifyOperationRemoved: test.nested_a
177-
// CHECK-AN: notifyOperationRemoved: test.nested_d
178-
// CHECK-AN: notifyOperationRemoved: cf.br
179-
// CHECK-AN: notifyOperationRemoved: test.nested_e
180-
// CHECK-AN: notifyOperationRemoved: cf.br
181-
// CHECK-AN: notifyOperationRemoved: test.nested_c
182-
// CHECK-AN: notifyOperationRemoved: test.foo
183-
// CHECK-AN: notifyOperationRemoved: cf.br
184-
// CHECK-AN: notifyOperationRemoved: test.dummy_op
185-
// CHECK-AN: notifyOperationRemoved: test.erase_op
172+
// CHECK-AN: notifyOperationErased: cf.br
173+
// CHECK-AN: notifyOperationErased: test.bar
174+
// CHECK-AN: notifyOperationErased: cf.br
175+
// CHECK-AN: notifyOperationErased: test.nested_b
176+
// CHECK-AN: notifyOperationErased: test.nested_a
177+
// CHECK-AN: notifyOperationErased: test.nested_d
178+
// CHECK-AN: notifyOperationErased: cf.br
179+
// CHECK-AN: notifyOperationErased: test.nested_e
180+
// CHECK-AN: notifyOperationErased: cf.br
181+
// CHECK-AN: notifyOperationErased: test.nested_c
182+
// CHECK-AN: notifyOperationErased: test.foo
183+
// CHECK-AN: notifyOperationErased: cf.br
184+
// CHECK-AN: notifyOperationErased: test.dummy_op
185+
// CHECK-AN: notifyOperationErased: test.erase_op
186186
// CHECK-AN-LABEL: func @test_remove_nested_ops()
187187
// CHECK-AN-NEXT: return
188188
func.func @test_remove_nested_ops() {
@@ -212,12 +212,12 @@ func.func @test_remove_nested_ops() {
212212

213213
// -----
214214

215-
// CHECK-AN: notifyOperationRemoved: test.qux
216-
// CHECK-AN: notifyOperationRemoved: cf.br
217-
// CHECK-AN: notifyOperationRemoved: test.foo
218-
// CHECK-AN: notifyOperationRemoved: cf.br
219-
// CHECK-AN: notifyOperationRemoved: test.bar
220-
// CHECK-AN: notifyOperationRemoved: cf.cond_br
215+
// CHECK-AN: notifyOperationErased: test.qux
216+
// CHECK-AN: notifyOperationErased: cf.br
217+
// CHECK-AN: notifyOperationErased: test.foo
218+
// CHECK-AN: notifyOperationErased: cf.br
219+
// CHECK-AN: notifyOperationErased: test.bar
220+
// CHECK-AN: notifyOperationErased: cf.cond_br
221221
// CHECK-AN-LABEL: func @test_remove_diamond(
222222
// CHECK-AN-NEXT: return
223223
func.func @test_remove_diamond(%c: i1) {
@@ -277,7 +277,7 @@ func.func @test_inline_block_before() {
277277
// CHECK-AN: notifyOperationInserted: test.op_2, was last in block
278278
// CHECK-AN: notifyOperationInserted: test.split_block_here, was last in block
279279
// CHECK-AN: notifyOperationInserted: test.new_op, was unlinked
280-
// CHECK-AN: notifyOperationRemoved: test.split_block_here
280+
// CHECK-AN: notifyOperationErased: test.split_block_here
281281
// CHECK-AN-LABEL: func @test_split_block(
282282
// CHECK-AN: "test.op_with_region"() ({
283283
// CHECK-AN: test.op_1

mlir/test/lib/Dialect/Test/TestPatterns.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,8 @@ struct DumpNotifications : public RewriterBase::Listener {
349349
}
350350
}
351351
}
352-
void notifyOperationRemoved(Operation *op) override {
353-
llvm::outs() << "notifyOperationRemoved: " << op->getName() << "\n";
352+
void notifyOperationErased(Operation *op) override {
353+
llvm::outs() << "notifyOperationErased: " << op->getName() << "\n";
354354
}
355355
};
356356

mlir/test/lib/Transforms/TestConstantFold.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ struct TestConstantFold : public PassWrapper<TestConstantFold, OperationPass<>>,
3131
OpBuilder::InsertPoint previous) override {
3232
existingConstants.push_back(op);
3333
}
34-
void notifyOperationRemoved(Operation *op) override {
34+
void notifyOperationErased(Operation *op) override {
3535
auto *it = llvm::find(existingConstants, op);
3636
if (it != existingConstants.end())
3737
existingConstants.erase(it);

0 commit comments

Comments
 (0)