From: Patrick Palka <ppalka@redhat.com>
To: Jonathan Wakely <jwakely@redhat.com>
Cc: "Jonathan Wakely via Libstdc++" <libstdc++@gcc.gnu.org>,
GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [committed] libstdc++: Use fast_float for long double if it uses binary64 format
Date: Mon, 24 Jan 2022 12:41:26 -0500 [thread overview]
Message-ID: <CAMOnLZYtM9VC6=fUguNShr7WSjxsyfRXBYzbWXg6=+WC1DyN9w@mail.gmail.com> (raw)
In-Reply-To: <20220123225012.641915-1-jwakely@redhat.com>
On Sun, Jan 23, 2022 at 5:53 PM Jonathan Wakely via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Tested powerpc64le-linux, pushed to trunk.
>
>
> We can use the new from_chars implementation when long double and double
> have the same representation.
I suppose we should also update <charconv> to sync the conditions that
define __cpp_lib_to_chars and that declare the fp to_chars overloads?
They're currently guarded by _GLIBCXX_HAVE_USELOCALE which is overly
conservative, as e.g. the float/double overloads are now also
available when _GLIBCXX_FLOAT_IS_IEEE_BINARY32 &&
_GLIBCXX_DOUBLE_IS_IEEE_BINARY64, and additionally the long double
overload is available when _GLIBCXX_LONG_DOUBLE_IS_IEEE_BINARY64.
>
> libstdc++-v3/ChangeLog:
>
> * src/c++17/floating_from_chars.cc (USE_STRTOD_FOR_FROM_CHARS):
> Define macro for case where std::from_chars is implemented in
> terms of strtod, strtof or strtold.
> (buffer_resource, valid_fmt, find_end_of_float, pattern)
> (from_chars_impl, make_result, reserve_string): Do not define
> unless USE_STRTOD_FOR_FROM_CHARS is defined.
> (from_chars): Define when at least one of USE_LIB_FAST_FLOAT and
> USE_STRTOD_FOR_FROM_CHARS is defined, instead of
> _GLIBCXX_HAVE_USELOCALE. Use fast_float for long double when it
> is binary64.
> ---
> libstdc++-v3/src/c++17/floating_from_chars.cc | 38 ++++++++++++++++---
> 1 file changed, 32 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 3158a3a96d3..ba1345db3f2 100644
> --- a/libstdc++-v3/src/c++17/floating_from_chars.cc
> +++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
> @@ -46,6 +46,13 @@
> # include <xlocale.h>
> #endif
>
> +#if _GLIBCXX_HAVE_USELOCALE
> +// FIXME: This should be reimplemented so it doesn't use strtod and newlocale.
> +// That will avoid the need for any memory allocation, meaning that the
> +// non-conforming errc::not_enough_memory result cannot happen.
> +# define USE_STRTOD_FOR_FROM_CHARS 1
> +#endif
> +
> #ifdef _GLIBCXX_LONG_DOUBLE_ALT128_COMPAT
> #ifndef __LONG_DOUBLE_IBM128__
> #error "floating_from_chars.cc must be compiled with -mabi=ibmlongdouble"
> @@ -56,6 +63,9 @@ extern "C" __ieee128 __strtoieee128(const char*, char**);
>
> #if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64
> # define USE_LIB_FAST_FLOAT 1
> +# if __LDBL_MANT_DIG__ == __DBL_MANT_DIG__
> +# undef USE_STRTOD_FOR_FROM_CHARS
> +# endif
> #endif
>
> #if USE_LIB_FAST_FLOAT
> @@ -66,13 +76,13 @@ namespace
> } // anon namespace
> #endif
>
> -#if _GLIBCXX_HAVE_USELOCALE
> namespace std _GLIBCXX_VISIBILITY(default)
> {
> _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> namespace
> {
> +#if USE_STRTOD_FOR_FROM_CHARS
> // A memory resource with a static buffer that can be used for small
> // allocations. At most one allocation using the freestore can be done
> // if the static buffer is insufficient. The callers below only require
> @@ -409,6 +419,7 @@ namespace
> return true;
> }
> #endif
> +#endif // USE_STRTOD_FOR_FROM_CHARS
>
> #if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64
> // If the given ASCII character represents a hexit, return that hexit.
> @@ -771,13 +782,11 @@ namespace
>
> return {first, errc{}};
> }
> -#endif
> +#endif // _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64
>
> } // namespace
>
> -// FIXME: This should be reimplemented so it doesn't use strtod and newlocale.
> -// That will avoid the need for any memory allocation, meaning that the
> -// non-conforming errc::not_enough_memory result cannot happen.
> +#if USE_LIB_FAST_FLOAT || USE_STRTOD_FOR_FROM_CHARS
>
> from_chars_result
> from_chars(const char* first, const char* last, float& value,
> @@ -855,6 +864,21 @@ from_chars_result
> from_chars(const char* first, const char* last, long double& value,
> chars_format fmt) noexcept
> {
> +#if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64 \
> + && ! USE_STRTOD_FOR_FROM_CHARS
> + double dbl_value;
> + from_chars_result result;
> + if (fmt == chars_format::hex)
> + result = __floating_from_chars_hex(first, last, dbl_value);
> + else
> + {
> + static_assert(USE_LIB_FAST_FLOAT);
> + result = fast_float::from_chars(first, last, dbl_value, fmt);
> + }
> + if (result.ec == errc{})
> + value = dbl_value;
> + return result;
> +#else
> errc ec = errc::invalid_argument;
> #if _GLIBCXX_USE_CXX11_ABI
> buffer_resource mr;
> @@ -875,6 +899,7 @@ from_chars(const char* first, const char* last, long double& value,
> fmt = chars_format{};
> }
> return make_result(first, len, fmt, ec);
> +#endif
> }
>
> #ifdef _GLIBCXX_LONG_DOUBLE_COMPAT
> @@ -909,6 +934,7 @@ from_chars(const char* first, const char* last, __ieee128& value,
> }
> #endif
>
> +#endif // USE_LIB_FAST_FLOAT || USE_STRTOD_FOR_FROM_CHARS
> +
> _GLIBCXX_END_NAMESPACE_VERSION
> } // namespace std
> -#endif // _GLIBCXX_HAVE_USELOCALE
> --
> 2.31.1
>
next prev parent reply other threads:[~2022-01-24 17:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-23 22:50 Jonathan Wakely
2022-01-24 17:41 ` Patrick Palka [this message]
2022-01-24 22:52 ` Jonathan Wakely
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='CAMOnLZYtM9VC6=fUguNShr7WSjxsyfRXBYzbWXg6=+WC1DyN9w@mail.gmail.com' \
--to=ppalka@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jwakely@redhat.com \
--cc=libstdc++@gcc.gnu.org \
/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).