[llvm-branch-commits] [mlir] [MLIR][ArmSVE] Add initial lowering of vector.contract to SVE `*MMLA` instructions (PR #135636)

Wed, 23 Apr 2025 07:14:30 -0700 

https://github.com/banach-space commented:

Thanks Momchil - this is great!

I skimmed through the pattern logic, and it's very neatly written. It's 
actually quite easy to follow, despite the underlying logic being a bit 
convoluted - well done! I've left a few minor suggestions, but nothing major.

Also, it seems like we should be able to extend this fairly easily to support 
NEON as well. Worth thinking about 🙂

Now, overall this patch is quite large, and I’d suggest extracting the 
end-to-end / integration tests into a separate PR. Additionally, the remaining 
tests currently use `--convert-vector-to-llvm=`, which lowers all the way to 
LLVM (i.e., it exercises a lot of patterns). Instead, I’d recommend testing 
`LowerContractionToSVEI8MMPattern` in isolation and only verifying that the 
correct sequence of ArmSVE ops (plus some Vector ops) is generated - for 
example:

```mlir
    (...)
    %33 = arm_sve.smmla %23, %7, %15 : vector<[16]xi8> to vector<[4]xi32>
    %34 = arm_sve.smmla %24, %7, %16 : vector<[16]xi8> to vector<[4]xi32>
    %35 = arm_sve.smmla %31, %13, %15 : vector<[16]xi8> to vector<[4]xi32>
    %36 = arm_sve.smmla %32, %13, %16 : vector<[16]xi8> to vector<[4]xi32>
```

That way, we will:
* reduce noise in the test output (by focusing on a single pattern),
* simplify expected output (fewer ops to match),
* avoid re-testing functionality already covered elsewhere (e.g., 
`arm_sve.smmla` → `arm_sve.intr.smmla` lowering).

Btw, this is already looking great, and I know I’m asking for a bit of a 
rewrite (especially around the tests), but I really think it’ll help with 
long-term maintainability.

https://github.com/llvm/llvm-project/pull/135636
_______________________________________________
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to