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

[Ingo Müller]

https://github.com/ingomueller-net updated https://github.com/llvm/llvm-project/pull/218351

>From 4d9b318409177a11ce13357faec80cf268ea9163 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ingo=20M=C3=BCller?= <ingomueller at google.com>
Date: Mon, 24 Aug 2026 10:29:36 +0200
Subject: [PATCH 1/2] [mlir:doc] Clarify requirements on `RewritePatterns`.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.

Assisted-by: Antigravity / Gemini 3.7 Flash
Signed-off-by: Ingo Müller <ingomueller at google.com>
---
 mlir/docs/Canonicalization.md  |  3 ++-
 mlir/docs/DialectConversion.md |  6 ++++--
 mlir/docs/PatternRewriter.md   | 10 +++++++++-
 3 files changed, 15 insertions(+), 4 deletions(-)

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

>From 063eedb1f80c8b81567e779495aa6e96587b0fef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ingo=20M=C3=BCller?= <ingomueller at google.com>
Date: Mon, 24 Aug 2026 13:50:26 +0200
Subject: [PATCH 2/2] Demote verifiability to best practice.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Ingo Müller <ingomueller at google.com>
---
 mlir/docs/Canonicalization.md |  4 ++--
 mlir/docs/PatternRewriter.md  | 16 ++++++++++++----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/mlir/docs/Canonicalization.md b/mlir/docs/Canonicalization.md
index c43dc4a2a3eb4..255d5cc3bffe2 100644
--- a/mlir/docs/Canonicalization.md
+++ b/mlir/docs/Canonicalization.md
@@ -186,8 +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. Like [rewrite patterns](PatternRewriter.md#restrictions), folding
-must always preserve IR verifiability.
+rewriter. As with [rewrite patterns](PatternRewriter.md#restrictions), folding
+should ideally 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/PatternRewriter.md b/mlir/docs/PatternRewriter.md
index c5bc919871653..39605c7acb7fe 100644
--- a/mlir/docs/PatternRewriter.md
+++ b/mlir/docs/PatternRewriter.md
@@ -73,8 +73,6 @@ 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
@@ -88,8 +86,18 @@ public:
     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).
+Additionally, there are some best practices that patterns are advised to follow:
+
+*   Patterns *should* transform verifiable IR into verifiable IR, i.e., the IR
+    should remain verifiable after every pattern application. However, there are
+    cases where rewrites are best split into several patterns and ensuring
+    verifiability would be cumbersome, such as changing a function declaration
+    and its call sites. In such cases it may be acceptable to temporarily have
+    unverifiable IR.
+
+**Note:** These restrictions and best practices can be checked at runtime by
+building with `-DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON` (ideally paired
+with ASan).
 
 
 ### Application Recursion