[mlir][transforms][NFC] Expand CanonicalizerPass documentation

Mention that canonicalization is best-effort and that pass pipelines should not rely on it for correctness.

RFC: https://discourse.llvm.org/t/rfc-canonicalizerpass-convergence-error-handling/67333

Differential Revision: https://reviews.llvm.org/D140729

diff --git a/mlir/docs/Canonicalization.md b/mlir/docs/Canonicalization.md
index 1d3f053..d1aed54 100644 (file)
--- a/mlir/docs/Canonicalization.md
+++ b/mlir/docs/Canonicalization.md
@@ -19,15 +19,28 @@ to capture IR-specific rules for reference.
 
 ## General Design
 
-MLIR has a single canonicalization pass, which iteratively applies
-canonicalization transformations in a greedy way until the IR converges. These
-transformations are defined by the operations themselves, which allows each
-dialect to define its own set of operations and canonicalizations together.
+MLIR has a single canonicalization pass, which iteratively applies the
+canonicalization patterns of all loaded dialects in a greedy way.
+Canonicalization is best-effort and not guaranteed to bring the entire IR in a
+canonical form. It applies patterns until either fixpoint is reached or the
+maximum number of iterations/rewrites (as specified via pass options) is
+exhausted. This is for efficiency reasons and to ensure that faulty patterns
+cannot cause infinite looping.
+
+Canonicalization patterns are registered with the operations themselves, which
+allows each dialect to define its own set of operations and canonicalizations
+together.
 
 Some important things to think about w.r.t. canonicalization patterns:
 
+*   Pass pipelines should not rely on the canonicalizer pass for correctness.
+    They should work correctly with all instances of the canonicalization pass
+    removed.
+
 *   Repeated applications of patterns should converge. Unstable or cyclic
-    rewrites will cause infinite loops in the canonicalizer.
+    rewrites are considered a bug: they can make the canonicalizer pass less
+    predictable and less effective (i.e., some patterns may not be applied) and
+    prevent it from converging.
 
 *   It is generally better to canonicalize towards operations that have fewer
     uses of a value when the operands are duplicated, because some patterns only

diff --git a/mlir/include/mlir/Transforms/Passes.td b/mlir/include/mlir/Transforms/Passes.td
index e7d1223..fd6351d 100644 (file)
--- a/mlir/include/mlir/Transforms/Passes.td
+++ b/mlir/include/mlir/Transforms/Passes.td
@@ -20,7 +20,11 @@ def Canonicalizer : Pass<"canonicalize"> {
   let summary = "Canonicalize operations";
   let description = [{
     This pass performs various types of canonicalizations over a set of
-    operations. See [Operation Canonicalization](Canonicalization.md) for more
+    operations by iteratively applying the canonicalization patterns of all
+    loaded dialects until either a fixpoint is reached or the maximum number of
+    iterations/rewrites is exhausted. Canonicalization is best-effort and does
+    not guarantee that the entire IR is in a canonical form after running this
+    pass. See [Operation Canonicalization](Canonicalization.md) for more
     details.
   }];
   let constructor = "mlir::createCanonicalizerPass()";

diff --git a/mlir/lib/Transforms/Canonicalizer.cpp b/mlir/lib/Transforms/Canonicalizer.cpp
index dc3bf97..a6fa775 100644 (file)
--- a/mlir/lib/Transforms/Canonicalizer.cpp
+++ b/mlir/lib/Transforms/Canonicalizer.cpp
@@ -57,6 +57,7 @@ struct Canonicalizer : public impl::CanonicalizerBase<Canonicalizer> {
     config.enableRegionSimplification = enableRegionSimplification;
     config.maxIterations = maxIterations;
     config.maxNumRewrites = maxNumRewrites;
+    // Canonicalization is best-effort. Non-convergence is not a pass failure.
     (void)applyPatternsAndFoldGreedily(getOperation(), patterns, config);
   }