[Mlir-commits] [mlir] [mlir][vector] Add vector.transpose with unit-dim to vector.shape_cast pattern (PR #72105)

[Lei Zhang]

antiagainst wrote:

Oh, lots of great comments happening here touching various parts.. I think it's beneificial to separate out a bit so we don't talk past one another:

1. Whether we can have a pattern to convert some flavor of `vector.transpose` to some flavor of `vector.shape_cast`: totally fine to me! I have no complains. It's a tool in the toolbox. (sorry if it becomes confusing that we are posting comments on a pull request that does this--we should actually move this into an issue proper or something..)
2. Whether we should make that pattern apply to _all_ conversion paths by bumping it into a canonical pattern that nobody can opt out unless opt out all canoncalizations. I agree with Mahesh here. I think this one needs more discussion, as not all code path can support the same feature set, and/or at the same development phase to support all even fundamentally they can. Aside, it's also touching:
3. Which is a more canonical form: `vector.transpose` or `vector.shape_cast`. I don't see we have consensus here. That's actually an indication maybe we also need to take a pause before making one or the other canonical. Note that the canonicalization is not only run before converting to LLVM or SPIR-V. It can trigger anytime. So it means I may have other furhter conversion steps following it unrelated to LLVM or SPIR-V. If I'm seeing a `vector.shape_cast`, I don't know it's meant to be a general shape cast that can do whatever, or just a transpose/no-op style shape cast that I can just go ahead without much analysis. So I'm still forced to do all the analysis to figure out it's a tranpose or no-op actually, as Mahesh said, effectively "undo" the canonicalzation, and as Quinn said, it's harder to rediscover certain information of `vector.shape_cast` comparing to just `vector.transpose`. (IMHO, I always think lightweight "syntax"-ish conversions are better as canonicalization patterns. But different people certainly have different understanding.)
4. It also seems that the semantics of `vector.shape_cast` is changing along the way. I think it's also great to take a closer look what it's meant to do and harden its boundary a bit before making it canonical form of other ops. 

I think 2-4 may need extra time--for us to think through the details / add missing pieces / fix implementation / etc. So I'd propose expose adding a `populate*Patterns` API entry to the pattern so Diego and others can have their workflows unblocked. I hope that's an okay intermediate step that buys us time (hey, it's Thanksgiving and we should be enjoying turkeys ;-P) and we can discuss more on the later points.  

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