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

[Jack Frankland]

FranklandJack wrote:

> I am a bit confused, but maybe a picture will help. Let's look at the Figure 1 in [here](https://docs.vulkan.org/spec/latest/chapters/textures.html). Is what you are trying to say that in `memref` the first dimension would be height, and the second dimension would be width, but in Vulkan (as in the figure) the first dimension is width, whereas the second dimension is height? And since in both cases you want to access data row by row, we need to reverse indices? So basically, without this patch the conversion silently changes iteration from row-major to column-major?
> 
> Am I understanding it right?

Yes, I think so.

The definition of `OpImageFetch` has the following wording for the coordinates argument:
```
Coordinate must be a scalar or vector of [integer type](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#Integer). It contains (u[, v] …​ [, array layer]) as needed by the definition of Sampled Image.
```

I'm not entirely sure what `u[, v] …​ [, array layer]`  refers to here but the Vulkan spec has for normalized coordinates:
```
    u: Coordinate in the first dimension of an image.
    v: Coordinate in the second dimension of an image.
    w: Coordinate in the third dimension of an image.
    a: Coordinate for array layer.
```

For a memref the innermost dimension is the first dimension (although maybe this isn't a safe assumption if the layout isn't row-major 🤷 ), so when you get the indices via a call to `getIndices()` it would seem you need to reverse them to get the coordinate ordering required by the SPIR-V op?

I may have misunderstood the SPIR-V/Vulkan specs here, so apologies if this isn't the case.

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