[libcxx] Only define __libcpp_is_floating_point<_Float16> for Clang

[phosek]

Summary

GCC doesn't define _Float16 for C++ since this type is only defined
to be a C11 extension, and it's unclear if other compiler do define
this type for C++, so restrict __libcpp_is_floating_point<_Float16>
type trait only to Clang.

[ldionne]

Why does Clang define it for C++?

The problem I have with this is that it means is_floating_point<_Float16> is true on Clang but false on GCC (in case GCC starts defining _Float16 for C++).

[phosek]

I don't know why we chosen that approach, but Arm Compiler Reference Guide explicitly mentions that _Float16 is being available in C and C++. @peter.smith might know why that's the case?

Do you have any other suggestion on how to handle this?

[ldionne]

In D53486#1271330, @phosek wrote:

I don't know why we chosen that approach, but Arm Compiler Reference Guide explicitly mentions that _Float16 is being available in C and C++. @peter.smith might know why that's the case?

Do you have any other suggestion on how to handle this?

I see roughly two options:

1. Never provide support for _Float16 in C++ -- _Float16 does not appear to be part of the C11 standard according to http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf (I think it may be part of a TS).
2. If only GCC is not providing _Float16 in C++ but other compilers are providing it, perhaps changing the condition to #if !defined(__GNUC__) (or whatever the #define is, and also filing a complaint against GCC for diverging from other implementations.

[phosek]

In D53486#1271514, @ldionne wrote:

I see roughly two options:

1. Never provide support for _Float16 in C++ -- _Float16 does not appear to be part of the C11 standard according to http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf (I think it may be part of a TS).

It's defined in ISO/IEC TS 18661-3:2015 extension to C11. I'd be fine not defining the type trait for _Float16 at all given that it's not defined by the C++ standard.

2. If only GCC is not providing _Float16 in C++ but other compilers are providing it, perhaps changing the condition to #if !defined(__GNUC__) (or whatever the #define is, and also filing a complaint against GCC for diverging from other implementations.

I don't have any way to check other compilers. I searched the ICC documentation but haven't found any mention of _Float16. I think it's safer to assume that only Clang defines _Float16 in C++ rather than assuming that GCC is the only one not defining it unless we have some way to confirm that.

[peter.smith]

To the best of my knowledge (references in the docs below) the _Float16 is defined in a TS: "ISO/IEC, Floating point extensions for C, ISO/IEC TS 18661-3".
I got that by following the links from the Arm C Library Extensions that refer to an older storage only type called __fp16 and its interaction with _Float16 (Links below)

To the best of my knowledge there is a known discrepancy in clang and gcc with respect to C++ support. It is supported in clang, but not gcc. Unfortunately I don't yet know why that was the case.

As implemented in clang https://reviews.llvm.org/D33719 there is some reference to the C++ support and the name mangling of the type at https://github.com/itanium-cxx-abi/cxx-abi/pull/22

ACLE:

• (Web) https://developer.arm.com/products/software-development-tools/compilers/arm-compiler-5/docs/101028/latest/3-c-language-extensions
• (pdf) https://static.docs.arm.com/101028/0006/Q1-ACLE_2018Q2_release-0006.pdf?_ga=2.91793754.1585046319.1540286457-1604496016.1486999657

[peter.smith]

For what it's worth a colleague pointed me at the GCC commit message https://patchwork.ozlabs.org/patch/638659/ there is a brief mention of C++ (search for C++ note)

[ldionne]

Thinking about this more, I'm fine with either this patch or one that does not define __libcpp_is_floating_point<_Float16> at all. This patch provides the best level of support that we seem to be able to provide, but is inconsistent across compilers and does not follow the standard perfectly. Not providing the definition at all decreases the level of support but increases consistency and simplicity. I'm fine with either (with a small preference towards consistency and simplicity, as a lazy maintainer :-)

[phosek]

In D53486#1272388, @ldionne wrote:

Thinking about this more, I'm fine with either this patch or one that does not define __libcpp_is_floating_point<_Float16> at all. This patch provides the best level of support that we seem to be able to provide, but is inconsistent across compilers and does not follow the standard perfectly. Not providing the definition at all decreases the level of support but increases consistency and simplicity. I'm fine with either (with a small preference towards consistency and simplicity, as a lazy maintainer :-)

A possible reason to go with the current option is __libcpp_is_floating_point<_Float16> that has been in libc++ for a few months and has been shipped with 7.0, so there might already be users depending on this trait. Is that a concern? If yes, then current version is probably the right way to go.

[mclow.lists]

I'm having trouble getting past the following wording in the standard: "There are three floating-point types: float, double, and long double."

that is_floating_point<T>::value is true if T is a floating point type. (reference to above wording).

[mclow.lists]

Searching through the current draft standard, I see the following bits that are not covered by this patch:

