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

llvmlistbot at llvm.org llvmlistbot at llvm.org
Thu Mar 28 16:16:14 PDT 2024


================
@@ -688,14 +688,22 @@ 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 currently has at most 2 users, add its defining op to the
+    // worklist. Indeed, after the op is deleted, then the operand will have at
+    // most 1 user left. If it has 0 users left, it can be deleted too,
+    // and if it has 1 user left, there may be further canonicalization
+    // opportunities.
+    if (!operand)
+      continue;
+    Operation *otherUser = nullptr;
+    if (!llvm::all_of(operand.getUsers(), [&](Operation *user) {
+      if (user == op) return true;
+      if (otherUser && user != otherUser) return false;
----------------
mlevesquedion wrote:

Hmm I must be missing something :thinking: 

You wrote:

```
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;
```

Because `otherUser` is initialized to `op`, if the check `user == op` fails, we know that `user != otherUser` will succeed, because `otherUser == op` so this is the same as `user != op`, and this code is only executed if `user != op`. So I think my tweak is actually necessary? Otherwise we will never execute `otherUser = user; return true;`.

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


More information about the Mlir-commits mailing list