[Mlir-commits] [mlir] [mlir][Vector] Add support for masks in castAwayContractionLeadingOneDim (PR #81906)

[Andrzej Warzyński]

================
@@ -346,6 +349,12 @@ mlir::vector::castAwayContractionLeadingOneDim(vector::ContractionOp contractOp,
   // greedily to drop more.
   int64_t dropDim = 1;
 
+  if (isMasked) {
----------------
banach-space wrote:

Sorry, returning to this after two weeks of being OOO.

@tanmaysachan , from a quick scan, you have adopted [VectorMaskOpConversionBase](https://github.com/llvm/llvm-project/blob/982e9022ca4abaad58c693d2b0aba0e908ee2d39/mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp#L849-L870) which matches `vector.mask`. IIUC, that's going to be insufficient in this case - we need something that would work both for:
*  `vector.mask {vector.contract}`, and 
* `vector.contract`.

`VectorMaskOpConversionBase` would only work for the first case. Perhaps @dcaballe had something else in mind, but IMHO we need another "base" class to accomodate for that. This a blocker for me, so I'm proposing one here (*):
* https://github.com/llvm/llvm-project/pull/83827

> I think we should perhaps create a utility for those and define some kind of canonical form to do this at pattern rewrite level.

Does #83827 make sense? Happy to try something else :) Also, regardless of the long-term approach that we take here, would you be OK with me landing this to unblock `linalg.mmt4d` investigation? 

(*) Apologies @tanmaysachan if you are also actively working on this, but this is quite urgent for me. Mindful of delays due to time difference, I decided to go ahead and draft something quickly.

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