[PATCH] D75775: [mlir][Vector] Add a vector.matrix_multiply op on 1-D vectors

[Aart Bik via Phabricator via llvm-commits]

aartbik added inline comments.


================
Comment at: mlir/include/mlir/Conversion/VectorToLLVM/ConvertVectorToLLVM.h:20
+/// Intrinsics. This needs to go through memory atm.
+void populateVectorToLLVMMatrixConversionPatterns(
+    LLVMTypeConverter &converter, OwningRewritePatternList &patterns);
----------------
why is this in its own population (instead of adding it to the general VectorToLLVM)?
I don't object, but just curious. I can see isolated testing, but you don't seem to exploit that


================
Comment at: mlir/include/mlir/Dialect/VectorOps/VectorOps.td:1330
+
+/// Vector dialect matrix multiplication op that operates on flattened 1-D
+/// MLIR vectors. This is the counterpart of llvm.matrix.multiply in MLIR.
----------------
Some parts of this comments could be moved into description (e.g. for some others, we also say why we picked it, like being close to the llvm implementation)


================
Comment at: mlir/include/mlir/Dialect/VectorOps/VectorOps.td:1349
+                      [AnySignlessInteger, AnySignedInteger, AnyFloat]>:$res)>
+{
+ let builders = [
----------------
can we get a summary and description please? we need to keep our docs a bit up to date ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75775