[Mlir-commits] [mlir] [mlir][linalg] Move vectorization tests for pad + insert_slice Ops (nfc) (PR #140877)

[Andrzej Warzyński]

banach-space wrote:

Hey @hanhanW - you've raised a lot of great points, thanks!

I do have a target end-state in mind that I’m trying to converge toward, and I should’ve communicated that more clearly - sorry about that. Here's a quick summary of the overall direction:

* https://github.com/llvm/llvm-project/issues/141025

This PR is just a small step in that direction. It’s intended to make one of my upcoming patches a bit easier to review (at least that’s the hope!).

> I think your rule may be moving all vectorization tests of non-linalg ops to vectorization/ directory, but it is not obvious IMO

Yes - at a minimum, I’d like to extract non-Linalg ops from "vectorization.mlir" and "vectorization-with-patterns.mlir". We already have a dedicated file for tensor.extract, so doing the same for other non-Linalg ops would bring more consistency.

(Btw, let me rename+move "vectorize-tensor-extract-masked.mlir" + "vectorize-tensor-extract.mlir" as part of this change - to avoid sending too many PRs).

> I don’t have issues finding tests

That's because you know this stuff too well! 😄

But from a different angle - when updating vectorization logic for something like `linalg.pack` and `linalg.unpack` (which use custom hooks), wouldn’t it be cleaner if we had dedicated test files for just those two ops? That way, the corresponding PR would only touch "vectorize-pack-unpack.mlir", and if something breaks inside `vectorizeAsTensorPackOp` or `vectorizeAsTensorUnPackOp`, triaging would be much simpler.

These changes aren’t critical, but they do address some real friction I’ve experienced while updating or reviewing code. It’s mostly about future-proofing and making long-term maintenance easier.

Hope this makes sense!

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