[Mlir-commits] [mlir] [mlir][vector] Make `TransposeOpLowering` configurable (PR #73915)

[Andrzej Warzyński]

banach-space wrote:

I will just re-iterate:

> Supporting SPIR-V is critical, and nobody wants to break it of course.

Now, trying to figure out how to move this forward:

> (a) Either we add patterns to lower vector.shape_cast to SPIR-V and then the vector lowering patterns can introduce the vector.shape_cast without control, or
(b) We allow opt-out so that for SPIR-V backend we dont introduce these shape_casts. (like this patch does).

My expectation is that (eventually) the SPIR-V community will implement the missing lowerings for `vector.shape_cast`. So (a) above is "when" rather than "if". Is my assumption correct? 

If my assumption is not correct then either the Vector dialect needs to be updated (*) or the conclusion is that SPIR-V is not compatible with Vector.

To me, adding support for lowering `vector.shape_cast` sounds like the most frictionless path. This could be followed by updates to the Vector dialect that would allow more optimal code generation. But I don't really work with SPIR-V so can't judge. 

> What we have is basically a vector lowering that introduces shape_cast irrespective of the backend being used, without it being supported on all backends.

True, but the opposite would mean limiting what the LLVM IR backend can use due to limitations of the SPIR-V backend. I know that that's not what you are suggesting, but what if there was a 3rd backend with even more limitation than SPIR-V? Should that mean limiting both LLVM IR and SPIR-V backends even further?

> (...) this needs some control to opt-out

IMHO this is a fair request and I support it. We can "guard" this opt-out mechanism with comments pointing at this discussion and discouraging from re-using it beyond this case. However, there should also be commitment from SPIR-V to fill the gaps in the existing lowerings. Or some roadmap with alternatives. That would really help the discussion.

Just to clarify - this patch wasn't meant as the solution, but merely as a workaround.

-Andrzej

(*) That's being proposed here: https://discourse.llvm.org/t/improving-handling-of-unit-dimensions-in-the-vector-dialect/

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