[Mlir-commits] [mlir] [MLIR][Test] Notify rewriter on in-place attrs in clone test patterns (PR #192215)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Hocky Yudhiono (hockyy)

<details>
<summary>Changes</summary>

Test patterns `CloneOp` and `CloneRegionBeforeOp` set the `was_cloned` unit attribute with a direct `setAttr` on the operation. That mutates the IR without going through `RewriterBase::finalizeOpModification`, so `notifyOperationModified` is never sent to listeners.

When `MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS` is enabled, the greedy pattern driver’s fingerprint bookkeeping still holds stale entries for those ops and aborts with `LLVM ERROR: operation finger print changed` after a successful rewrite. This does not change IR semantics of the patterns; it aligns them with the rewriter notification contract expected by expensive checks and other listeners.

**Testing**

`mlir-opt` on IR using `test.clone_region_before` with `-test-strict-pattern-driver=strictness=AnyOp` (no fingerprint fatal error with expensive checks on).

Assisted-by: Cursor (Composer 2)

---
Full diff: https://github.com/llvm/llvm-project/pull/192215.diff


1 Files Affected:

- (modified) mlir/test/lib/Dialect/Test/TestPatterns.cpp (+6-2) 


``````````diff
diff --git a/mlir/test/lib/Dialect/Test/TestPatterns.cpp b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
index c8be4bf3f0f8d..249ed7a42da0a 100644
--- a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
+++ b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
@@ -365,7 +365,9 @@ struct CloneOp : public RewritePattern {
     if (op->hasAttr("was_cloned"))
       return failure();
     Operation *cloned = rewriter.clone(*op);
-    cloned->setAttr("was_cloned", rewriter.getUnitAttr());
+    rewriter.modifyOpInPlace(cloned, [&]() {
+      cloned->setAttr("was_cloned", rewriter.getUnitAttr());
+    });
     return success();
   }
 };
@@ -383,7 +385,9 @@ struct CloneRegionBeforeOp : public RewritePattern {
       return failure();
     for (Region &r : op->getRegions())
       rewriter.cloneRegionBefore(r, op->getBlock());
-    op->setAttr("was_cloned", rewriter.getUnitAttr());
+    rewriter.modifyOpInPlace(op, [&]() {
+      op->setAttr("was_cloned", rewriter.getUnitAttr());
+    });
     return success();
   }
 };

``````````

</details>


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