[Mlir-commits] [mlir] [mlir][vector] Clarify the semantics of gather/scatter indexing (nfc) (PR #181357)

[Andrzej Warzyński]

banach-space wrote:

> So, this PR implements the semantics I was pushing for initially with its "going off the end of the last dimension proceeds as if in the identity layout" wording.

Just to clarify: this PR does not implement new semantics - these are NFC documentation changes.

(see also my comment below re: trimming this patch)

> That definition doesn't match the lowering implemented in https://github.com/llvm/llvm-project/pull/184706 , which attempts to account for the multi-dimensionality of the memref

That PR addresses the lowering of `vector.gather to `vector.load`. During this lowering (`vector.gather` → `vector.load`), the semantics previously captured by vector.gather need to be re-expressed, which is where the linearization/delinearization comes in.

In particular, this accounts for:
* the multi-dimensionality of the memref, and
* the loss of the index argument (which only exists on `vector.gather`).

Effectively, the (`offset`, `index`) pair is replaced by a single `offset`. So while the mechanism differs, I believe the semantics remain consistent.

That said, I’m happy to take a step back and trim the part of this PR where we don’t yet have agreement - see this commit: https://github.com/llvm/llvm-project/pull/181357/changes/9513d44b9fc1f921485fec10b8fb84aa82d78378. Thanks for pointing that out.

Just to make sure I understand your concern correctly: are you disagreeing with the direction taken in https://github.com/llvm/llvm-project/pull/184706, or with the "logical vs physical" index distinction from the RFC discussion? Are you OK with the current wording in this PR?

> has semantics that are meaningfully different from lowering directly to something like LLVM's gather.

Good catch - that should indeed be fixed.

--- 

Given the confusion and the length of the review (~1 month), I’d like to bring in more eyes.

@rengolin - since you participated in the Discourse discussion, would you mind taking a look?

(As per Jakub’s suggestion, I’ve avoided using “physical” vs “logical” terminology in the docs.)

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