[Mlir-commits] [mlir] [MLIR][Vector] Add DropUnitDimFromBroadcastOp pattern (PR #92938)

[Andrzej Warzyński]

banach-space wrote:

Hi Hugo, thanks for sending this!

> This MR is part of a list of MRs aiming to generalize DropUnitDimFromElementwiseOps for other ops.

I think that it would be good to take a step back and discuss whether this is actually needed. IIUC, you are trying to solve the problem outlined here:
* https://discourse.llvm.org/t/on-improving-arm-sme-lowering-resilience-in-mlir/78543?u=nujaa#canonicalization-patterns-1

As I pointed out in my reply in that thread:
>> Solutions: Should we match broadcast(+transpose)+mulf+addf to generate a contractionOp, or is it the developer’s responsibility to use cse / canonicalize with care?
>
> IMHO, no. 
* https://discourse.llvm.org/t/on-improving-arm-sme-lowering-resilience-in-mlir/78543/2

Also, as @dcaballe hinted:
> As mentioned multiple times in the past, the vector.multi_reduction -> vector.contract step should be removed in favor of direct linalg.matmul -> vector.contract direct lowering.
* https://discourse.llvm.org/t/on-improving-arm-sme-lowering-resilience-in-mlir/78543/15

Before moving ahead with this - **what's your long-term goal?** If we manage to get rid of `vector.multi_reduction` from
* the `linalg.matmul` -> `vector.outer_product` lowering path,

then, IIUC, this change won't be needed. Given that getting rid of `vector.multi_reduction` is the long-term design goal (so it's bound to happen at some point), I'm wondering whether we shouldn't re-focus on that instead?

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