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

[springerm]

Summary

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

[mehdi_amini]

mlir/docs/Canonicalization.md

28

38–39

What is sentence bringing to this documentation? I'm wondering "what's the alternative anyway?"

39–40

I'm not sure about this advice right now, it seems to say too much or not enough to be more helpful than confusing.

42–45

43–44

I don't get why "fewer patterns may be applied"? And why is this the right parenthesis to go with "can cause inefficiencies"?

[mehdi_amini]

Thanks for improving the doc!

[springerm]

address comments

mlir/docs/Canonicalization.md

43–44

I meant "less effective" instead of "inefficient".

We say above: It applies patterns until either fixpoint is reached or the maximum number of iterations/rewrites is exhausted. This could give the wrong impression that as long as these limits are in place, there is no problem with unstable/cyclic rewrites.

What I wanted to say with this sentence:

Situation 1: No unstable/cyclic rewrites => many different patterns are applied, maybe even fixpoint is reached

Situation 2: There are unstable/cyclic rewrites => we may spend lots of time rewriting the same piece of IR over and over without making progress => IR that was canonicalized in Situation 1 is no longer canonicalized (some patterns may not be applied) because the max_iterations/max_num_rewrites limit is reached.

Situation 2 is more desirable.

mlir/docs/Canonicalization.md

 reuse it across many different IRs that it represents. This document describes
 the general approach, global canonicalizations performed, and provides sections
 to capture IR-specific rules for reference.
 
 [TOC]
 
 ## 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 recursion.
+
+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. Separate passes should be used for lowering IR from one level of
+    abstraction to another one. Such passes can reuse existing canonicalization
+    patterns. (Ideally only the ones that are actually needed.)
+
 *   Repeated applications of patterns should converge. Unstable or cyclic
-    rewrites will cause infinite loops in the canonicalizer.
+    rewrites can cause inefficiencies in the canonicalizer pass (i.e., fewer
+    patterns may be applied) and in the worst case infinite loops if no maximum
+    number of iterations/rewrites is specified via pass options.
 
 *   It is generally better to canonicalize towards operations that have fewer
     uses of a value when the operands are duplicated, because some patterns only
     match when a value has a single user. For example, it is generally good to
     canonicalize "x + x" into "x * 2", because this reduces the number of uses
     of x by one.
 
 *   It is always good to eliminate operations entirely when possible, e.g. by

mlir/include/mlir/Transforms/Passes.td

 
 include "mlir/Pass/PassBase.td"
 include "mlir/Rewrite/PassUtil.td"
 
 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()";
   let options = [
     Option<"topDownProcessingEnabled", "top-down", "bool",
            /*default=*/"true",
            "Seed the worklist in general top-down order">,
     Option<"enableRegionSimplification", "region-simplify", "bool",

mlir/lib/Transforms/Canonicalizer.cpp

LogicalResult initialize(MLIRContext *context) override {
     return success();
   }
   void runOnOperation() override {
     GreedyRewriteConfig config;
     config.useTopDownTraversal = topDownProcessingEnabled;
     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);
   }
 
   FrozenRewritePatternSet patterns;
 };
 } // namespace
 
 /// Create a Canonicalizer pass.