Re: [PATCH] c++: Implement P1467R9 - Extended floating-point types and standard names compiler part except for bfloat16 [PR106652]

[Jason Merrill <jason@redhat.com>]

On 9/16/22 13:34, Jakub Jelinek wrote:
> On Fri, Sep 16, 2022 at 01:48:54PM +0200, Jason Merrill wrote:
>> On 9/12/22 04:05, Jakub Jelinek wrote:
>>> The following patch implements the compiler part of C++23
>>> P1467R9 - Extended floating-point types and standard names compiler part
>>> by introducing _Float{16,32,64,128} as keywords and builtin types
>>> like they are implemented for C already since GCC 7.
>>> It doesn't introduce _Float{32,64,128}x for C++, those remain C only
>>> for now, mainly because https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling
>>> has mangling for:
>>> ::= DF <number> _ # ISO/IEC TS 18661 binary floating point type _FloatN (N bits)
>>> but doesn't for _FloatNx.  And it doesn't add anything for bfloat16_t
>>> support, see below.
>>> Regarding mangling, I think mangling _FloatNx as DF <number> x _ would be
>>> possible, but would need to be discussed and voted in.
>>
>> As you've seen, I opened a pull request for these.  I think we can go ahead
>> and implement that and make sure it's resolved before the GCC 13 release.
>>
>> Or we could temporarily mangle them as an extension, i.e. u9_Float32x.
>>
>> I would expect _Float64x, at least, to be fairly popular.
> 
> If we get the mangling for _Float<N>x agreed on, whether it is DFx<N>_ as
> you proposed, or DF<N>x or DF<N>x_, sure, I agree we should just enable
> those too and will tweak the patch.  It would then fix also PR85518.
> 
> Though, when we add support for _Float<N>x and declare they are extended
> floating-point types, the question is about subrank comparisons, shall those
> have lower conversion subrank than _Float<M> type with the same conversion
> rank or the other way around?  Say on x86_64 where _Float32x and _Float64
> has the same rank, shall (_Float32x) x + 0.0f64 have _Float32x type or
> _Float64?
> And, shall we support f32x etc. constant literal suffixes (with pedwarn
> always even in C++23)?
> 
>>> The patch wants to keep backwards compatibility with how __float128 has
>>> been handled in C++ before, both for mangling and behavior in binary
>>> operations, overload resolution etc.  So, there are some backend changes
>>> where for C __float128 and _Float128 are the same type (float128_type_node
>>> and float128t_type_node are the same pointer), but for C++ they are distinct
>>> types which mangle differently and _Float128 is treated as extended
>>> floating-point type while __float128 is treated as non-standard floating
>>> point type.
>>
>> How important do you think this backwards compatibility is?
>>
>> As I mentioned in the ABI proposal, I think it makes sense to make
>> __float128 an alias for std::float128_t, and continue using the current
>> mangling for __float128.
> 
> I thought it is fairly important because __float128 has been around in GCC
> for 19 years already.  To be precise, I think e.g. for x86_64 GCC 3.4
> introduced it, but mangling was implemented only in GCC 4.1 (2006), before we ICEd
> on those.  Until glibc 2.26 (2017) one had to use libquadmath when
> math library functions were needed, but since then one can just use libm.
> __float128 is on some targets (e.g. PA) just another name for long double,
> not a distinct type.

I think we certainly want to continue to support __float128, what I'm 
wondering is how much changing it to mean _Float128 will affect existing 
code.  I would guess that a lot of code that just works on __float128 
will continue to work without modification.  Does anyone know of 
significant existing uses of __float128?

> Another thing are the PowerPC __ieee128 and __ibm128 type, I think for the
> former we can't make it the same type as _Float128, because e.g. libstdc++
> code relies on __ieee128 and __ibm128 being long double type of the other
> ABI, so they should mangle as long double of the other ABI.  But in that
> case they can't act as distinct types when long double should mangle the
> same as they do.  And it would be weird if those types in one
> -mabi=*longdouble mode worked as standard floating-point type and in another
> as extended floating-point type, rather than just types which are neither
> standard nor extended as before.

Absolutely we don't want to mess with __ieee128 and __ibm128.  And I 
guess that means that we need to preserve the non-standard type handling 
for the alternate long double.

I think we can still change __float128 to be _Float128 on PPC and other 
targets where it's currently an alias for long double.

It seems to me that it's a question of what provides the better 
transition path for users.  I imagine we'll want to encourage people to 
replace __float128 with std::float128_t everywhere.

In the existing model, it's not portable whether

void f(long double) { }
void f(__float128) { }

is an overload or an erroneous redefinition.  In the new model, you can 
portably write

void f(long double) { }
void f(std::float128_t) { }

and existing __float128 code will call the second one.  Old code that 
had conditional __float128 overloads when it's different from long 
double will need to change to have unconditional _Float128 overloads.

If we don't change __float128 to mean _Float128, we require fewer 
immediate changes for a library that does try to support all 
floating-point types, but it will need changes to support _Float128 and 
will need to keep around conditional __float128 overloads indefinitely.

>> I don't think we want the two types to have different semantics.  If we want
>> to support existing __float128 code that relies on implicit narrowing
>> conversions, we could allow them generally with a pedwarn using the 'bad'
>> conversion machinery.  That's probably useful anyway for better diagnostics.
> 
> So you mean instead of
> +      if (fcode == REAL_TYPE
> +         && tcode == REAL_TYPE
> +         && (extended_float_type_p (from)
> +             || extended_float_type_p (to))
> +         && cp_compare_floating_point_conversion_ranks (from, to) >= 2)
> +       return NULL;
> before conv = build_conv (ck_std, to, conv); do those checks in else if
> after:
>        if ((same_type_p (to, type_promotes_to (from))
>             || (underlying_type && same_type_p (to, underlying_type)))
>            && next_conversion (conv)->rank <= cr_promotion)
>          conv->rank = cr_promotion;
> and pedwarn there (or somewhere later?) and set conv->bad_p = true;?
> I can certainly try that what will it do on the tests in the patch.

Set bad_p there; the pedwarn should go in the bad_p handling in 
convert_like_internal.

Jason