[PATCH] D76612: [Matrix] Add draft specification for matrix support in Clang.

[Florian Hahn via Phabricator via cfe-commits]

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