[Mlir-commits] [mlir] [mlir][vector]advance support extract insert under dynamic case. (PR #121631)

[Oleksandr Alex Zinenko]

https://github.com/ftynse requested changes to this pull request.

>  It's reasonable for the lowering to expect a canonical form of the operation

Not necessarily. Canonicalization is expensive and it may not be desirable to run it after every single lowering pass. Plus, canonicalization patterns should not be mixed with conversion patterns. It's a trade-off and I'd rather have some simple cheap additional logic, then require to run the canonicalizer in the middle of lowerings.

> This seems to just add support for a non canonical form where the constant positions arent folded in.

This looks through `unrealized_conversion_cast`. This is beyond canonicalization. IMO, canonicalization should not be able to see through unrealized casts precisely because it doesn't know how they are going to be realized.

That being said, just running the canonicalizer after `affine-unroll` here doesn't fold the constant indices into `extract`. I'd start by implementing a folder for `vector.extract` that can fold in constants as static positions, and seeing if that is enough.

I left some comments, but these are mostly for general education. I don't expect this approach to land.

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