fhahn marked 3 inline comments as done.
fhahn added inline comments.

================
Comment at: clang/docs/MatrixTypes.rst:12
+fixed-size matrices as language values and perform arithmetic on them.
+
+This feature is currently experimental, and both its design and its
----------------
SjoerdMeijer wrote:
> Would it be good to set expectations here or in the section below: define 
> that we're talking about 2-dimensional m × n matrices?
I've changed it to `fixed-size 2-dimensional matrices`. I think the type 
definition below should be already clear enough about being 2 dimensional.


================
Comment at: clang/docs/MatrixTypes.rst:27
+internal layout, overall size and alignment are implementation-defined.
+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
----------------
SjoerdMeijer wrote:
> above you're using *element type* and here *matrix element type*. Since 
> hopefully we're talking about the same things, "matrix *element type*" would 
> be more consistent.
> 
> But this is just a nit, my main question is about the types:
> why not e.g. define this to be the C11 types, that include _FloatN types, so 
> that we can include N=16? Or is this intentionally omitted? I haven't even 
> checked if this is supported in the architecture extension, but might make 
> sense? And also, an element type cannot be an integer type? 
> 
>  
> above you're using *element type* and here *matrix element type*. Since 
> hopefully we're talking about the same things, "matrix *element type*" would 
> be more consistent.

Yes it is referring to the same thing. I had a look at most uses, and in most 
cases `element type` is used to refer to the element type of a given matrix 
type. In that context it seems a bit verbose to use `matrix element type`, 
although I am more than happy to change that if it helps with clarifying things.

I intentionally used `matrix element type` in `Arithmetic Conversions`, because 
there it is standing on its own and refers exactly to the set of types defined 
as valid matrix element types here.

> why not e.g. define this to be the C11 types, that include _FloatN types, so 
> that we can include N=16? Or is this intentionally omitted? I haven't even 
> checked if this is supported in the architecture extension, but might make 
> sense?

I couldn't find any reference to _FloatN types in the C11 draft version I 
checked. Do you by any chance have a reference to the _FloatN types?

> And also, an element type cannot be an integer type?

The current definition should include it (real types include integer and real 
floating point types according to  C99 6.2.5p17). I don't think there is any 
reason to exclude them I think.


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