• For every pair of floating-point types L and R , there exists a candidate operator function of the form

std::partial_ordering operator<=>(L, R);

• though, to be fair, we don't have the rest of these in libc++ yet.

• A non-type template-parameter shall not be declared to have floating-point or void type. [Example: template<double d> class X; ]
• What about numeric_limits? "Specializations shall be provided for each arithmetic type, both floating-point and integer, including bool"
• Where are the IO operations: operator<< and operator>>? What about to_string?
• How does this hook into `<cfenv>?
• What about all the stuff in <cmath>? acos/asin/atan/etc? They're defined for all the *other* floating point types.
• There's the fun wording in [time.traits.is_fp] about durations with floating point representations.
• How about support for duration literals? *(chrono again)
• No support for complex; that's fine, since "The effect of instantiating the template complex for any type other than float, double, or long double is unspecified"
• No support for atomic_ref; that's allowed, since "There are specializations of the atomic_ref class template for the floating-point types float, double, and long double. "
• No support for atomic; that's allowed, since "There are specializations of the atomic class template for the floating-point types float, double, and long double. "

The point I'm trying to make here is that it's a lot more complicated than saying "Hey - this type here is a floating point type".

[mclow.lists]

In D53486#1273206, @phosek wrote:

A possible reason to go with the current option is __libcpp_is_floating_point<_Float16> that has been in libc++ for a few months and has been shipped with 7.0, so there might already be users depending on this trait. Is that a concern? If yes, then current version is probably the right way to go.

This was added in r333103 by someone at Apple, but Apple has not shipped a set of headers that include this yet (unless they have done so for Mac OS 10.14 - which shipped in the last few weeks), so I would say that the set of people using this is very small.

[phosek]

Spoke to @mclow.lists and he prefers to avoid defining _Float16 altogether so I removed it in the latest version.

[phosek]

Should we remove the __fp16 type trait as well since that one is also Clang specific?

[mclow.lists]

In D53486#1273541, @mclow.lists wrote:

The point I'm trying to make here is that it's a lot more complicated than saying "Hey - this type here is a floating point type".

https://wg21.link/P0192R4, which proposes adding a new floating-point type "short float", has a pretty good list of what is involved

[ldionne]

The original intent of the change was to allow isnan to be called on __fp16. It may be possible to achieve the same goal without adding specializations of is_floating_point, which promises more than what we want to promise here (I agree with Marshall).

[ldionne]

Question: @mclow.lists if __fp16 supported all the proper operations required to be "a floating point type", would there be pushback against defining is_floating_point for __fp16 as a library extension?

[mclow.lists]

In D53486#1274294, @ldionne wrote:

Question: @mclow.lists if __fp16 supported all the proper operations required to be "a floating point type", would there be pushback against defining is_floating_point for __fp16 as a library extension?

I can only speak for myself here - not other people, but I have two concerns; one rooted in the standard, the other more personal.

• The standard makes no provisions for implementations to define additional floating point types. (Contrast this with integral types, where the standard goes on an on about implementation-defined integral types)
• Adding support for another FP type is a lot of work. (again, refer to P0192R4) There's a lot of other stuff that I'd rather be doing.

[jfb]

We're looking into this. Stay tuned.

[ldionne]

After considering the different options and the problems with __fp16, we think it would be better to revert r333103 altogether. __fp16 is not well-behaved on x86 (can't pass it as a function parameter, for example), and so trying to "fully" support it (in the sense of "floating point types") would be messy. Furthermore, the original use case can be solved with a much narrower workaround.

In the longer term, we look forward to p0192 solving this problem and paving the way for "implementation defined floating point types." This would probably require an additional paper.

[mclow.lists]

Thanks.

I have *just* (as in, in the last 10 minutes) become aware of other people interested in "extended floating-point types", and adding such support to the standard, but not for C++20.

[phosek]

OK, in that case I'm going to abandon this change and revert r333103.

[phosek]

Abandoned in favor of D53670.

libcxx/include/type_traits

 #endif
 
 // is_floating_point
 
 template <class _Tp> struct __libcpp_is_floating_point              : public false_type {};
 #ifdef __clang__
 template <>          struct __libcpp_is_floating_point<__fp16>      : public true_type {};
 #endif
-#ifdef __FLT16_MANT_DIG__
-template <>          struct __libcpp_is_floating_point<_Float16>    : public true_type {};
-#endif
 template <>          struct __libcpp_is_floating_point<float>       : public true_type {};
 template <>          struct __libcpp_is_floating_point<double>      : public true_type {};
 template <>          struct __libcpp_is_floating_point<long double> : public true_type {};
 
 template <class _Tp> struct _LIBCPP_TEMPLATE_VIS is_floating_point
     : public __libcpp_is_floating_point<typename remove_cv<_Tp>::type> {};
 
 #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)