[committed] libstdc++: Fix detection of std::format support for __float128 [PR107693]

Message ID 20221115143119.1155190-1-jwakely@redhat.com
State Unresolved
Headers
Series [committed] libstdc++: Fix detection of std::format support for __float128 [PR107693] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jonathan Wakely Nov. 15, 2022, 2:31 p.m. UTC
  Tested x86_64-linux and x86_64-w64-mingw32. Pushed to trunk.

-- >8 --

std::format gives linker errors on targets that define __float128 but
do not support using it with std::to_chars. This improves the handling
of 128-bit flaoting-point types so they are disabled if unsupportable.

libstdc++-v3/ChangeLog:

	PR libstdc++/107693
	* include/std/format (_GLIBCXX_FORMAT_F128): Define to 2 when
	basic_format_arg needs to use its _M_f128 member.
	(__extended_floating_point, __floating_point): Replace with ...
	(__formattable_floating_point): New concept.
	* testsuite/std/format/functions/format.cc: Check whether
	__float128 is supported. Also test _Float128.
---
 libstdc++-v3/include/std/format               | 77 ++++++++++---------
 .../testsuite/std/format/functions/format.cc  | 20 ++++-
 2 files changed, 59 insertions(+), 38 deletions(-)
  

Comments

Jakub Jelinek Nov. 15, 2022, 2:42 p.m. UTC | #1
On Tue, Nov 15, 2022 at 02:31:19PM +0000, Jonathan Wakely via Gcc-patches wrote:
> Tested x86_64-linux and x86_64-w64-mingw32. Pushed to trunk.
> 
> -- >8 --
> 
> std::format gives linker errors on targets that define __float128 but
> do not support using it with std::to_chars. This improves the handling
> of 128-bit flaoting-point types so they are disabled if unsupportable.
> 
> libstdc++-v3/ChangeLog:
> 
> 	PR libstdc++/107693
> 	* include/std/format (_GLIBCXX_FORMAT_F128): Define to 2 when
> 	basic_format_arg needs to use its _M_f128 member.
> 	(__extended_floating_point, __floating_point): Replace with ...
> 	(__formattable_floating_point): New concept.
> 	* testsuite/std/format/functions/format.cc: Check whether
> 	__float128 is supported. Also test _Float128.

> --- a/libstdc++-v3/include/std/format
> +++ b/libstdc++-v3/include/std/format

> +#elif __FLT128_DIG__ && defined(__GLIBC_PREREQ) // see floating_to_chars.cc

I'd just use here
#elif __FLT128_DIG__ && defined(_GLIBCXX_HAVE_FLOAT128_MATH)
instead.

The reason for defined(__GLIBC_PREREQ) in floating_{to,from}_chars.cc
is that I didn't want to make the ABI of linux libstdc++.so.6 dependent
on whether gcc was built against glibc 2.26+ or older glibc.
So, the symbols exist in libstdc++.so.6 even for older glibcs, but it will
actually only work properly (without losing precision; otherwise it will
just go through long double) if at runtime one uses glibc 2.26+.

But in the headers, defined(_GLIBCXX_HAVE_FLOAT128_MATH) is used everywhere
else (which is true only when compiling against glibc 2.26+).

	Jakub
  
Jonathan Wakely Nov. 15, 2022, 3:45 p.m. UTC | #2
On Tue, 15 Nov 2022 at 14:42, Jakub Jelinek wrote:
>
> On Tue, Nov 15, 2022 at 02:31:19PM +0000, Jonathan Wakely via Gcc-patches wrote:
> > Tested x86_64-linux and x86_64-w64-mingw32. Pushed to trunk.
> >
> > -- >8 --
> >
> > std::format gives linker errors on targets that define __float128 but
> > do not support using it with std::to_chars. This improves the handling
> > of 128-bit flaoting-point types so they are disabled if unsupportable.
> >
> > libstdc++-v3/ChangeLog:
> >
> >       PR libstdc++/107693
> >       * include/std/format (_GLIBCXX_FORMAT_F128): Define to 2 when
> >       basic_format_arg needs to use its _M_f128 member.
> >       (__extended_floating_point, __floating_point): Replace with ...
> >       (__formattable_floating_point): New concept.
> >       * testsuite/std/format/functions/format.cc: Check whether
> >       __float128 is supported. Also test _Float128.
>
> > --- a/libstdc++-v3/include/std/format
> > +++ b/libstdc++-v3/include/std/format
>
> > +#elif __FLT128_DIG__ && defined(__GLIBC_PREREQ) // see floating_to_chars.cc
>
> I'd just use here
> #elif __FLT128_DIG__ && defined(_GLIBCXX_HAVE_FLOAT128_MATH)
> instead.
>
> The reason for defined(__GLIBC_PREREQ) in floating_{to,from}_chars.cc
> is that I didn't want to make the ABI of linux libstdc++.so.6 dependent
> on whether gcc was built against glibc 2.26+ or older glibc.
> So, the symbols exist in libstdc++.so.6 even for older glibcs, but it will
> actually only work properly (without losing precision; otherwise it will
> just go through long double) if at runtime one uses glibc 2.26+.

