[Mlir-commits] [mlir] [mlir][ArmNeon] Add pack/mmt4d path to f32 linalg.matmul integration test (PR #225063)

[Federico Bruzzone]

FedericoBruzzone wrote:

Thanks a lot for guiding me through this PR! 🙏

> OK, I forgot that the number of variants is starting to grow. I propose the following:
> 
> * `pack-unpack-mmt4d-f32.mlir` should implement _only one_ matmul type (static) and lower that through different means (_without_ packing vs _with_ packing). The focus should be on the MMT4D path. Plain `linalg.matmul` is lowered via `-test-lower-to-llvm`. Element type - `f32`.
> * `pack-unpack-mmt4d-i8.mlir` should implement _only one_ matmul type (static) and lower that through different means (_without_ packing vs _with_ packing). The focus should be on the MMT4D path. Plain `linalg.matmul` is lowered via `-test-lower-to-llvm` Element type - `i8`.
> * `matmul-f32.mlir` - dynamic matmul, only f32. Lowered via tiling + vectorization.
> * `matmul.mlir` -  should be split into e.g. `matmul-f32.mlir` and `matmul-i8.mlir`.
> 
> Importantly, could you add a note in `pack-unpack-mmt4d-f32.mlir` that `linalg.matmul` is lowered via `-test-lower-to-llvm` and only added to provide reference check?

Done. Split into `pack-unpack-mmt4d-f32.mlir` (static) and `matmul-f32.mlir` (dynamic, tiling+vectorization). @matmul in `pack-unpack-mmt4d-f32.mlir` is now lowered only via `-test-lower-to-llvm`, no schedule, with a comment noting it's a reference check.


> I am slowly realising where the confusion is coming from - you are referring to `fmmla` rather than `fmla`. Why would you refer to an SVE instruction in a patch that's about Neon? 😅 Also, we don't have support for `fmmla` ATM, that's effectively a TODO.
> 
> I would drop the reference to `fmmla` - it confused me, so there's a chance it will confuse others as well.

Ups! Sorry for the noise. Dropped the `fmmla`/`bfmmla` mention entirely. The comment/PR description now just says NEON has no instruction that multiplies two f32 matrices in one go. And it's interesting that `fmmla` isn't currently supported... I want to know more about that :'D

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