public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jason Merrill <jason@redhat.com>
To: Jakub Jelinek <jakub@redhat.com>,
	Jonathan Wakely <jwakely@redhat.com>,
	"Joseph S. Myers" <joseph@codesourcery.com>,
	Bruce Korb <bkorb@gnu.org>
Cc: 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 13:48:54 +0200	[thread overview]
Message-ID: <d792506c-6782-e9b1-058c-62903c91e6b6@redhat.com> (raw)
In-Reply-To: <Yx7oXsjWwmgsaj/A@tucnak>

On 9/12/22 04:05, Jakub Jelinek wrote:
> Hi!
> 
> 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.

> As there is no _FloatNx support for C++, I think it is wrong to announce
> it through __FLT{32,64,128}X_*__ predefined macros (so the patch disables
> those for C++; unfortunately g++ 7 to 12 will predefine those and also
> __FLT{32,64,128}_*__ even when _FloatN support isn't implemented).

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

> The various C++23 changes about how floating-point types
> are changed are actually implemented as written in the spec only if at least
> one of the types involved is _Float{16,32,64,128} and kept previous behavior
> otherwise.  For float/double/long double the rules are actually written that
> they behave the same as before.
> There is some backwards incompatibility at least on x86 regarding _Float16,
> because that type was already used by that name and with the DF16_ mangling
> (but only since GCC 12 and I think it isn't that widely used in the wild
> yet).  E.g. config/i386/avx512fp16intrin.h shows the issues, where
> in C or in GCC 12 in C++ one could pass 0.0f to a builtin taking _Float16
> argument, but with the changes that is not possible anymore, one needs
> to either use 0.0f16 or (_Float16) 0.0f.
> We have also a problem with glibc headers, where since glibc 2.27
> math.h and complex.h aren't compilable with these changes.  One gets
> errors like:
> In file included from /usr/include/math.h:43,
>                   from abc.c:1:
> /usr/include/bits/floatn.h:86:9: error: multiple types in one declaration
>     86 | typedef __float128 _Float128;
>        |         ^~~~~~~~~~
> /usr/include/bits/floatn.h:86:20: error: declaration does not declare anything [-fpermissive]
>     86 | typedef __float128 _Float128;
>        |                    ^~~~~~~~~
> In file included from /usr/include/bits/floatn.h:119:
> /usr/include/bits/floatn-common.h:214:9: error: multiple types in one declaration
>    214 | typedef float _Float32;
>        |         ^~~~~
> /usr/include/bits/floatn-common.h:214:15: error: declaration does not declare anything [-fpermissive]
>    214 | typedef float _Float32;
>        |               ^~~~~~~~
> /usr/include/bits/floatn-common.h:251:9: error: multiple types in one declaration
>    251 | typedef double _Float64;
>        |         ^~~~~~
> /usr/include/bits/floatn-common.h:251:16: error: declaration does not declare anything [-fpermissive]
>    251 | typedef double _Float64;
>        |                ^~~~~~~~
> This is from snippets like:
> /* The remaining of this file provides support for older compilers.  */
> # if __HAVE_FLOAT128
> 
> /* The type _Float128 exists only since GCC 7.0.  */
> #  if !__GNUC_PREREQ (7, 0) || defined __cplusplus
> typedef __float128 _Float128;
> #  endif
> where it hardcodes that C++ doesn't have _Float{16,32,64,128} support nor
> {f,F}{16,32,64,128} literal suffixes nor _Complex _Float{16,32,64,128}.
> The patch fixincludes this for now and hopefully if this is committed, then
> glibc can change those.  Right now the patch changes those
> #  if !__GNUC_PREREQ (7, 0) || defined __cplusplus
> conditions to
> #  if !__GNUC_PREREQ (7, 0) || (defined __cplusplus && !__GNUC_PREREQ (13, 1) && defined __FLT32X_MANT_DIG__)
> where it relies on __FLT32X_*__ macros no longer being predefined for C++.
> Now, I guess for the fixincludes it could also use
> #  if !__GNUC_PREREQ (7, 0) || (defined __cplusplus && !__GNUC_PREREQ (13, 0))
> where earlier GCC 13 snapshots would not be doing the fixincludes,
> but the question is what to use for upstream glibc, because
> there will be 13.0 snapshots where C++ doesn't support _Float{16,32,64,128}
> and where it is essential to use what glibc has been doing previously
> and using the #else would fail miserably, and then 13.0 snapshots where it
> does support it and where using the if would fail miserably.
> One option is to use
> #  if !__GNUC_PREREQ (7, 0) || (defined __cplusplus && !__GNUC_PREREQ (13, 1))
> in glibc and rely on fixincludes for 13.0 snapshots (or of course later when
> using 13.1+ with older glibc).
> Another thing is mangling, as said above, Itanium C++ ABI specifies
> DF <number> _ as _Float{16,32,64,128} mangling, but GCC was implementing
> a mangling incompatible with that starting with DF for fixed point types.
> Fixed point was never supported in C++ though, I believe the reason why
> the mangling has been added was that due to a bug it would leak into the
> C++ FE through decltype (0.0r) etc.  But that has been shortly after the
> mangling was added fixed (I think in the same GCC release cycle), so we
> now reject 0.0r etc. in C++.  If we ever need the fixed point mangling,
> I think it can be readded but better with a different prefix so that it
> doesn't conflict with the published standard manglings.  So, this patch
> also kills the fixed point mangling and implements the DF <number> _
> demangling.

Sounds good.

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

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

> Conversion from BFmode to SFmode is easy, left shift by 16 and ought to be
> implemented inline, SFmode -> BFmode conversion is harder,
> I think it is roughly:
> __bf16
> __truncsfbf2 (_Float32 x)
> {
>    unsigned int y;
>    memcpy (&y, &x, sizeof (y));
>    unsigned int z = x & 0x7fffffff;
>    unsigned short r;
>    __bf16 ret;
>    if (z < 0x800000)
>      // Zero or denormal, flush to zero.
>      r = (x & 0x80000000) >> 16;
>    else if (z < 0x7f800000)
>      // Normal, round to nearest.
>      r = (x + 0x7fff + ((x >> 16) & 1)) >> 16;
>    else if (z == 0x7f800000)
>      // Inf.
>      r = x >> 16;
>    else
>      // NaN.
>      r = (x >> 16) | (1 << 6);
>    memcpy (&ret, &r, sizeof (r));
>    return ret;
> }
> (untested) and the question is if it should be implemented in libgcc
> (and using soft-fp or not), or inline, or both depending on -Os.

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

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

Jason


  parent reply	other threads:[~2022-09-16 11:49 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 [this message]
2022-09-16 17:34   ` Jakub Jelinek
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=d792506c-6782-e9b1-058c-62903c91e6b6@redhat.com \
    --to=jason@redhat.com \
    --cc=bkorb@gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@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).