[Mlir-commits] [mlir] [MLIR][Vector] Fix transferOps optimization inside maskOp (PR #90835)

[Andrzej Warzyński]

================

----------------
banach-space wrote:

Thanks for checking!

>From what I can tell, the tests in "mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir" are "meta" tests" that:
*  focus on checking `transform.apply_patterns.vector.lower_transfer max_transfer_rank`, 
* in some cases have `transfer_permutation_patterns` added on top. 

That particular test (`transfer_write_broadcast_unit_dim`) doesn't check for `vector.load`/`vector.store`, so doesn't seem to belong in "vector-transfer-to-vector-load-store.mlir". IMHO, that file should be audited first 😅  In the meantime, let's focus on "vector-transfer-permutation-lowering.mlir".

Now, I think that this file could also benefit from some additional comments and small re-org. This way it will be easier to see what cases are being tested. ATM that's not really clear and I'm to blame 😅  Trying to fix here:
* https://github.com/llvm/llvm-project/pull/91943

Could you take a look?

> Do you think I should add the masked case with it or move them both to vector-transfer-permutation-lowering.mlir ?

I think that what you have here is sufficient. There are 3 possibilities:
1. non-masked,
2. `vector.xfer_read` with mask,
3. masked `vector.xfer_read|write` (i.e. with `vector.mask`)

Option 2 is already tested and that effectively covers 1. as well. So we are only missing 3., right? And that's what you are testing.

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