[Mlir-commits] [llvm] [mlir] [mlir][vector] Fold `in_bounds` for transfers with loop-derived indices (PR #215340)

[Dhairyashil R G]

dhairyashilRG wrote:

I have force-pushed a restructured version. Since the code moved out of `VectorOps.cpp` entirely, the inline comments above no longer have lines to point at, so here is what changed and where it went.

@matthias-springer, @banach-space and @dcaballe all asked for this to be an opt-in pass rather than a folder, so that is what it now is:

- the value-bounds logic is now a new pass, `-vector-infer-in-bounds`, in `mlir/lib/Dialect/Vector/Transforms/VectorInferInBounds.cpp`;
- `isInBounds` in `VectorOps.cpp` is **untouched**. The diff is purely additive, 519 lines added and none removed;
- the tests moved to a new `mlir/test/Dialect/Vector/vector-infer-in-bounds.mlir` and now run against the pass alone, with no `-canonicalize` in the RUN line. The `mlir/test/Dialect/Linalg/hoisting.mlir` churn is gone with them, since it only existed because that file canonicalizes;
- `MLIRValueBoundsOpInterface` is added to `MLIRVectorTransforms` in both CMake and the Bazel overlay. The overlay has `layering_check` on, so the direct dependency is required there rather than optional.

One correction to the original description: it offered to gate the value-bounds query behind a flag. That is not implementable. `TransferReadOp::fold(FoldAdaptor)` carries no options and no pass context,
so there is nowhere to read a flag from. I have withdrawn the offer rather than leave it standing.

Two things I would still like a view on:

1. `inferInBounds` in the new file repeats the permutation-map and broadcast-dimension walk from `foldTransferInBoundsAttribute`. Sharing it would mean exporting a helper from the dialect library to the transforms library. I left them separate on the grounds that the folder is deliberately static-only while the pass does analysis, but I am happy to unify them if you would rather.
2. The pass currently declines on dynamic source dimensions. Value-bounds analysis could compare the index against a `memref.dim` instead, which would cover them. That felt like scope creep for this PR; say the word if it belongs here.

Separately, I have opened a small independent fix for the folder's constant path: it marks a negative constant index in-bounds, which contradicts the `in_bounds` definition in `VectorOps.td` ("including the starting point"). That one does not depend on this PR and this PR does not depend on it.

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