Yes, and my intention was that std::format would also support
__float128, with the same imprecise behaviour. But could just limit
std::format support to when std::to_chars works properly.

> But in the headers, defined(_GLIBCXX_HAVE_FLOAT128_MATH) is used everywhere
> else (which is true only when compiling against glibc 2.26+).
  

Patch

diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 1796362ceef..c79c8f2ce31 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -1213,40 +1213,35 @@  namespace __format
       _Spec<_CharT> _M_spec{};
     };
 
+  // Decide how 128-bit floating-point types should be formatted (or not).
+  // When supported, the typedef __format::__float128_t is the type that
+  // format arguments should be converted to for storage in basic_format_arg.
+  // Define the macro _GLIBCXX_FORMAT_F128 to say they're supported.
+  // _GLIBCXX_FORMAT_F128=1 means __float128, _Float128 etc. will be formatted
+  // by converting them to long double (or __ieee128 for powerpc64le).
+  // _GLIBCXX_FORMAT_F128=2 means basic_format_arg needs to enable explicit
+  // support for _Float128, rather than formatting it as another type.
+#undef _GLIBCXX_FORMAT_F128
+
 #ifdef _GLIBCXX_LONG_DOUBLE_ALT128_COMPAT
-# define _GLIBCXX_FORMAT_F128 1
+
+  // Format 128-bit floating-point types using __ieee128.
   using __float128_t = __ieee128;
-#elif defined _GLIBCXX_LDOUBLE_IS_IEEE_BINARY128
 # define _GLIBCXX_FORMAT_F128 1
+
+#elif defined _GLIBCXX_LDOUBLE_IS_IEEE_BINARY128
+
+  // Format 128-bit floating-point types using long double.
   using __float128_t = long double;
-#elif __FLT128_DIG__
-# define _GLIBCXX_FORMAT_F128 2
+# define _GLIBCXX_FORMAT_F128 1
+
+#elif __FLT128_DIG__ && defined(__GLIBC_PREREQ) // see floating_to_chars.cc
+
+  // Format 128-bit floating-point types using _Float128.
   using __float128_t = _Float128;
-#else
-# undef _GLIBCXX_FORMAT_F128
-#endif
+# define _GLIBCXX_FORMAT_F128 2
 
-#ifdef _GLIBCXX_LONG_DOUBLE_ALT128_COMPAT
-  template<typename _Tp>
-    concept __extended_floating_point = __is_same(_Tp, _Float128)
-					  || __is_same(_Tp, __ibm128)
-					  || __is_same(_Tp, __ieee128);
-#elif _GLIBCXX_FORMAT_F128
-  template<typename _Tp>
-    concept __extended_floating_point = __is_same(_Tp, __float128_t);
-#else
-  template<typename _Tp>
-    concept __extended_floating_point = false;
-#endif
-
-  template<typename _Tp>
-    concept __floating_point = std::floating_point<_Tp>
-				 || __extended_floating_point<_Tp>;
-
-  using std::to_chars;
-
-#if _GLIBCXX_FORMAT_F128 == 2 \
-  && (__cplusplus == 202002L || !defined(_GLIBCXX_HAVE_FLOAT128_MATH))
+# if __cplusplus == 202002L || !defined(_GLIBCXX_HAVE_FLOAT128_MATH)
   // These overloads exist in the library, but are not declared for C++20.
   // Make them available as std::__format::to_chars.
   to_chars_result
@@ -1260,8 +1255,16 @@  namespace __format
   to_chars_result
   to_chars(char*, char*, _Float128, chars_format, int) noexcept
     __asm("_ZSt8to_charsPcS_DF128_St12chars_formati");
+# endif
 #endif
 
