From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oo1-xc32.google.com (mail-oo1-xc32.google.com [IPv6:2607:f8b0:4864:20::c32]) by sourceware.org (Postfix) with ESMTPS id E5A6D3851C23 for ; Fri, 18 Dec 2020 18:28:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E5A6D3851C23 Received: by mail-oo1-xc32.google.com with SMTP id x23so775595oop.1 for ; Fri, 18 Dec 2020 10:28:40 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rIE/I/InoIe+bOHzAKRt1GhVApHENDGeNFkDwpXeLyA=; b=tJrCMkMZM2/Nx4eKQcpOG4VAKe/QP/QUaprk5bZixALVdRHOHaJON1tlMKd2+jERej TlC7jKZ0pW5LAEyXkKHE+X2Hs3eBsNReV+AU+L5nTKRBcPRwVQKRVcZ6+MZm4RwWm1hF fyXasS4tUdS+q018y93N6P6vXVUZlQ7hyB+n63uLVb9rWInjMXPnLgyfoIpzv7fNdNtZ GNdQ/T2Dc2G+EeOcSauLVZ1YxNWz9lG7x0beixLqWi1oW3pkpHGVktouHXRv62tkVmVm 5q8I42YJ80JIShmj/2yM0MnyjXD/QUyj5GDhQLVOolhErKtRnZGiBSjwxM4P58/6+Mpv uQfQ== X-Gm-Message-State: AOAM530/1jjMVSvZhl50TaEv1chFLVUPwD78q9ZZYysPLpzErrlYAYQ7 /7gQzYC58uXOsVkGGPMKttZrkuQAalWelf9dgF+2Lw== X-Google-Smtp-Source: ABdhPJwIPEeh3zVSsq+9RhrXRfzrqtR93xvvX09WB3+f8lXGGFYMG7IRx46C2LtfyC3T4eiT3ZHkeXUp3I2XP2mzXM4= X-Received: by 2002:a4a:c191:: with SMTP id w17mr3872389oop.1.1608316119918; Fri, 18 Dec 2020 10:28:39 -0800 (PST) MIME-Version: 1.0 References: <20200720123123.GA3215@redhat.com> <20200720141330.GB3215@redhat.com> <20201217143248.GA2309743@redhat.com> <733b888c-2ce9-6fa3-81b7-3031995d705a@idea> <1144aaaa-a21a-5f-4918-4191639890@idea> In-Reply-To: <1144aaaa-a21a-5f-4918-4191639890@idea> From: Christophe Lyon Date: Fri, 18 Dec 2020 19:28:29 +0100 Message-ID: Subject: Re: [PATCH 3/4] libstdc++: Add floating-point std::to_chars implementation To: Patrick Palka Cc: Jonathan Wakely , "Jonathan Wakely via Libstdc++" , GCC Patches X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Dec 2020 18:28:47 -0000 Le ven. 18 d=C3=A9c. 2020 =C3=A0 18:03, Patrick Palka a= =C3=A9crit : > On Fri, 18 Dec 2020, Christophe Lyon wrote: > > > On Fri, 18 Dec 2020 at 16:00, Patrick Palka wrote: > > > > > > On Fri, 18 Dec 2020, Christophe Lyon wrote: > > > > > > > Hi, > > > > > > > > > > > > On Fri, 18 Dec 2020 at 05:13, Patrick Palka via Gcc-patches > > > > wrote: > > > > > > > > > > On Thu, Dec 17, 2020 at 9:32 AM Jonathan Wakely < > jwakely@redhat.com> wrote: > > > > > > > > > > > > On 19/08/20 17:57 -0400, Patrick Palka via Libstdc++ wrote: > > > > > > >On Wed, 22 Jul 2020, Patrick Palka wrote: > > > > > > > > > > > > > >> On Mon, 20 Jul 2020, Patrick Palka wrote: > > > > > > >> > > > > > > >> > On Mon, 20 Jul 2020, Jonathan Wakely wrote: > > > > > > >> > > > > > > > >> > > On 20/07/20 08:53 -0400, Patrick Palka via Libstdc++ > wrote: > > > > > > >> > > > On Mon, 20 Jul 2020, Jonathan Wakely wrote: > > > > > > >> > > > > > > > > > >> > > > > On 19/07/20 23:37 -0400, Patrick Palka via Libstdc++ > wrote: > > > > > > >> > > > > > On Fri, 17 Jul 2020, Patrick Palka wrote: > > > > > > >> > > > > > > > > > > > >> > > > > > > On Fri, 17 Jul 2020, Patrick Palka wrote: > > > > > > >> > > > > > > > > > > > > >> > > > > > > > On Wed, 15 Jul 2020, Patrick Palka wrote: > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > On Tue, 14 Jul 2020, Patrick Palka wrote: > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > This implements the floating-point > std::to_chars overloads for > > > > > > >> > > > > > > float, > > > > > > >> > > > > > > > > > double and long double. We use the Ryu > library to compute the > > > > > > >> > > > > > > shortest > > > > > > >> > > > > > > > > > round-trippable fixed and scientific forms > of a number for > > > > > > >> > > > > float, > > > > > > >> > > > > > > double > > > > > > >> > > > > > > > > > and long double. We also use Ryu for > performing fixed and > > > > > > >> > > > > > > scientific > > > > > > >> > > > > > > > > > formatting of float and double. For > formatting long double with > > > > > > >> > > > > an > > > > > > >> > > > > > > > > > explicit precision argument we use a print= f > fallback. > > > > > > >> > > > > Hexadecimal > > > > > > >> > > > > > > > > > formatting for float, double and long > double is implemented from > > > > > > >> > > > > > > > > > scratch. > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > The supported long double binary formats > are float64 (same as > > > > > > >> > > > > > > double), > > > > > > >> > > > > > > > > > float80 (x86 extended precision), float128 > and ibm128. > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > Much of the complexity of the > implementation is in computing the > > > > > > >> > > > > > > exact > > > > > > >> > > > > > > > > > output length before handing it off to Ryu > (which doesn't do > > > > > > >> > > > > bounds > > > > > > >> > > > > > > > > > checking). In some cases it's hard to > compute the output length > > > > > > >> > > > > > > before > > > > > > >> > > > > > > > > > the fact, so in these cases we instead > compute an upper bound on > > > > > > >> > > > > the > > > > > > >> > > > > > > > > > output length and use a sufficiently-sized > intermediate buffer > > > > > > >> > > > > (if > > > > > > >> > > > > > > the > > > > > > >> > > > > > > > > > output range is smaller than the upper > bound). > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > Another source of complexity is in the > general-with-precision > > > > > > >> > > > > > > formatting > > > > > > >> > > > > > > > > > mode, where we need to do zero-trimming of > the string returned > > > > > > >> > > > > by > > > > > > >> > > > > > > Ryu, and > > > > > > >> > > > > > > > > > where we also take care to avoid having to > format the string a > > > > > > >> > > > > > > second > > > > > > >> > > > > > > > > > time when the general formatting mode > resolves to fixed. > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > Tested on x86_64-pc-linux-gnu, > aarch64-unknown-linux-gnu, > > > > > > >> > > > > > > > > > s390x-ibm-linux-gnu, and > powerpc64-unknown-linux-gnu. > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > libstdc++-v3/ChangeLog: > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > * acinclude.m4 (libtool_VERSION): Bump > to 6:29:0. > > > > > > >> > > > > > > > > > * config/abi/pre/gnu.ver: Add new > exports. > > > > > > >> > > > > > > > > > * configure: Regenerate. > > > > > > >> > > > > > > > > > * include/std/charconv (to_chars): > Declare the floating-point > > > > > > >> > > > > > > > > > overloads for float, double and long > double. > > > > > > >> > > > > > > > > > * src/c++17/Makefile.am (sources): Add > floating_to_chars.cc. > > > > > > >> > > > > > > > > > * src/c++17/Makefile.in: Regenerate. > > > > > > >> > > > > > > > > > * src/c++17/floating_to_chars.cc: New > file. > > > > > > >> > > > > > > > > > * > testsuite/20_util/to_chars/long_double.cc: New test. > > > > > > >> > > > > > > > > > * testsuite/util/testsuite_abi.cc: Add > new symbol version. > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > Here is v2 of this patch, which fixes a buil= d > failure on i386 due > > > > > > >> > > > > to > > > > > > >> > > > > > > > > __int128 being unavailable, by refactoring > the long double binary > > > > > > >> > > > > > > format > > > > > > >> > > > > > > > > selection to avoid referring to __int128 whe= n > it doesn't exist. > > > > > > >> > > > > The > > > > > > >> > > > > > > > > patch also makes the hex formatting for > 80-bit long double use > > > > > > >> > > > > > > uint64_t > > > > > > >> > > > > > > > > instead of __int128 since the mantissa has > exactly 64 bits in this > > > > > > >> > > > > > > case. > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > Here's v3 which just makes some minor stylisti= c > adjustments, and > > > > > > >> > > > > most > > > > > > >> > > > > > > > notably replaces the use of _GLIBCXX_DEBUG wit= h > _GLIBCXX_ASSERTIONS > > > > > > >> > > > > > > > since we just want to enable __glibcxx_assert > and not all of debug > > > > > > >> > > > > mode. > > > > > > >> > > > > > > > > > > > > >> > > > > > > Here's v4, which should now correctly support > using with > > > > > > >> > > > > > > -mlong-double-64 on targets with a large default > long double type. > > > > > > >> > > > > > > This is done by defining the long double to_char= s > overloads as inline > > > > > > >> > > > > > > wrappers around the double overloads within > whenever > > > > > > >> > > > > > > __DBL_MANT_DIG__ equals __LDBL_MANT_DIG__. > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > -- >8 -- > > > > > > >> > > > > > > > > > > > > >> > > > > > > Subject: [PATCH 3/4] libstdc++: Add > floating-point std::to_chars > > > > > > >> > > > > > > implementation > > > > > > >> > > > > > > > > > > > > >> > > > > > > This implements the floating-point std::to_chars > overloads for float, > > > > > > >> > > > > > > double and long double. We use the Ryu library > to compute the > > > > > > >> > > > > shortest > > > > > > >> > > > > > > round-trippable fixed and scientific forms of a > number for float, > > > > > > >> > > > > double > > > > > > >> > > > > > > and long double. We also use Ryu for performing > explicit-precision > > > > > > >> > > > > > > fixed and scientific formatting of float and > double. For > > > > > > >> > > > > > > explicit-precision formatting of long double we > fall back to using > > > > > > >> > > > > > > printf. Hexadecimal formatting for float, doubl= e > and long double is > > > > > > >> > > > > > > implemented from scratch. > > > > > > >> > > > > > > > > > > > > >> > > > > > > The supported long double binary formats are > binary64, binary80 (x86 > > > > > > >> > > > > > > 80-bit extended precision), binary128 and ibm128= . > > > > > > >> > > > > > > > > > > > > >> > > > > > > Much of the complexity of the implementation is > in computing the exact > > > > > > >> > > > > > > output length before handing it off to Ryu (whic= h > doesn't do bounds > > > > > > >> > > > > > > checking). In some cases it's hard to compute > the output length > > > > > > >> > > > > > > beforehand, so in these cases we instead compute > an upper bound on the > > > > > > >> > > > > > > output length and use a sufficiently-sized > intermediate buffer if > > > > > > >> > > > > > > necessary. > > > > > > >> > > > > > > > > > > > > >> > > > > > > Another source of complexity is in the > general-with-precision > > > > > > >> > > > > formatting > > > > > > >> > > > > > > mode, where we need to do zero-trimming of the > string returned by Ryu, > > > > > > >> > > > > > > and where we also take care to avoid having to > format the string a > > > > > > >> > > > > > > second time when the general formatting mode > resolves to fixed. > > > > > > >> > > > > > > > > > > > > >> > > > > > > This implementation is non-conforming in a coupl= e > of ways: > > > > > > >> > > > > > > > > > > > > >> > > > > > > 1. For the shortest hexadecimal formatting, we > currently follow the > > > > > > >> > > > > > > Microsoft implementation's approach of being > consistent with the > > > > > > >> > > > > > > output of printf's '%a' specifier at the > expense of sometimes not > > > > > > >> > > > > > > printing the shortest representation. For > example, the shortest > > > > > > >> > > > > hex > > > > > > >> > > > > > > form of 1.08p+0 is 2.1p-1, but we output the > former instead of the > > > > > > >> > > > > > > latter, as does printf. > > > > > > >> > > > > > > > > > > > > >> > > > > > > 2. The Ryu routines for doing shortest formattin= g > on types larger than > > > > > > >> > > > > > > binary64 use the __int128 type, and some > targets (e.g. i386) have a > > > > > > >> > > > > > > large long double type but lack __int128. Fo= r > such targets we make > > > > > > >> > > > > > > the long double to_chars overloads go through > the double overloads, > > > > > > >> > > > > > > which means we lose precision in the output. > (The mantissa of long > > > > > > >> > > > > > > double is 64 bits on i386, so I think we coul= d > potentially fix this > > > > > > >> > > > > > > by writing a specialized version of the > generic Ryu formatting > > > > > > >> > > > > > > routine which works with uint64_t instead of > __int128.) > > > > > > >> > > > > > > > > > > > > >> > > > > > > 3. The __ibm128 shortest formatting routines > don't guarantee > > > > > > >> > > > > > > round-trippability if the difference between > the high- and > > > > > > >> > > > > low-order > > > > > > >> > > > > > > exponent is too large. This is because we > treat the type as if it > > > > > > >> > > > > > > has a contiguous 105-bit mantissa by merging > the high- and > > > > > > >> > > > > low-order > > > > > > >> > > > > > > mantissas, so we potentially lose precision > from the low-order > > > > > > >> > > > > part. > > > > > > >> > > > > > > Although this precision-dropping behavior is > non-conforming, it > > > > > > >> > > > > seems > > > > > > >> > > > > > > consistent with how printf formats __ibm128. > > > > > > >> > > > > > > > > > > > > >> > > > > > > libstdc++-v3/ChangeLog: > > > > > > >> > > > > > > > > > > > > >> > > > > > > * acinclude.m4 (libtool_VERSION): Bump to 6:29:= 0. > > > > > > >> > > > > > > * config/abi/pre/gnu.ver: Add new exports. > > > > > > >> > > > > > > * configure: Regenerate. > > > > > > >> > > > > > > * include/std/charconv (to_chars): Declare the > floating-point > > > > > > >> > > > > > > overloads for float, double and long double. > > > > > > >> > > > > > > * src/c++17/Makefile.am (sources): Add > floating_to_chars.cc. > > > > > > >> > > > > > > * src/c++17/Makefile.in: Regenerate. > > > > > > >> > > > > > > * src/c++17/floating_to_chars.cc: New file. > > > > > > >> > > > > > > * testsuite/20_util/to_chars/long_double.cc: Ne= w > test. > > > > > > >> > > > > > > * testsuite/util/testsuite_abi.cc: Add new > symbol version. > > > > > > >> > > > > > > --- > > > > > > >> > > > > > > libstdc++-v3/acinclude.m4 | > 2 +- > > > > > > >> > > > > > > libstdc++-v3/config/abi/pre/gnu.ver | > 12 + > > > > > > >> > > > > > > libstdc++-v3/configure | > 2 +- > > > > > > >> > > > > > > libstdc++-v3/include/std/charconv | > 43 + > > > > > > >> > > > > > > libstdc++-v3/src/c++17/Makefile.am | > 1 + > > > > > > >> > > > > > > libstdc++-v3/src/c++17/Makefile.in | > 5 +- > > > > > > >> > > > > > > libstdc++-v3/src/c++17/floating_to_chars.cc | > 1514 > > > > > > >> > > > > +++++++++++++++++ > > > > > > >> > > > > > > .../testsuite/20_util/to_chars/long_double.cc | > 197 +++ > > > > > > >> > > > > > > libstdc++-v3/testsuite/util/testsuite_abi.cc | > 3 +- > > > > > > >> > > > > > > 9 files changed, 1774 insertions(+), 5 > deletions(-) > > > > > > >> > > > > > > create mode 100644 > libstdc++-v3/src/c++17/floating_to_chars.cc > > > > > > >> > > > > > > create mode 100644 > > > > > > >> > > > > libstdc++-v3/testsuite/20_util/to_chars/long_double.= cc > > > > > > >> > > > > > > > > > > > > >> > > > > > > diff --git a/libstdc++-v3/acinclude.m4 > b/libstdc++-v3/acinclude.m4 > > > > > > >> > > > > > > index ee5e0336f2c..e3926e1c9c2 100644 > > > > > > >> > > > > > > --- a/libstdc++-v3/acinclude.m4 > > > > > > >> > > > > > > +++ b/libstdc++-v3/acinclude.m4 > > > > > > >> > > > > > > @@ -3846,7 +3846,7 @@ changequote([,])dnl > > > > > > >> > > > > > > fi > > > > > > >> > > > > > > > > > > > > >> > > > > > > # For libtool versioning info, format is > CURRENT:REVISION:AGE > > > > > > >> > > > > > > -libtool_VERSION=3D6:28:0 > > > > > > >> > > > > > > +libtool_VERSION=3D6:29:0 > > > > > > >> > > > > > > > > > > > > >> > > > > > > # Everything parsed; figure out what files and > settings to use. > > > > > > >> > > > > > > case $enable_symvers in > > > > > > >> > > > > > > diff --git a/libstdc++-v3/config/abi/pre/gnu.ver > > > > > > >> > > > > > > b/libstdc++-v3/config/abi/pre/gnu.ver > > > > > > >> > > > > > > index edf4485e607..9a1bcfd25d1 100644 > > > > > > >> > > > > > > --- a/libstdc++-v3/config/abi/pre/gnu.ver > > > > > > >> > > > > > > +++ b/libstdc++-v3/config/abi/pre/gnu.ver > > > > > > >> > > > > > > @@ -2299,6 +2299,18 @@ GLIBCXX_3.4.28 { > > > > > > >> > > > > > > > > > > > > >> > > > > > > } GLIBCXX_3.4.27; > > > > > > >> > > > > > > > > > > > > >> > > > > > > +GLIBCXX_3.4.29 { > > > > > > >> > > > > > > + # to_chars(char*, char*, [float|double|long > double]) > > > > > > >> > > > > > > + _ZSt8to_charsPcS_[fdeg]; > > > > > > >> > > > > > > + > > > > > > >> > > > > > > + # to_chars(char*, char*, [float|double|long > double], > > > > > > >> > > > > chars_format) > > > > > > >> > > > > > > + _ZSt8to_charsPcS_[fdeg]St12chars_format; > > > > > > >> > > > > > > + > > > > > > >> > > > > > > + # to_chars(char*, char*, [float|double|long > double], > > > > > > >> > > > > chars_format, > > > > > > >> > > > > > > int) > > > > > > >> > > > > > > + _ZSt8to_charsPcS_[fdeg]St12chars_formati; > > > > > > >> > > > > > > + > > > > > > >> > > > > > > +} GLIBCXX_3.4.28; > > > > > > >> > > > > > > + > > > > > > >> > > > > > > # Symbols in the support library (libsupc++) > have their own tag. > > > > > > >> > > > > > > CXXABI_1.3 { > > > > > > >> > > > > > > > > > > > > >> > > > > > > diff --git a/libstdc++-v3/configure > b/libstdc++-v3/configure > > > > > > >> > > > > > > index dd54bd406a9..73f771e7335 100755 > > > > > > >> > > > > > > --- a/libstdc++-v3/configure > > > > > > >> > > > > > > +++ b/libstdc++-v3/configure > > > > > > >> > > > > > > @@ -75231,7 +75231,7 @@ $as_echo "$as_me: > WARNING: =3D=3D=3D Symbol > > > > > > >> > > > > versioning > > > > > > >> > > > > > > will be disabled." >&2;} > > > > > > >> > > > > > > fi > > > > > > >> > > > > > > > > > > > > >> > > > > > > # For libtool versioning info, format is > CURRENT:REVISION:AGE > > > > > > >> > > > > > > -libtool_VERSION=3D6:28:0 > > > > > > >> > > > > > > +libtool_VERSION=3D6:29:0 > > > > > > >> > > > > > > > > > > > > >> > > > > > > # Everything parsed; figure out what files and > settings to use. > > > > > > >> > > > > > > case $enable_symvers in > > > > > > >> > > > > > > diff --git a/libstdc++-v3/include/std/charconv > > > > > > >> > > > > > > b/libstdc++-v3/include/std/charconv > > > > > > >> > > > > > > index cc7dd0e3758..bd59924f7e7 100644 > > > > > > >> > > > > > > --- a/libstdc++-v3/include/std/charconv > > > > > > >> > > > > > > +++ b/libstdc++-v3/include/std/charconv > > > > > > >> > > > > > > @@ -688,6 +688,49 @@ namespace __detail > > > > > > >> > > > > > > operator^=3D(chars_format& __lhs, chars_forma= t > __rhs) noexcept > > > > > > >> > > > > > > { return __lhs =3D __lhs ^ __rhs; } > > > > > > >> > > > > > > > > > > > > >> > > > > > > + // Floating-point std::to_chars > > > > > > >> > > > > > > + > > > > > > >> > > > > > > + // Overloads for float. > > > > > > >> > > > > > > + to_chars_result to_chars(char* __first, char* > __last, float > > > > > > >> > > > > __value) > > > > > > >> > > > > > > noexcept; > > > > > > >> > > > > > > + to_chars_result to_chars(char* __first, char* > __last, float > > > > > > >> > > > > __value, > > > > > > >> > > > > > > + chars_format __fmt) > noexcept; > > > > > > >> > > > > > > + to_chars_result to_chars(char* __first, char* > __last, float > > > > > > >> > > > > __value, > > > > > > >> > > > > > > + chars_format __fmt, > int __precision) > > > > > > >> > > > > noexcept; > > > > > > >> > > > > > > + > > > > > > >> > > > > > > + // Overloads for double. > > > > > > >> > > > > > > + to_chars_result to_chars(char* __first, char* > __last, double > > > > > > >> > > > > __value) > > > > > > >> > > > > > > noexcept; > > > > > > >> > > > > > > + to_chars_result to_chars(char* __first, char* > __last, double > > > > > > >> > > > > __value, > > > > > > >> > > > > > > + chars_format __fmt) > noexcept; > > > > > > >> > > > > > > + to_chars_result to_chars(char* __first, char* > __last, double > > > > > > >> > > > > __value, > > > > > > >> > > > > > > + chars_format __fmt, > int __precision) > > > > > > >> > > > > noexcept; > > > > > > >> > > > > > > + > > > > > > >> > > > > > > + // Overloads for long double. > > > > > > >> > > > > > > + to_chars_result to_chars(char* __first, char* > __last, long double > > > > > > >> > > > > > > __value) > > > > > > >> > > > > > > + noexcept; > > > > > > >> > > > > > > + to_chars_result to_chars(char* __first, char* > __last, long double > > > > > > >> > > > > > > __value, > > > > > > >> > > > > > > + chars_format __fmt) > noexcept; > > > > > > >> > > > > > > + to_chars_result to_chars(char* __first, char* > __last, long double > > > > > > >> > > > > > > __value, > > > > > > >> > > > > > > + chars_format __fmt, > int __precision) > > > > > > >> > > > > noexcept; > > > > > > >> > > > > > > + > > > > > > >> > > > > > > + // If long double has the same binary format > as double, then we > > > > > > >> > > > > just > > > > > > >> > > > > > > define > > > > > > >> > > > > > > + // the long double overloads as wrappers > around the corresponding > > > > > > >> > > > > > > double > > > > > > >> > > > > > > + // overloads. > > > > > > >> > > > > > > +#if __LDBL_MANT_DIG__ =3D=3D __DBL_MANT_DIG__ > > > > > > >> > > > > > > + inline to_chars_result > > > > > > >> > > > > > > + to_chars(char* __first, char* __last, long > double __value) noexcept > > > > > > >> > > > > > > + { return to_chars(__first, __last, > double(__value)); } > > > > > > >> > > > > > > + > > > > > > >> > > > > > > + inline to_chars_result > > > > > > >> > > > > > > + to_chars(char* __first, char* __last, long > double __value, > > > > > > >> > > > > > > + chars_format __fmt) noexcept > > > > > > >> > > > > > > + { return to_chars(__first, __last, > double(__value), __fmt); } > > > > > > >> > > > > > > + > > > > > > >> > > > > > > + inline to_chars_result > > > > > > >> > > > > > > + to_chars(char* __first, char* __last, long > double __value, > > > > > > >> > > > > > > + chars_format __fmt, int __precision) > noexcept > > > > > > >> > > > > > > + { return to_chars(__first, __last, > double(__value), __fmt, > > > > > > >> > > > > > > __precision); } > > > > > > >> > > > > > > +#endif > > > > > > >> > > > > > > > > > > > >> > > > > > Hmm, I think this approach for supporting > -mlong-double-64 might > > > > > > >> > > > > > introduce an ODR violation because each long doubl= e > to_chars overload > > > > > > >> > > > > > could potentially have two different definitions > available in a program, > > > > > > >> > > > > > one out-of-line in floating_to_chars.cc (compiled > without > > > > > > >> > > > > > -mlong-double-64) and another inline in > (compiled with > > > > > > >> > > > > > -mlong-double-64).. > > > > > > >> > > > > > > > > > > >> > > > > But they have different mangled names, so there's no > ODR violation. > > > > > > >> > > > > The 64-bit long double is mangled as 'e' and the > 128-bit long double > > > > > > >> > > > > is mangled as __float128. You *will* get an ODR > violation on targets > > > > > > >> > > > > where there's no -mlong-double-64 switch, where > double and long double > > > > > > >> > > > > are always the same representation. > > > > > > >> > > > > > > > > > > >> > > > > What I'm doing for std::from_chars is adding this in > the new > > > > > > >> > > > > src/c++17/floating_from_chars.cc file: > > > > > > >> > > > > > > > > > > >> > > > > #ifdef _GLIBCXX_LONG_DOUBLE_COMPAT > > > > > > >> > > > > #pragma GCC diagnostic ignored "-Wattribute-alias" > > > > > > >> > > > > extern "C" from_chars_result > > > > > > >> > > > > _ZSt10from_charsPKcS0_ReSt12chars_format(double) > > > > > > >> > > > > __attribute__((alias > ("_ZSt10from_charsPKcS0_RdSt12chars_format"))); > > > > > > >> > > > > #endif > > > > > > >> > > > > > > > > > > >> > > > > This just defines the > _ZSt10from_charsPKcS0_ReSt12chars_format symbol > > > > > > >> > > > > (i.e. from_chars for 64-bit long double) as an alias > of > > > > > > >> > > > > _ZSt10from_charsPKcS0_RdSt12chars_format (i.e. > from_chars for 64-bit > > > > > > >> > > > > double). > > > > > > >> > > > > > > > > > >> > > > Aha, that makes sense. I'll follow suit for > std::to_chars. > > > > > > >> > > > > > > > > >> > > Actually that should be: > > > > > > >> > > > > > > > > >> > > #ifdef _GLIBCXX_LONG_DOUBLE_COMPAT > > > > > > >> > > extern "C" from_chars_result > > > > > > >> > > _ZSt10from_charsPKcS0_ReSt12chars_format(const char* > first, const char* last, > > > > > > >> > > long double& value, > > > > > > >> > > chars_format fmt) > noexcept > > > > > > >> > > __attribute__((alias > ("_ZSt10from_charsPKcS0_RdSt12chars_format"))); > > > > > > >> > > #endif > > > > > > >> > > > > > > > > >> > > With the right parameter list I don't need to disable th= e > warning. > > > > > > >> > > > > > > > >> > Sounds good. Here's patch v5 that defines such aliases, > tested so far > > > > > > >> > on x86_64-pc-linux-gnu and on powerpc64le-unknown-linux-gn= u. > > > > > > >> > > > > > > >> Here's v6 which is rebased against the floating-point > from_chars patch > > > > > > >> and which also works around a false-positive > -Wmaybe-uninitialized > > > > > > >> warning in __floating_to_chars_hex, as well as performs some > minor > > > > > > >> commentary/style cleanups: > > > > > > > > > > > > > >Here's v7, with the following changes: > > > > > > > > > > > > > >* Remove extraneous indentation (two spaces when inside the st= d > > > > > > > namespace, two spaces after a template header) for better > consistency > > > > > > > with floating_from_chars.cc > > > > > > >* Guard the calls to fe[gs]etround with a preprocessor test fo= r > > > > > > > _GLIBCXX_USE_C99_FENV_TR1; > > > > > > >* Properly XFAIL the new long_double.cc test on targets with a > > > > > > > large long double type but without __int128, most notably i3= 86 > > > > > > >* Reword the commit message slightly. > > > > > > > > > > > > > >-- >8 -- > > > > > > > > > > > > > >Subject: [PATCH] libstdc++: Add floating-point std::to_chars > implementation > > > > > > > > > > > > > >This implements the floating-point std::to_chars overloads for > float, > > > > > > >double and long double. We use the Ryu library to compute the > shortest > > > > > > >round-trippable fixed and scientific forms for float, double > and long > > > > > > >double. We also use Ryu for performing explicit-precision > fixed and > > > > > > >scientific formatting of float and double. For > explicit-precision > > > > > > >formatting of long double we fall back to using printf. > Hexadecimal > > > > > > >formatting for float, double and long double is implemented fr= om > > > > > > >scratch. > > > > > > > > > > > > > >The supported long double binary formats are binary64, binary8= 0 > (x86 > > > > > > >80-bit extended precision), binary128 and ibm128. > > > > > > > > > > > > > >Much of the complexity of the implementation is in computing > the exact > > > > > > >output length before handing it off to Ryu (which doesn't do > bounds > > > > > > >checking). In some cases it's hard to compute the output leng= th > > > > > > >beforehand, so in these cases we instead compute an upper boun= d > on the > > > > > > >output length and use a sufficiently-sized intermediate buffer > only if > > > > > > >necessary. > > > > > > > > > > > > > >Another source of complexity is in the general-with-precision > formatting > > > > > > >mode, where we need to do zero-trimming of the string returned > by Ryu, > > > > > > >and where we also take care to avoid having to format the > number through > > > > > > >Ryu a second time when the general formatting mode resolves to > fixed > > > > > > >(which we determine by doing a scientific formatting first and > > > > > > >inspecting the scientific exponent). We avoid going through > Ryu twice > > > > > > >by instead transforming the scientific form to the > corresponding fixed > > > > > > >form via in-place string manipulation. > > > > > > > > > > > > > >This implementation is non-conforming in a couple of ways: > > > > > > > > > > > > > >1. For the shortest hexadecimal formatting, we currently follo= w > the > > > > > > > Microsoft implementation's decision to be consistent with t= he > > > > > > > output of printf's '%a' specifier at the expense of > sometimes not > > > > > > > printing the shortest representation. For example, the > shortest hex > > > > > > > form for the number 1.08p+0 is 2.1p-1, but we output the > former > > > > > > > instead of the latter, as does printf. > > > > > > > > > > > > > >2. The Ryu routine generic_binary_to_decimal that we use for > performing > > > > > > > shortest formatting for large floating point types is > implemented > > > > > > > using the __int128 type, but some targets with a large long > double > > > > > > > type lack __int128 (e.g. i686), so we can't perform shortes= t > > > > > > > formatting of long double on such targets through Ryu. As = a > > > > > > > temporary stopgap this patch makes the long double to_chars > overloads > > > > > > > just dispatch to the double overloads on these targets, > which means > > > > > > > we lose precision in the output. (We could potentially fix > this by > > > > > > > writing a specialized version of Ryu's > generic_binary_to_decimal > > > > > > > routine that uses uint64_t instead of __int128.) [Though I > wonder if > > > > > > > there's a better way to work around the lack of __int128 on > i686 > > > > > > > specifically?] > > > > > > > > > > > > > >3. Our shortest formatting for __ibm128 doesn't guarantee the > round-trip > > > > > > > property if the difference between the high- and low-order > exponent > > > > > > > is large. This is because we treat __ibm128 as if it has a > > > > > > > contiguous 105-bit mantissa by merging the mantissas of the > high- > > > > > > > and low-order parts (using code extracted from glibc), so w= e > > > > > > > potentially lose precision from the low-order part. This > seems to be > > > > > > > consistent with how glibc printf formats __ibm128. > > > > > > > > > > > > > >libstdc++-v3/ChangeLog: > > > > > > > > > > > > > > * config/abi/pre/gnu.ver: Add new exports. > > > > > > > * include/std/charconv (to_chars): Declare the > floating-point > > > > > > > overloads for float, double and long double. > > > > > > > * src/c++17/Makefile.am (sources): Add > floating_to_chars.cc. > > > > > > > * src/c++17/Makefile.in: Regenerate. > > > > > > > * src/c++17/floating_to_chars.cc: New file. > > > > > > > (to_chars): Define for float, double and long double. > > > > > > > * testsuite/20_util/to_chars/long_double.cc: New test. > > > > > > > > > > > > Sorry it took so long to review, this is OK for trunk. > > > > > > > > > > > > The patch needs some minor changes to rebase it on the current > trunk: > > > > > > The linker script has additions since you send this patch, so t= he > > > > > > context in the patch is wrong and it doesn't apply, and in > > > > > > > the first line of context in the patch needs to have 'noexcept' > added. > > > > > > That rebase should be easy though. > > > > > > > > > > > > I'll look at adding __float128 support for powerpc64le. > > > > > > > > > > Thanks a lot. I committed the patch series just now, after > rebasing > > > > > and retesting on x86_64, aarch64 and ppc64le. > > > > > > > > > > > > > My newlib-based toolchains (arm-eabi, aarch64-elf) fail to build > after > > > > this commit, because: > > > > libstdc++-v3/src/c++17/floating_to_chars.cc:951:40: error: > > > > 'FE_TONEAREST' was not declared in this scope > > > > 951 | if (saved_rounding_mode !=3D FE_TONEAREST) > > > > > > > > I'm (still) using newlib-3.3.0, and I think newlib's fenv.h was > updated > > > > this year for arm/aarch64, so I suspect bumping to 3.4.0 would > > > > avoid the problem. However, is this something you want to support? > > > > (I mean the possibility that FE_TONEAREST is not supported etc...) > > > > > > Thanks for the heads up. Yes, it seems we should be testing that > > > FE_TONEAREST is defined before using it or fegetround()/fesetround(). > > > This'll fix the build breakage and is also more conforming to the C > > > standard, which says that the latter functions work only if this macr= o > > > is defined. > > > > > > Does the following patch look OK to commit? > > > > > > -- >8 -- > > > > > > Subject: [PATCH] libstdc++: Check FE_TONEAREST is defined before usin= g > it > > > > > > We need to check that FE_TONEAREST is defined before using it and > > > fegetround/fesetround to adjust the floating-point rounding mode > > > (as per the specification of fenv.h). > > > > > > libstdc++-v3/ChangeLog: > > > > > > * src/c++17/floating_from_chars.cc (from_chars_impl): Check > > > FE_TONEAREST is defined before adjusting the rounding mode. > > > * src/c++17/floating_to_chars.cc > (__floating_to_chars_precision): > > > Likewise. > > > > Thanks, that's better but not sufficient: > > /libstdc++-v3/src/c++17/floating_to_chars.cc: In instantiation of > > 'std::to_chars_result std::__floating_to_chars_shortest(char*, char*, > > T, std::chars_format) [with T =3D float]': > > /libstdc++-v3/src/c++17/floating_to_chars.cc:1484:73: required from > here > > /libstdc++-v3/src/c++17/floating_to_chars.cc:978:37: error: no > > matching function for call to 'max(long int, int)' > > 978 | const int whole_digits =3D max(mantissa_length + > fd.exponent, 1); > > > > are we just missing a cast? > > Ah yes, I think this was also reported earlier today as PR libstdc++/9837= 0. > I just went ahead and committed the above FE_TONEAREST fix and the > proposed PR98370 fix under the "obvious" rule. Does latest trunk build > for you now? > > Yes, thanks ! > > > > > > --- > > > libstdc++-v3/src/c++17/floating_from_chars.cc | 4 ++-- > > > libstdc++-v3/src/c++17/floating_to_chars.cc | 8 ++++---- > > > 2 files changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc > b/libstdc++-v3/src/c++17/floating_from_chars.cc > > > index a1943351493..86d6d0645a9 100644 > > > --- a/libstdc++-v3/src/c++17/floating_from_chars.cc > > > +++ b/libstdc++-v3/src/c++17/floating_from_chars.cc > > > @@ -307,7 +307,7 @@ namespace > > > { > > > locale_t orig =3D ::uselocale(loc); > > > > > > -#if _GLIBCXX_USE_C99_FENV_TR1 > > > +#if _GLIBCXX_USE_C99_FENV_TR1 && defined(FE_TONEAREST) > > > const int rounding =3D std::fegetround(); > > > if (rounding !=3D FE_TONEAREST) > > > std::fesetround(FE_TONEAREST); > > > @@ -333,7 +333,7 @@ namespace > > > #endif > > > const int conv_errno =3D std::__exchange(errno, save_errno); > > > > > > -#if _GLIBCXX_USE_C99_FENV_TR1 > > > +#if _GLIBCXX_USE_C99_FENV_TR1 && defined(FE_TONEAREST) > > > if (rounding !=3D FE_TONEAREST) > > > std::fesetround(rounding); > > > #endif > > > diff --git a/libstdc++-v3/src/c++17/floating_to_chars.cc > b/libstdc++-v3/src/c++17/floating_to_chars.cc > > > index dd83f5eea93..474e791e717 100644 > > > --- a/libstdc++-v3/src/c++17/floating_to_chars.cc > > > +++ b/libstdc++-v3/src/c++17/floating_to_chars.cc > > > @@ -946,13 +946,13 @@ template > > > // digit, and carefully compute and write the last digit > > > // ourselves. > > > char buffer[expected_output_length+1]; > > > -#if _GLIBCXX_USE_C99_FENV_TR1 > > > +#if _GLIBCXX_USE_C99_FENV_TR1 && defined(FE_TONEAREST) > > > const int saved_rounding_mode =3D fegetround(); > > > if (saved_rounding_mode !=3D FE_TONEAREST) > > > fesetround(FE_TONEAREST); // We want round-to-nearest > behavior. > > > #endif > > > const int output_length =3D sprintf(buffer, "%.0Lf", valu= e); > > > -#if _GLIBCXX_USE_C99_FENV_TR1 > > > +#if _GLIBCXX_USE_C99_FENV_TR1 && defined(FE_TONEAREST) > > > if (saved_rounding_mode !=3D FE_TONEAREST) > > > fesetround(saved_rounding_mode); > > > #endif > > > @@ -1139,14 +1139,14 @@ template > > > > > > // Do the sprintf into the local buffer. > > > char buffer[output_length_upper_bound+1]; > > > -#if _GLIBCXX_USE_C99_FENV_TR1 > > > +#if _GLIBCXX_USE_C99_FENV_TR1 && defined(FE_TONEAREST) > > > const int saved_rounding_mode =3D fegetround(); > > > if (saved_rounding_mode !=3D FE_TONEAREST) > > > fesetround(FE_TONEAREST); // We want round-to-nearest > behavior. > > > #endif > > > int output_length > > > =3D sprintf(buffer, output_specifier, effective_precision, > value); > > > -#if _GLIBCXX_USE_C99_FENV_TR1 > > > +#if _GLIBCXX_USE_C99_FENV_TR1 && defined(FE_TONEAREST) > > > if (saved_rounding_mode !=3D FE_TONEAREST) > > > fesetround(saved_rounding_mode); > > > #endif > > > -- > > > 2.30.0.rc0 > > > > > > > > >