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

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 14 12:58:20 PDT 2020


rjmccall added inline comments.


================
Comment at: clang/docs/MatrixTypes.rst:79
+  floating point type, convert the integer or floating point operand to the
+  underlying element type of the operand of matrix type.
+
----------------
fhahn wrote:
> rjmccall wrote:
> > You should standardize on one term and then be clear what you mean by it.  Here you're saying "integer or floating point type", but elsewhere you use "arithmetic type".  Unfortunately, the standard terms mean somewhat different things in different standards: "integer" includes enums in C but not in C++, "arithmetic" doesn't include complex types in C++ (although it does by extension in Clang), etc.  I think for operands you probably want arithmetic types in the real domain (which in Clang is `isRealType()`).  However, you'll want to use a narrower term for the restriction on element types because Clang does support fixed-point types, but you probably don't want to support matrices of them quite yet (and you may not want to allow matrices of bools, either).
> > 
> > Also, your description of the scalar conversions no longer promotes them to matrix type.
> > You should standardize on one term and then be clear what you mean by it. Here you're saying "integer or floating point type", but elsewhere you use "arithmetic type". Unfortunately, the standard terms mean somewhat different things in different standards: "integer" includes enums in C but not in C++, "arithmetic" doesn't include complex types in C++ (although it does by extension in Clang), etc. I think for operands you probably want arithmetic types in the real domain (which in Clang is isRealType()). However, you'll want to use a narrower term for the restriction on element types because Clang does support fixed-point types, but you probably don't want to support matrices of them quite yet (and you may not want to allow matrices of bools, either).
> 
> I've added the following to the Matrix Type section:
> ` 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.`
> 
> Other places are updated to use `a valid matrix element type` instead.
> 
> I think we explicitly want to allow half-precision types (like __fp16 and Float16 in Clang). I think by referring to real type as in the C99 spec, we naturally exclude Clang's fixed-point types and bool, right? 
> 
> > Also, your description of the scalar conversions no longer promotes them to matrix type.
> 
> Right, I think we can just refer to the standard conversion rules here, as in
> 
> `If one operand is of matrix type and the other operand is of a valid matrix element type, convert the non-matrix type operand to the matrix type according to the standard conversion rules.`
> I think we explicitly want to allow half-precision types (like __fp16 and Float16 in Clang). I think by referring to real type as in the C99 spec, we naturally exclude Clang's fixed-point types and bool, right?

C says:

> The integer and real floating types are collectively called *real types*.

> The type `char`, the signed and unsigned integer types, and the enumerated types are collectively called *integer types*.

> The standard and extended unsigned integer types are collectively called *unsigned integer types*.

> The type `_Bool` and the unsigned integer types that correspond to the standard signed integer types are the *standard unsigned integer types*.

Embedded C (TR 18037) says:

> Clause 6.2.5 - Types, paragraph 17: change last sentence as follows.
>
> Integer, fixed-point and real floating types are collectively called *real types*.

So you'll have to explicitly exclude enumerated types, `_Bool`, and the fixed-point types.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76612/new/

https://reviews.llvm.org/D76612





More information about the cfe-commits mailing list