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

[Igor Wodiany]

IgWod-IMG wrote:

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

I am not a SPIR-V/Vulkan expert either, but I agree with your assessment. I guess it's a bit tricky since Image does not really have a layout, just coordinates.

> (although maybe this isn't a safe assumption if the layout isn't row-major 🤷 )

I think this is an important point, we only need to reverse indices for row-major `memref`s, and column-major should just work as it is. And I guess any other layout should not be supported - unless conversion can transform indices to match the logical Image coordinates.

So, my recommendation would be:

  1. Only do the reversal for row-major `memref`s.
  2. Update the description with what we discussed so the reason behind the decision is clear; also, I would use `i/u`, `j/v`, `k/w`, instead of `x`, `y`, `z`.
  3. minor: Update tests to use non-square/cube images; having all dimension the same in tests does not help the mental model.

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