+  using std::to_chars;
+
+  // We can format a floating-point type iff it is usable with to_chars.
+  template<typename _Tp>
+    concept __formattable_float = requires (_Tp __t, char* __p)
+    { __format::to_chars(__p, __p, __t, chars_format::scientific, 6); };
+
   template<__char _CharT>
     struct __formatter_fp
     {
@@ -1984,7 +1987,7 @@  namespace __format
 #endif
 
   /// Format a floating-point value.
-  template<__format::__floating_point _Tp, __format::__char _CharT>
+  template<__format::__formattable_float _Tp, __format::__char _CharT>
     struct formatter<_Tp, _CharT>
     {
       formatter() = default;
@@ -2607,7 +2610,7 @@  namespace __format
 #ifdef _GLIBCXX_LONG_DOUBLE_ALT128_COMPAT
 	__ieee128 _M_f128;
 	__ibm128  _M_ibm128;
-#elif _GLIBCXX_FORMAT_F128
+#elif _GLIBCXX_FORMAT_F128 == 2
 	__float128_t _M_f128;
 #endif
       };
@@ -2663,7 +2666,7 @@  namespace __format
 	  else if constexpr (is_same_v<_Tp, unsigned __int128>)
 	    return __u._M_u128;
 #endif
-#if _GLIBCXX_FORMAT_F128
+#if _GLIBCXX_FORMAT_F128 == 2
 	  else if constexpr (is_same_v<_Tp, __float128_t>)
 	    return __u._M_f128;
 #endif
@@ -2843,13 +2846,15 @@  namespace __format
 	    return type_identity<_Float64>();
 # endif
 #endif
-#ifdef __FLT128_DIG__
+#if _GLIBCXX_FORMAT_F128
+# if __FLT128_DIG__
 	  else if constexpr (is_same_v<_Td, _Float128>)
 	    return type_identity<__format::__float128_t>();
-#endif
-#if _GLIBCXX_USE_FLOAT128
+# endif
+# if __SIZEOF_FLOAT128__
 	  else if constexpr (is_same_v<_Td, __float128>)
 	    return type_identity<__format::__float128_t>();
+# endif
 #endif
 	  else if constexpr (__is_specialization_of<_Td, basic_string_view>)
 	    return type_identity<basic_string_view<_CharT>>();
@@ -2926,7 +2931,7 @@  namespace __format
 	  else if constexpr (is_same_v<_Tp, _Float64>)
 	    return _Arg_f64;
 #endif
-#if _GLIBCXX_FORMAT_F128
+#if _GLIBCXX_FORMAT_F128 == 2
 	  else if constexpr (is_same_v<_Tp, __format::__float128_t>)
 	    return _Arg_f128;
 #endif
@@ -3015,7 +3020,7 @@  namespace __format
 #endif
 	      // TODO _Arg_f16 etc.
 
-#if _GLIBCXX_FORMAT_F128
+#if _GLIBCXX_FORMAT_F128 == 2
 	    case _Arg_f128:
 	      return std::forward<_Visitor>(__vis)(_M_val._M_f128);
 #endif
diff --git a/libstdc++-v3/testsuite/std/format/functions/format.cc b/libstdc++-v3/testsuite/std/format/functions/format.cc
index e9e61694f7d..165ef41b4b3 100644
--- a/libstdc++-v3/testsuite/std/format/functions/format.cc
+++ b/libstdc++-v3/testsuite/std/format/functions/format.cc
@@ -5,6 +5,7 @@ 
 #include <string>
 #include <limits>
 #include <cstdint>
+#include <cstdio>
 #include <testsuite_hooks.h>
 
 void
@@ -289,12 +290,27 @@  test_p1652r1() // printf corner cases in std::format
   VERIFY( s == "3.31" );
 }
 
+template<typename T>
+bool format_float()
+{
+    auto s = std::format("{:#} != {:<+7.3f}", (T)-0.0, (T)0.5);
+    return s == "-0. != +0.500 ";
+}
+
 void
 test_float128()
 {
 #ifdef __SIZEOF_FLOAT128__
-  auto s = std::format("{:#} != {:<+7.3f}", (__float128)-0.0, (__float128)0.5);
-  VERIFY( s == "-0. != +0.500 " );
+  if constexpr (std::formattable<__float128, char>)
+    VERIFY( format_float<__float128>() );
+  else
+    std::puts("Cannot format __float128 on this target");
+#endif
+#if __FLT128_DIG__
+  if constexpr (std::formattable<_Float128, char>)
+    VERIFY( format_float<_Float128>() );
+  else
+    std::puts("Cannot format _Float128 on this target");
 #endif
 }