[Mlir-commits] [mlir] [MLIR] Make 1-D memref flattening a prerequisite for vector narrow type emulation (PR #157771)

[Andrzej Warzyński]

================
@@ -2228,6 +2246,9 @@ void vector::populateVectorNarrowTypeEmulationPatterns(
     const arith::NarrowTypeEmulationConverter &typeConverter,
     RewritePatternSet &patterns, bool disableAtomicRMW) {
 
+  // As a prerequisite, make sure memrefs in vector ops are linearized.
+  memref::populateFlattenVectorMemrefPatterns(patterns);
+
----------------
banach-space wrote:

I see what you are trying to achieve here, but I would prefer if `populateVectorNarrowTypeEmulationPatterns` only contained patterns from this file (for better separation of concerns).

If a "meta" `populate-*` method like this is desirable, then I suggest the following:
```cpp
void vector::populateMemRefFlattenAndVectorNarrowTypeEmulationPatterns(RewritePatternSet &patterns) {
  vector::populateVectorNarrowTypeEmulationPatterns(patterns);
   memref::populateFlattenVectorMemrefPatterns(patterns);
}
```

For a meta-op like that, I suggest https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/Vector/TransformOps/VectorTransformOps.td and the related logic. Also, as already suggested by Mahesh, we will need tests :) That should cover all relevant Ops. However, given that `populateVectorNarrowTypeEmulationPatterns` and `populateFlattenVectorMemrefPatterns` are tested separately, we can keep the newly added tests quite minimal (e.g. `CHECK-NOT: vector.load i4` and `CHECK: vector.load i8`).

Hopefully this makes sense!

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