rjmccall added a comment. Reading through the rest of the spec.
================ Comment at: clang/docs/LanguageExtensions.rst:500 +Clang provides a matrix extension, which is currently being implemented. See +:ref:`matrixsupport` for more details. + ---------------- rjmccall wrote: > This should include just a bit more detail about the extension. I would > suggest: > > > Clang supports matrix types as an experimental extension. See > > :`ref`matrices` for more details. If we're calling the extension "matrix types", that should be reflected in this section name and in the file name. ================ Comment at: clang/docs/MatrixSupport.rst:28 +includes storage for ``rows * columns`` values of the *element type*. The +internal layout, overall size and alignment are implementation-defined. + ---------------- No need to italicize "element type" the second time. The italics introduce a term, so consider italicizing "rows" and "columns" as well in the first sentence. ================ Comment at: clang/docs/MatrixSupport.rst:39 + +Future Work: Initialization syntax. + ---------------- Maybe break the TODOs here into their own sections, which would come much later. ================ Comment at: clang/docs/MatrixSupport.rst:70 +between the original and resulting type, the program is ill-formed. Otherwise +the resulting value is as follows: + ---------------- I don't think you need to list out the kinds of promotion and conversion here, and it doesn't make sense to define the "resulting type" this way when it's really a parameter. I'd just say: > A value of matrix type can be converted to another matrix type if the number > of rows and columns are the size and the value's elements can be converted to > the element type of the result type. The result is a matrix where each > element is the converted corresponding element. > A value of non-matrix type can be converted to a matrix type if it can be > converted to the element type of the matrix. The result is a matrix where > all elements are the converted original value. ================ Comment at: clang/docs/MatrixSupport.rst:126 +The operands of ``+``, ``-`` and ``*`` shall have either matrix type, arithmetic or +unscoped enumeration type. At least one of the operands shall be of matrix type. + ---------------- I don't think this paragraph adds anything, and the restriction is kindof weird — it's just a restriction on when to consider applying these rules, rather than a restriction with absolute significance. Also, "arithmetic type" includes unscoped enumeration types in both C and C++. ================ Comment at: clang/docs/MatrixSupport.rst:129 +For ``BIN_OP`` in ``+``, ``-``, ``*`` given the expression ``M1 BIN_OP M2`` where, for +``*``, one of M1 or M2 is of arithmetic type: + ---------------- Here I think you can say "where at least one of M1 or M2 is of matrix type and, for `*`, the other is of arithmetic type". I think you'll need to separately describe the restrictions on `+=`, `-=`, and `*=`, but you should be able to say that the semantics are as if for the expansion. ================ Comment at: clang/docs/MatrixSupport.rst:176 + +Matrix Type builtin Operations +------------------------------ ---------------- "builtin" should be capitalized here. ================ Comment at: clang/docs/MatrixSupport.rst:211 + +``M __builtin_matrix_column_load(T *ptr, int row, int col, int stride)`` + ---------------- This name sounds like it's loading a column, when I think you're saying that the memory has to be in column-major order. I would call `stride` something like `columnStride` to make it clear that it's the stride between columns, as opposed to a stride between the elements within a column, which is also something that's theoretically interesting. Should `stride` be an optional argument to make it easier to write the (I expect) common case where the matrix is dense? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76612/new/ https://reviews.llvm.org/D76612 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
