[PATCH] D75852: [mlir][LLVM] Add remaining llvm.matrix intrinsics

Aart Bik via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 9 09:41:20 PDT 2020


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


================
Comment at: mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td:794
 
+/// As specified in the LLVM MatrixBuilder:
+/// Create a columnwise, strided matrix load.
----------------
this is very minor, but I slightly prefer stating what the class does (Create a columnwise....) and then state "as specific in the LLMV Matrix builder'


================
Comment at: mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td:795
+/// As specified in the LLVM MatrixBuilder:
+/// Create a columnwise, strided matrix load.
+/// data    - Start address of the matrix read
----------------
since we heavily overload 'vector's and 'tensors', perhaps make the 2-D matrix clear in the comment
(although later implied by rows/columns of course)


================
Comment at: mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td:814
+/// As specified in the LLVM MatrixBuilder:
+/// matrix  - Matrix to store
+/// ptr     - Pointer to write back to
----------------
misses:

Creates a columnwise, strided matrix store.

to be consistent with all others.


================
Comment at: mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td:850
+/// As specified in the LLVM MatrixBuilder:
+/// Create a llvm.matrix.transpose call, transposing `matrix` with `rows`
+/// rows and `columns` columns.
----------------
how about repeating 'rows' and row, as

... call, transposing a 'rows' x 'columns' 2-D 'matrix'




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75852





More information about the llvm-commits mailing list