[Mlir-commits] [mlir] [mlir][docs] Add docs on canonicalizers being folders or patterns (PR #129517)

[Kunwar Grover]

Groverkss wrote:

> Hey @Groverkss , thank you so much for sending this!
> 
> I've left a couple of comments inline, but I also have one high-level comment.
> 
> There are already two fairly dense paragraphs on the two approaches to implementing "canonicalizations":
> 
> * [Canonicalizing with `RewritePattern`s
>   ](https://github.com/llvm/llvm-project/blob/1aea0241f1cce9eb4eba3e4add3be9370e30e415/mlir/docs/Canonicalization.md?plain=1#L113)
> * [Canonicalizing with the `fold` method](https://github.com/llvm/llvm-project/blob/1aea0241f1cce9eb4eba3e4add3be9370e30e415/mlir/docs/Canonicalization.md?plain=1#L156)
> 
> I think that you can safely trim your addition to the following paragraph (I tweaked the title so that the naming aligns with what's currently used in this document):
> 
> ```
> ### When to use the `fold` method vs `RewriterPattern`s for canonicalizations
> 
> A canonicalization should always be implemented as a Folder if it fits the "local" definition of a folder, otherwise it should be implemented as a `RewritePattern`.
> ```
> 
> From what I can tell, this is the main _new bit of info_ added in this PR . The other items seem to repeat what's already here. For example:
> 
> > If a transformation is a canonicalization, there are two ways to implement
> > it: As a Folder or as a `RewritePattern`, both of which are implementation
> 
> That's already covered by the paragraphs that I linked :) (i.e. that's why we have them)
> 
> Also, this part doesn't really get us closer to understanding what to prefer - the "fold" method vs `RewriterPattern`:
> 
> > If a transformation should be a canonicalizer is a different question from
> > if a transformation should be a folder or a `RewritePattern`. A
> > transformation is promoted to a canonicalization as defined by the General
> > Design of canonicalizations.
> 
> One other point, could there be some other heuristics to consider here? For example, the cost and the frequency with which different "implementations" would be run.

Removed repeated lines and changed to a one liner.

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