Message ID | 20220123225012.641915-1-jwakely@redhat.com |
---|---|
State | New |
Headers | show |
Series | [committed] libstdc++: Use fast_float for long double if it uses binary64 format | expand |
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 >
On Mon, 24 Jan 2022 at 17:42, Patrick Palka via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > 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. Ah yes, we definitely should.
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