[committed] libstdc++: Use fast_float for long double if it uses binary64 format

Message ID 20220123225012.641915-1-jwakely@redhat.com
State Committed
Commit 416b6fc7483b1b4daaab2dc25cc9a916178ba376
Headers
Series [committed] libstdc++: Use fast_float for long double if it uses binary64 format |

Commit Message

Jonathan Wakely Jan. 23, 2022, 10:50 p.m. UTC
  Tested powerpc64le-linux, pushed to trunk.


We can use the new from_chars implementation when long double and double
have the same representation.

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(-)
  

Comments

Patrick Palka Jan. 24, 2022, 5:41 p.m. UTC | #1
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
>
  
Jonathan Wakely Jan. 24, 2022, 10:52 p.m. UTC | #2
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.
  

Patch

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