[Mlir-commits] [mlir] [mlir][vector] Add opt-in `inbounds`/`nneg` flags to `vector.load`/`vector.store` (PR #202118)

[Krzysztof Drewniak]

krzysz00 wrote:

1. I'm quite opposed to encoding properties of your program in *IR pass options*. If you're going to rely on things being in-bounds, that should be marked in your code somewhere, not a flag on the lowering.
2. Re in-bounds being useful MLIR-level, I'm going to point at, for example https://github.com/llvm/llvm-project/blob/9c6a5f064611b12fd38f7282c0a3b600eb30950d/mlir/lib/Dialect/Vector/Transforms/IndexedAccessOpInterfaceImpl.cpp#L179 being a method I've added *specifically* to capture whether or not a vector.load is starting in-bounds
3. Re "inbounds isn't used", I point to things like https://github.com/llvm/llvm-project/blob/9c6a5f064611b12fd38f7282c0a3b600eb30950d/mlir/lib/Dialect/Vector/Transforms/IndexedAccessOpInterfaceImpl.cpp#L179

> [amdgpu example] TBH, it's not clear to me how that is related to `ConvertVectorToLLVM

Because this sort of thing gets used on the way to LLVM! Consider https://github.com/llvm/llvm-project/blob/main/mlir/lib/Dialect/AMDGPU/Transforms/MaskedloadToLoad.cpp , which is a valid transformation that *could* (depending on how a notional LLVM RFC plays out) get broken by `inbounds` | `nuw`, and is definitely broken by asserting that vector indices are in-bounds at the MLIR level without some sort of per-op control!

Side note re negative strides ... I think they *are* sound, so long as you require that the index as a whole is non-negative (and thus, when you compute the final offset from the memref base, you're `nuw`)

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