[Mlir-commits] [mlir] Add operands to worklist when only used by deleted op (PR #86990)

[Mehdi Amini]

================
@@ -688,14 +688,19 @@ void GreedyPatternRewriteDriver::notifyOperationModified(Operation *op) {
   addToWorklist(op);
 }
 
-void GreedyPatternRewriteDriver::addOperandsToWorklist(ValueRange operands) {
-  for (Value operand : operands) {
-    // If the use count of this operand is now < 2, we re-add the defining
-    // operation to the worklist.
-    // TODO: This is based on the fact that zero use operations
-    // may be deleted, and that single use values often have more
-    // canonicalization opportunities.
-    if (!operand || (!operand.use_empty() && !operand.hasOneUse()))
+void GreedyPatternRewriteDriver::addOperandsToWorklist(Operation *op) {
+  for (Value operand : op->getOperands()) {
+    // If this operand was only used by the op under consideration, we re-add
+    // the operation that defined it to the worklist. Indeed, if the op is about
+    // to be deleted and it was the sole user of the operand, the operand may
+    // also be deleted.
+    // TODO: if the operand has a single use besides the op under consideration,
+    // there may be further canonicalization opportunities, so it should be
+    // added to the worklist.
----------------
joker-eph wrote:

This TODO does not seem hard to fix, can we do it now?

Should be something like this:
```
   Operation *otherUser = op;
  if (!llvm::all_of(operand.getUsers(),
                       [&op](Operation *user) { 
     if (user == op) return true;
     if (user != otherUser)  return false;
     otherUser = user;
     return true;
  }))
  continue;
```

(avoid single letter variables for readability)

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