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

[Andrzej Warzyński]

banach-space wrote:

Hey @MaheshRavishankar , sorry that this is causing issues :(

>  this is not a "downstream" issue. It is really something that cannot be handled in SPIR-V and as such should not be on the default path

Is there any way for us to check what's suitable for SPIR-V and what isn't? Otherwise we might be hitting similar issue with every single canonicalisation that we try (hypothetically).  Ideally, there would be a test upstream that would start failing after moving this to the canonicalization pass.

Any suggestion what to use instead? As in, how to disable certain patterns for a particular target? From what I can see, this canonicalisation is actually legal/correct and it would be good to have a mechanism to avoid such situations in the future (i.e. canonicalisation that benefit the LLVM lowering path, but are invalid for SPIR-V, or vice verse).

> @c-rhodes at any rate we should probably move this out of the canonicalization pass.

That will solve the problem for you and should be sufficient for us, however ... (from Diego's comment)

> Could we make this a transpose canonicalization pattern instead? Getting redundant transposes around before its lowering is not helpful. We are actually hitting an issue related to 1xn to nx1 transposes and would need that type of canonicalization.

In ideal world, I'd wait for Diego to chime and suggest what other alternative could solve his particular issue. But, AFAIK, he's OOO this week. So, how about:
* we revert the patch that moved this parttern to the canonicalizaton pass (https://github.com/llvm/llvm-project/pull/72918),
* keep this particular change in tree.

As for whether this should be a canonicalization or not, I feel that we need more input.

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