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

[Andrzej Warzyński]

banach-space wrote:

Thanks for the reply!

>  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.

IIUC, your interpretation of the docs is something like this (please correct me if I’m misinterpreting your comment):

* "When you have two options (canonicalization pattern vs. folder), always opt for a folder."

I don’t see any such indication in the docs. This is also why I requested that you remove "NFC" from the description - thanks for doing that!

> I'm happy to update the docs.

I think that would be really helpful! It would save us this discussion and also provide clear guidance for our future selves and other contributors.

I’m happy for this to be merged (thanks for working on it and for addressing my comments!), but I’d appreciate it if you could follow up with a docs update - assuming I correctly interpreted your interpretation. 😅

Thanks, Kunwar!

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