[Mlir-commits] [mlir] [mlir][transform] Do not maintain mappings for dead handles (PR #73558)

[Matthias Springer]

matthias-springer wrote:

> I suppose the desired behavior is the tracking rewriter not failing to update the mapping. 

The tracking rewriter already has similar code: if a replacement op could not be found, no error is produced if the handle is dead.


> Moving the print before the application "fixes" it.

That behavior is confusing indeed. With that in mind, I think we should not drop dead handles (i.e., closing this PR) and even remove the aliveness check in the tracking rewriter that I mentioned above. It is very confusing when commenting out a seemingly unrelated line of code changes the behavior.

In many cases, when there is a problem with replacement operations, the problem could be solved by:
- modifying the IR without notifying the rewriter
- sending the "op replaced notification" manually

Maybe we just need better API around that.

E.g., consider a hypothetical transform that drops an OpResult from a mapped `scf.for` loop and replaces it with a constant:

```c++
auto replacementForOp = rewriter.clone(forOp);
SmallVector<Value> replacements;
replacements.push_back(rewriter.create<ConstantIndexOp>(loc, 42));
llvm::append_range(replacement, ArrayRef(replacementForOp.getResults()).drop_front());
// Will fail to find replacement
rewriter.replaceOp(forOp, replacements);
```

This would work, maybe we can find a better API around that:
```c++
auto replacementForOp = rewriter.clone(forOp);
SmallVector<Value> replacements;
replacements.push_back(rewriter.create<ConstantIndexOp>(loc, 42));
llvm::append_range(replacement, ArrayRef(replacementForOp.getResults()).drop_front());
// Will fail to find replacement
Listener *l = rewriter.getListener();
rewriter.setListener(nullptr);
l->notifyOpReplaced(forOp, replacementForOp);
rewriter.replaceOp(forOp, replacements);
rewriter.setListener(l);
```

In the mean time, we could also have a `transform.consume` op to explicitly consume handles. With this op, we could remove the liveness check from the tracking listener today (making "replacement op not found" errors less surprising), while providing a way to work around the issue without modifying the transformation C++ code (and until we have a better API around the code snippet above).


https://github.com/llvm/llvm-project/pull/73558