public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: Jonathan Wakely <jwakely@redhat.com>,
	"Joseph S. Myers" <joseph@codesourcery.com>,
	Bruce Korb <bkorb@gnu.org>,
	gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++: Implement P1467R9 - Extended floating-point types and standard names compiler part except for bfloat16 [PR106652]
Date: Fri, 16 Sep 2022 19:34:03 +0200	[thread overview]
Message-ID: <YySzi9ltIjCoOIoy@tucnak> (raw)
In-Reply-To: <d792506c-6782-e9b1-058c-62903c91e6b6@redhat.com>

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.

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.

> 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.

> > The patch predefines __STDCPP_FLOAT{16,32,64,128}_T__ macros when
> > those types are available, but only for C++23, while the underlying types
> > are available in C++98 and later including the {f,F}{16,32,64,128} literal
> > suffixes (but those with a pedwarn for C++20 and earlier).  My understanding
> > is that it needs to be predefined by the compiler, on the other side
> > predefining even for older modes when <stdfloat> is a new C++23 header
> > would be weird.  One can find out if _Float{16,32,64,128} is supported in
> > C++ by
> > defined(__FLT{16,32,64,128}_MANT_DIG__) && !defined(__FLT32X_MANT_DIG__)
> > (unfortunately not just the former because GCC 7-12 predefined those too)
> > or perhaps __GNUC__ >= 13 && defined(__FLT{16,32,64,128}_MANT_DIG__)
> > (but that doesn't work well with older G++ 13 snapshots).
> 
> As Joseph says, I wouldn't worry about older GCC 13 snapshots.

Ok.

> > As for std::bfloat16_t, three targets (aarch64, arm and x86) apparently
> > "support" __bf16 type which has the bfloat16 format, but isn't really
> > usable, e.g. {aarch64,arm,ix86}_invalid_conversion disallow any conversions
> > from or to type with BFmode, {aarch64,arm,ix86}_invalid_unary_op disallows
> > any unary operations on those except for ADDR_EXPR and
> > {aarch64,arm,ix86}_invalid_binary_op disallows any binary operation on
> > those.  So, I think we satisfy:
> > "If the implementation supports an extended floating-point type with the
> > properties, as specified by ISO/IEC/IEEE 60559, of radix (b) of 2, storage
> > width in bits (k) of 16, precision in bits (p) of 8, maximum exponent (emax)
> > of 127, and exponent field width in bits (w) of 8, then the typedef-name
> > std::bfloat16_t is defined in the header <stdfloat> and names such a type,
> > the macro __STDCPP_BFLOAT16_T__ is defined, and the floating-point literal
> > suffixes bf16 and BF16 are supported."
> > because we don't really support those right now.
> > The question is (mainly for aarch64, arm and x86 backend maintainers) if we
> > shouldn't support it, in the PR there is a partial patch to do so, but
> > the big question is if it should be supported as the __bf16 type those
> > 3 targets use with u6__bf16 mangling and remove those *_invalid_* cases
> > and add conversions to/from at least SFmode but probably also DFmode, TFmode
> > and XFmode on x86 and implement arithmetics on those through conversion to
> > SFmode, performing arithmetics there and conversion back.
> 
> Sounds good.  And I've proposed DFb16_ mangling.

I saw, thanks for doing that.

> I'll leave that question to people more involved with FP codegen.

Yeah, we'll need to discuss this with the ARM and Intel folks.
In any case, that part is for incremental changes.

> > Or there is the possibility to keep __bf16 a lame type one can't convert
> > to/from or perform most unary and all binary ops on it, and add for C++
> > a new type (say __bfloat16_t or whatever else), agree on mangling in
> > Itanium ABI and implement full support just for that type.
> 
> Preserving the existing useless semantics doesn't sound worthwhile.

Ok.

	Jakub


  reply	other threads:[~2022-09-16 17:34 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-12  8:05 Jakub Jelinek
2022-09-12 19:36 ` Joseph Myers
2022-09-12 20:52   ` Jakub Jelinek
2022-09-12 21:00     ` Jakub Jelinek
2022-09-13 17:50     ` Joseph Myers
2022-09-16 11:48 ` Jason Merrill
2022-09-16 17:34   ` Jakub Jelinek [this message]
2022-09-17  8:58     ` Jason Merrill
2022-09-19 16:39       ` Jakub Jelinek
2022-09-26 21:15         ` Jason Merrill
2022-09-26 22:11           ` Jakub Jelinek
2022-09-20  3:35 ` Hongtao Liu
2022-09-20  7:14   ` Hongtao Liu
2022-09-20  8:51   ` Jakub Jelinek
2022-09-22 15:56     ` [RFC PATCH] __trunc{tf,xf,df,sf,hf}bf2, __truncbfhf2 and __extendbfsf2 Jakub Jelinek
2022-09-23  0:44       ` Hongtao Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YySzi9ltIjCoOIoy@tucnak \
    --to=jakub@redhat.com \
    --cc=bkorb@gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=jwakely@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).