[Mlir-commits] [mlir] [mlir:doc] Clarify requirements on `RewritePatterns`. (PR #218351)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Ingo Müller (ingomueller-net)

<details>
<summary>Changes</summary>

This PR clarifies and extends the documentation of what `RewritePattern`s and folders are allowed to do. The most noteworthy addition is the requirement that they must produce verifiable IR. While this is enforced by `MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS`, it has not been mentioned in the markdown docs yet. The other changes are clarifications on edge cases that I have seen people misunderstand or overlook. The change also adds a note to the build flag that enables the API checks.

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


3 Files Affected:

- (modified) mlir/docs/Canonicalization.md (+2-1) 
- (modified) mlir/docs/DialectConversion.md (+4-2) 
- (modified) mlir/docs/PatternRewriter.md (+9-1) 


``````````diff
diff --git a/mlir/docs/Canonicalization.md b/mlir/docs/Canonicalization.md
index 6fbcf09c51df3..c43dc4a2a3eb4 100644
--- a/mlir/docs/Canonicalization.md
+++ b/mlir/docs/Canonicalization.md
@@ -186,7 +186,8 @@ root operation may be replaced (but not erased). It allows for updating an
 operation in-place, or returning a set of pre-existing values (or attributes) to
 replace the operation with. This ensures that the `fold` method is a truly
 "local" transformation, and can be invoked without the need for a pattern
-rewriter.
+rewriter. Like [rewrite patterns](PatternRewriter.md#restrictions), folding
+must always preserve IR verifiability.
 
 In [ODS](DefiningDialects/Operations.md), an operation can set the `hasFolder` bit to generate
 a declaration for the `fold` method. This method takes on a different form,
diff --git a/mlir/docs/DialectConversion.md b/mlir/docs/DialectConversion.md
index b73aa04398103..7323f473802bf 100644
--- a/mlir/docs/DialectConversion.md
+++ b/mlir/docs/DialectConversion.md
@@ -559,8 +559,10 @@ to the entry block of the region. The types of the entry block arguments are
 often tied semantically to the operation, e.g., `func::FuncOp`, `AffineForOp`,
 etc.
 
-To convert the signature of just one given block, the
-`applySignatureConversion` hook can be used.
+To convert the signature of just one given block, the `applySignatureConversion`
+hook can be used. Note that `applySignatureConversion` replaces and erases the
+original block, so each block's signature can be converted at most once per
+conversion run.
 
 A signature conversion, `TypeConverter::SignatureConversion`, can be built
 programmatically:
diff --git a/mlir/docs/PatternRewriter.md b/mlir/docs/PatternRewriter.md
index 105a554b95851..c5bc919871653 100644
--- a/mlir/docs/PatternRewriter.md
+++ b/mlir/docs/PatternRewriter.md
@@ -73,15 +73,23 @@ public:
 
 #### Restrictions
 
+*   Patterns must transform verifiable IR into verifiable IR, i.e., the IR must
+    be verifiable after every pattern application.
 *   All IR mutations, including creation, *must* be performed by the given
     `PatternRewriter`. This class provides hooks for performing all of the
     possible mutations that may take place within a pattern. For example, this
     means that an operation should not be erased via its `erase` method. To
     erase an operation, the appropriate `PatternRewriter` hook (in this case
-    `eraseOp`) should be used instead.
+    `eraseOp`) should be used instead. Note that changes to nested ops, regions,
+    and blocks need to go through the rewriter as well.
 *   The root operation is required to either be: updated in-place, replaced, or
     erased.
 *   `matchAndRewrite` must return "success" if and only if the IR was modified.
+    In particular, this means that the pattern is not allowed to have made any
+    modification if it returns "failure".
+
+**Note:** These restrictions can be checked at runtime by building with
+`-DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON` (ideally paired with ASan).
 
 
 ### Application Recursion

``````````

</details>


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