[PATCH] D74797: [mlir] [VectorOps] Framework for progressive lowering of vector.contract

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/lib/Dialect/VectorOps/VectorTransforms.cpp:924
+      if (rhsContractingDimSet.count(rhsIndex) == 0) {
+        rewriter.replaceOp(op, lowerContract(op, -1, rhsIndex, rewriter));
+        return matchSuccess();
----------------
Can we add /*...=*/ parameter comments to these constant -1s?


================
Comment at: mlir/lib/Dialect/VectorOps/VectorTransforms.cpp:967
+    } else {
+      assert(rhsIndex >= 0);
+      iterIndex =
----------------
Can you add messages to these asserts?


================
Comment at: mlir/lib/Dialect/VectorOps/VectorTransforms.cpp:988
+        assert(it.value().cast<StringAttr>().getValue() ==
+               mlir::getParallelIteratorTypeName());
+        continue;
----------------
Drop the mlir:: here.


================
Comment at: mlir/lib/Dialect/VectorOps/VectorTransforms.cpp:1032
+    }
+    return -1;
+  }
----------------
nit: Use Optional<int64_t> instead.


================
Comment at: mlir/lib/Dialect/VectorOps/VectorTransforms.cpp:1065
+    for (int64_t i = 0; i < rank; ++i) {
+      if (i == index) // omit index
+        continue;
----------------
nit: Don't use trailing comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74797