[Mlir-commits] [mlir] [mlir] Implement indexed access op interfaces for memref, vector, gpu, nvgpu (PR #177014)

[Krzysztof Drewniak]

================
@@ -2103,6 +2111,14 @@ def Vector_GatherOp :
     memory at an address aligned to this boundary. Violating this requirement
     triggers immediate undefined behavior.
 
+    Note that if the base argument is not contiguous in memory (for example,
+    it is the result of a `tensor.extract` or a `memref.subview`), the entries
+    of `index_vec` must respect the strieds in the underlying memory and are
+    applied as pure 1-D offsets and are not decomposed in order to apply
+    the strides on the base.
----------------
krzysz00 wrote:

I do claim that the lowering is intentional - the vector of gather indices (as opposed to the offsets into the memref) can be k-dimensional because it's being used to construct a k-dimensional result. If we expected the indices to be interpreted as offsets into the memref following the memref layout, we'd require R indices for each element being gathered / R vectors of indices, where R is the rank of the memref. If we did not have this requirement, we'd need to do a bunch of splitting of the index (according to size? stride? it's ambiguous) to work out how to interpret it as a memref index and apply the layout.

That is, if I'm gathering from a `memref<?x?, strided<[?, ?]>` , I'd need to somehow break each element of `%v` into some `%v1` and `%v2` so I could apply those two unknown strides. Because the gather operation does not contemplate, or give any indication of an intent to contemplate, such a transformation, I do not expect those to be the intended semantics.

Furthermore, the sort of higher-level behavior you're interested in is covered by the proposed transfer_gather (which didn't make it upstream but is in IREE), see https://discourse.llvm.org/t/rfc-improving-gather-codegen-for-vector-dialect/85011

I'll also note that vector.gather *does* use getStridedElementPointer() for its lowering - to get the base pointer of the gather operation.

That is, my point is that `vector.gather` doesn't have the dimensions to do what you think it does. I think an RFC to change this behavior could happen, but I see no reason not to document the current semantics and land this.

(I'm going to point at https://github.com/llvm/llvm-project/blob/a6c926b0cf6bde1f46146e66cf17b72b62597818/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp#L907 as well, where linalg code is well-aware that the gather index needs to be a linear offset, 

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