fhahn added inline comments.
================ Comment at: clang/docs/MatrixSupport.rst:3 +Matrix Support +================== + ---------------- rjmccall wrote: > This extension should be called something like "Matrices" or "Matrix Types". > The "X Support" name makes it sound like it's a support layer for some > external technology. I changed it to "Matrix Types" ================ Comment at: clang/docs/MatrixSupport.rst:30 +directly. An *attribute-argument-clause* must be present and it shall have the +form: + ---------------- rjmccall wrote: > You can assume the existence of a hypothetical external language > specification for GNU attribute syntax, so these starting paragraphs whittle > down to: > > > Matrix types can be declared by adding the `matrix_type` attribute to the > > declaration of a `typedef` (or a C++ alias declaration). The underlying > > type of the `typedef` must be an unqualified integer or floating-point > > type. The attribute takes two arguments, both of which must be integer > > constant expressions that evaluate to a value greater than zero. The first > > specifies the number of rows, and the second specifies the number of > > columns. The underlying type of the `typedef` becomes a matrix type with > > the given dimensions and an element type of the former underlying type. > > The paragraph about redeclarations is good. Thanks, that helps to simplify this section a lot. ================ Comment at: clang/docs/MatrixSupport.rst:48 +Matrix Type +----------- + ---------------- rjmccall wrote: > I would put this first before getting into the spelling. You can also put > the stuff about implementation limits on dimensions in here. Sounds good! Moved. ================ Comment at: clang/docs/MatrixSupport.rst:57 + +A matrix type is a *scalar type* and its alignment is implementation defined. + ---------------- rjmccall wrote: > These are both important to include, but they're unrelated and shouldn't be > in the same sentence. I moved the `scalar type` bit to the first sentence and move the part about the alignment to a separate sentence stating that the layout overall size and alignment are implementation defined. ================ Comment at: clang/docs/MatrixSupport.rst:64 + +TODO: Specify how matrix values are passed to functions. + ---------------- rjmccall wrote: > That doesn't belong in a language specification, but you could reasonably add > a non-normative section at the end about the decisions that Clang currently > makes for things like size, alignment, internal layout, and argument/result > conventions. I've added a new `Decisions for the Implementation in Clang` section ================ Comment at: clang/docs/MatrixSupport.rst:110 +the matrix. The expression E1 is sequenced before E2 and E3. The expressions +E2 and E3 are unsequenced. + ---------------- rjmccall wrote: > I'd put all this like: > > > An expression of the form ``E1 [E2] [E3]``, where ``E1`` has matrix type > > ``cv M``, is a matrix element access expression. Let ``T`` be the element > > type of ``M``, and let ``R`` and ``C`` be the number of rows and columns in > > ``M`` respectively. The index expressions shall have integral or unscoped > > enumeration type and shall not be uses of the comma operator unless > > parenthesized. The first index expression shall evaluate to a non-negative > > value less than ``R``, and the second index expression shall evaluate to a > > non-negative value less than ``C``, or else the expression has undefined > > behavior. If ``E1`` is a prvalue, the result is a prvalue with type ``T`` > > and is the value of the element at the given row and column in the matrix. > > Otherwise, the result is a glvalue with type ``cv T`` and with the same > > value category as ``E1`` which refers to the element at the given row and > > column in the matrix. Updated, thanks. ================ Comment at: clang/docs/MatrixSupport.rst:118 +builtins to extract rows and columns from a matrix. This makes the operations +more explicit. + ---------------- rjmccall wrote: > You should add a normative paragraph saying that a program is ill-formed if > it insufficiently subscripts into a matrix. I added the following > Programs containing a single subscript expression into a matrix are > ill-formed. 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
