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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
