[PATCH] D80393: [mlir][Vector] Add more vector.contract -> outerproduct lowerings and fix vector.contract type inference.

[Alex Zinenko via Phabricator via llvm-commits]

ftynse accepted this revision.
ftynse added inline comments.
This revision is now accepted and ready to land.


================
Comment at: mlir/lib/Dialect/Vector/VectorOps.cpp:275
+    SmallVector<AffineExpr, 4> extents(lhsMap.getNumInputs());
+    extents.reserve(lhsMap.getNumInputs());
+    for (auto pair :
----------------
No need to reserve after constructing with a size


================
Comment at: mlir/lib/Dialect/Vector/VectorOps.cpp:286
     }
+    assert(llvm::all_of(extents, [](AffineExpr e) { return e; }));
+
----------------
Nit: assert(... && "textual explanation") would be helpful here


================
Comment at: mlir/lib/Dialect/Vector/VectorOps.cpp:293
+    AffineMap expectedMap = simplifyAffineMap(resMap.compose(extentsMap));
+    assert(llvm::all_of(expectedMap.getResults(), [](AffineExpr e) {
+      return e.isa<AffineConstantExpr>();
----------------
Nit: same as above


================
Comment at: mlir/lib/Dialect/Vector/VectorTransforms.cpp:1525
+  bindDims(op.getContext(), m, n, k);
+  SmallVector<int64_t, 2> perm{1, 0};
+  SmallVector<AffineMap, 4> maps = op.getIndexingMaps();
----------------
This looks unused.


================
Comment at: mlir/lib/Dialect/Vector/VectorTransforms.cpp:1558
+  if (maps == infer({{m, k}, {k, n}, {m, n}})) {
+    // This is the classical row-major matmul. Just permute the rhs.
+    reductionSize = lhsType.getShape()[1];
----------------
The code permutes the Lhs, while the comment claims its Rhs.


================
Comment at: mlir/lib/Dialect/Vector/VectorTransforms.cpp:1576
+  else if (maps == infer({{m, k}, {k, n}, {n, m}})) {
+    // This is the classical row-major matmul. Just permute the rhs.
+    reductionSize = lhsType.getShape()[1];
----------------
Same as above


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80393/new/

https://reviews.llvm.org/D80393