[Mlir-commits] [mlir] [mlir][vector] Verify non-unit strides on `masked/expand/compress` ops and fix `SparseVectorization` miscompilation (PR #210952)

[Federico Bruzzone]

FedericoBruzzone wrote:

> > Note: This is intentionally more permissive than vector.load/vector.store, the sparsifier legitimately produces masked loads on memrefs whose last-dim stride is contiguous by construction but not provable at the type level (see below).
> 
> This a bit confusing, TBH. If dynamic stride is considered as "unit" in some scenarios, then it should be considered as "unit" everywhere. Special-casing like this can be quite problematic. For now, I would at least:
> 
> * Add tests for `vector.load` + `vector.store` with dynamic strides (so that the difference in behaviour is documented through tests)
> * Update your table from this [comment](https://github.com/llvm/llvm-project/pull/210952#issuecomment-5033367754) accordingly (it's very useful, thank you!).
> 
> Also, your summary is quite long and verbose for a relatively small change like this. All in all, the discussion is relevant, but please remember that the summary will end-up as a commit message. A commit message should provide a _brief_ overview of _what_ and _why_. Here are some additional tips https://chris.beams.io/git-commit (we [reccomend](https://mlir.llvm.org/getting_started/Contributing/) that all contributors to MLIR follow it source in MLIR).
> 
> I suggest that you open a GitHub issue for the wider discussion and to track all the relevant PRs. The summary for this change should be much shorter.`
> 
> Last, but not least, thank you so much for all the fixes, that is greatly appreciated 🙏🏻
> 
> **EDIT** Btw, this feels like two independent changes. Could you split it?

Thanks @banach-space! 
I sincerely appreciate these suggestions and the time you are dedicating to guiding me (and the project in general) 🫶.

So,
- Split done: SparseVectorization fix is now standalone in #211004 (this PR is stacked on it).
- Added `load_dynamic_stride`/`store_dynamic_stride` to `invalid.mlir` documenting that `load/store` reject `[?]`, unlike the `masked/expand/compress` ops. (I hope this is what you meant. If not, let me know and I'll make the changes.)

Thanks for the thorough review!

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