rjmccall added inline comments.

================
Comment at: clang/docs/LanguageExtensions.rst:511
+    *r = *a +  (*b * *c);
+  }
+
----------------
This is kindof an unnecessarily unreadable example.  I know you haven't decided 
on calling convention treatment yet, but maybe the leading example could be 
just a little ahead of the implementation and just take the matrices as 
arguments and then return the result.


================
Comment at: clang/docs/MatrixTypes.rst:29
+A *matrix element type* must be a real type (as in C99 6.2.5p17) excluding
+enumeration types or an implementation-defined half-precision floating point
+type, otherwise the program is ill-formed.
----------------
fhahn wrote:
> rjmccall wrote:
> > fhahn wrote:
> > > SjoerdMeijer wrote:
> > > > Now I am wondering if this requires some explanations on binary 
> > > > operations for these implemenation-defined types? For example, for 
> > > > `__fp16` and matrices with this `__fp16` element type, I assume 
> > > > arithmetic is performed in at least the (single) floating-point 
> > > > precision. So I guess in section "Arithmetic Conversions" a rule needs 
> > > > to be added that the conversion of these implementation defined types 
> > > > need to performed?
> > > > 
> > > I don't think we need to specifically discuss the implementation defined 
> > > types here, as the conversions and binary operator definitions are framed 
> > > in terms of the existing rules for the element types used. I am 
> > > potentially missing something, but with the current wording the 
> > > conversions for `__fp16` would use the conversion rules for that type and 
> > > the binary operators would use the arithmetic rules for it.
> > Yeah, for the scalar conversions / scalar operands, you should just say 
> > that the source has to be a real type and not otherwise restrict it.  All 
> > of those types should already be convertible to any matrix element type.
> Thanks, I've updated the wording to ensure the scalar values are of a real 
> type in the scalar -> matrix conversion and scalar, matrix binary operator 
> contexts. I hope that is enough to clarify things.
This would be clearer as something like:

> Currently, the element type of a matrix is only permitted to be one of the 
> following types:
>
> - an integer type (as in C2x 6.2.5p19), but excluding enumerated types and 
> `_Bool`
> - a standard floating type (as in C2x 6.2.5p10)
> - a half-precision floating point type, if one is supported on the target
>
> Other types may be supported in the future.

Although I don't know if you actually want to unconditionally support `long 
double`; you might just want to say "the standard floating types `float` and 
`double`".


================
Comment at: clang/docs/MatrixTypes.rst:62
+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
----------------
s/size/same/


================
Comment at: clang/docs/MatrixTypes.rst:66
+
+A value of a real 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
----------------
"A value of any real type (as in C2x 6.2.5p17) can be converted..."


================
Comment at: clang/docs/MatrixTypes.rst:167
+operations on matrix types match the behavior of the elementwise operations
+in the corresponding expansions provided above.
+
----------------
The expansions have a lot of statement boundaries that contraction wouldn't be 
allowed across.  I'd suggest saying something like:

> Operations on floating-point matrices have the same rounding and 
> floating-point environment behavior as ordinary floating-point operations in 
> the expression's context.  For the purposes of floating-point contraction, 
> all calculations done as part of a matrix operation are considered 
> intermediate operations, and their results need not be rounded to the format 
> of the element type until the final result in the containing expression.  
> This is subject to the normal restrictions on contraction, such as `#pragma 
> STDC FP_CONTRACT`.


================
Comment at: clang/docs/MatrixTypes.rst:216
+to ``row`` and ``col`` respectively. The parameter ``columnStride`` is optional
+and if ommitted ``row`` is used as ``columnStride``.
+
----------------
"omitted".

I would expect these operands to have type either `size_t` or `ptrdiff_t`.  Of 
course it only really matters for `columnStride`.


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

Reply via email to