[Mlir-commits] [mlir] [mlir][vector] Allow lowering multi-dim scatters to LLVM (PR #132227)

[Kunwar Grover]

Groverkss wrote:

> Thanks for the addition! Could you add tests for the new pattern?
> 
> Basically, if we are reaching feature parity between `vector.gather` and `vector.scatter`, we should also make sure that the testing coverage is similar. I see two test files that exercise unrolling/lowering of `vector.gather`:
> 
> * https://github.com/llvm/llvm-project/blob/main/mlir/test/Dialect/Vector/vector-gather-lowering.mlir
> * https://github.com/llvm/llvm-project/blob/main/mlir/test/Dialect/Vector/vector-transfer-unroll.mlir

I thought about this, and my reasoning about this is:

- This patch only adds a single pattern, UnrollScatter, which is used by ConvertToLLVM to lower multi-dimensional scatters. We have the same tests for gather for gather in test/Conversions/VectorToLLVM/vector-to-llvm.mlir . Previous patches added a negative test to lower multi-dimensional scatters, which this patch makes positive (i.e. we can lower them now).

- test/Dialect/Vector/vector-gather-lowering.mlir tests are tests for unroll + linearize + lower to conditional loads for gather operations. Currently, the scatter op only implements unroll here, so adding tests here would not be correct.

- test/Dialect/Vector/vector-transfer-unroll contains tests for a seperate style of unrolling, where a single dimension is unrolled into smaller chunks. This pattern unrolls the outer dimension completly. So these tests are unrelated.

Hopefully this makes it clear why the test coverage in this patch is enough.

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