[Mlir-commits] [mlir] [mlir][Vector] Move vector.extract canonicalizers for DenseElementsAttr to folders (PR #127995)

[Kunwar Grover]

Groverkss wrote:

> The code changes LGTM, thanks!
> 
> That said, I don’t quite follow this from the summary:
> 
> > Folders are local, and it's always better to implement a folder than a canonicalizer.
> 
> Why? This is a rather strong statement, and I don’t see any clear indication of this in our documentation: [MLIR Canonicalization Docs](https://mlir.llvm.org/docs/Canonicalization/)
> 
> I have no objections to landing this, but let’s ensure we have a valid reason for this preference. Otherwise, it feels somewhat arbitrary.

I don't think it's a very strong statement, but a natural interpretation of the docs. I'm happy to update the docs, but let me explain my interpretation.

>From the docs on canonicalization:

```
The fold mechanism is an intentionally limited, but powerful mechanism that allows for applying canonicalizations in many places throughout the compiler. For example, outside of the canonicalizer pass, fold is used within the [dialect conversion infrastructure](https://mlir.llvm.org/docs/DialectConversion/) as a legalization mechanism, and can be invoked directly anywhere with an OpBuilder via OpBuilder::createOrFold.

fold has the restriction that no new operations may be created, and only the 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.
```

Based on the docs, folders are a restricted way of implementing canonicalizations, where the transformation is "local" (as defined in the docs). The other way is to implement a RewritePattern, which can change the IR in any way a pattern can as long as it converges. Folders have an additional property, which can be used by rewrite drivers (dialect conversion, greedy rewrite driver) to do canonicalizations on the fly, because folding cannot create new operations (so no extra operations to run the rewrite driver on, adding only a constant cost for the rewrite driver, instead of the rewrite driver having to run on the newly created ops).

But, in the end, both are canonicalizations (as defined by the docs). A folder or a RewritePattern is just an implementation detail. If a transformation should be a canonicalization (this is a harder question that needs proof) is a different question than if it should be a folder or a rewrite pattern (this is an easier question, it's an implementation detail). The choice between implementing a canonicalization as a folder or a rewrite pattern comes down to if the canonicalization is local or not. The canonicalization in this PR is local, which is why it's better to implement it as a folder.

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