[Mlir-commits] [mlir] [mlir][memref-to-spirv]: Reverse Image Load Coordinates (PR #160495)

[Georgios Pinitas]

GeorgeARM wrote:

Hello, not a SPIR-V expert but here are my 2cents.

In general the SPIR-V image coordinate system is well-defined and independent of the memory layout itself. So the indexing reversal does make sense to reconcile the `memref` indexing order with the `spir-v` one from a logical perspective.

Note though there is caveat on the return type on texel fetch as this will always  be 4 elements and depending on the format we might need to extract one or expect the `memref` to be encoded in such a way to capture this.
For formats like R32F this is straight-forward (we always extract the element we want in the conversion) but for the likes of RGBA8 which are multi-channel then we probably need to expect `memref` with elements of `vector<4xi8>`.

Finally, you are right the image layout is opaque in SPIR-V, and for good reason, but it shouldn't be conflated with `memref` layouts.  We shouldn't try to retrofit the memref layout onto images; this could be quite tricky; hence to keep thinks safe and clean we can initially require that memref are in the image address space (as you probably already do) and expect identity layout. It then up to the client/user to ensure that image is created and populated correctly though the API.

Think this is a great start to get use of basic image support mainly in compute and approach seems quite reasonable.

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