Re: [committed] libstdc++: Fix preprocessor conditions for std::from_chars [PR109921]

[Christophe Lyon <christophe.lyon@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 5844 bytes --]

Hi,


On Wed, 31 May 2023 at 14:25, Jonathan Wakely via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

> Tested powerpc64le-linux. Pushed to trunk.
>
> -- >8 --
>
> We use the from_chars_strtod function with __strtof128 to read a
> _Float128 value, but from_chars_strtod is not defined unless uselocale
> is available. This can lead to compilation failures for some targets,
> because we try to define the _Flaot128 overload in terms of a
> non-existing from_chars_strtod function.
>
> Only try to use __strtof128 if uselocale is available, otherwise
> fallback to the long double overload of std::from_chars (which might
> fallback to the double overload, which should use fast_float).
>
> This ensures we always define the full set of overloads, even if they
> are not always accurate for all values of the wider types.
>
> libstdc++-v3/ChangeLog:
>
>         PR libstdc++/109921
>         * src/c++17/floating_from_chars.cc (USE_STRTOF128_FOR_FROM_CHARS):
>         Only define when USE_STRTOD_FOR_FROM_CHARS is also defined.
>         (USE_STRTOD_FOR_FROM_CHARS): Do not undefine when long double is
>         binary64.
>         (from_chars(const char*, const char*, double&, chars_format)):
>         Check __LDBL_MANT_DIG__ == __DBL_MANT_DIG__ here.
>         (from_chars(const char*, const char*, _Float128&, chars_format))
>         Only use from_chars_strtod when USE_STRTOD_FOR_FROM_CHARS is
>         defined, otherwise parse a long double and convert to _Float128.
>


This is causing a regression on aarch64:
 FAIL: libstdc++-abi/abi_check

The log says:

3 added symbols
0
_ZNSt7__cxx1112basic_stringIwSt11char_traitsIwESaIwEE11_S_allocateERS3_m
std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>,
std::allocator<wchar_t> >::_S_allocate(std::allocator<wchar_t>&, unsigned
long)
version status: compatible
GLIBCXX_3.4.32
type: function
status: added

1
_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE11_S_allocateERS3_m
std::__cxx11::basic_string<char, std::char_traits<char>,
std::allocator<char> >::_S_allocate(std::allocator<char>&, unsigned long)
version status: compatible
GLIBCXX_3.4.32
type: function
status: added

2
_ZSt10from_charsPKcS0_RDF128_St12chars_format
std::from_chars(char const*, char const*, _Float128&, std::chars_format)
version status: incompatible
GLIBCXX_3.4.31
type: function
status: added


2 undesignated symbols
0
_ZSt11__once_call
std::__once_call
version status: compatible
GLIBCXX_3.4.11
type: tls
type size: 8
status: undesignated

1
_ZSt15__once_callable
std::__once_callable
version status: compatible
GLIBCXX_3.4.11
type: tls
type size: 8
status: undesignated


1 incompatible symbols
0
_ZSt10from_charsPKcS0_RDF128_St12chars_format
std::from_chars(char const*, char const*, _Float128&, std::chars_format)
version status: incompatible
GLIBCXX_3.4.31
type: function
status: added



                ==== libstdc++-v3 check-abi Summary ====

# of added symbols:              3
# of missing symbols:            0
# of undesignated symbols:       2
# of incompatible symbols:       1


Can you have a look?

Thanks,
Christophe

---
>  libstdc++-v3/src/c++17/floating_from_chars.cc | 20 ++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc
> b/libstdc++-v3/src/c++17/floating_from_chars.cc
> index ebd428d5be3..eea878072b0 100644
> --- a/libstdc++-v3/src/c++17/floating_from_chars.cc
> +++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
> @@ -64,7 +64,7 @@
>  // strtold for __ieee128
>  extern "C" __ieee128 __strtoieee128(const char*, char**);
>  #elif __FLT128_MANT_DIG__ == 113 && __LDBL_MANT_DIG__ != 113 \
> -      && defined(__GLIBC_PREREQ)
> +      && defined(__GLIBC_PREREQ) && defined(USE_STRTOD_FOR_FROM_CHARS)
>  #define USE_STRTOF128_FOR_FROM_CHARS 1
>  extern "C" _Float128 __strtof128(const char*, char**)
>    __asm ("strtof128")
> @@ -77,10 +77,6 @@ extern "C" _Float128 __strtof128(const char*, char**)
>  #if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64 \
>      && __SIZE_WIDTH__ >= 32
>  # define USE_LIB_FAST_FLOAT 1
> -# if __LDBL_MANT_DIG__ == __DBL_MANT_DIG__
> -// No need to use strtold.
> -#  undef USE_STRTOD_FOR_FROM_CHARS
> -# endif
>  #endif
>
>  #if USE_LIB_FAST_FLOAT
> @@ -1261,7 +1257,7 @@ from_chars_result
>  from_chars(const char* first, const char* last, long double& value,
>            chars_format fmt) noexcept
>  {
> -#if ! USE_STRTOD_FOR_FROM_CHARS
> +#if __LDBL_MANT_DIG__ == __DBL_MANT_DIG__ || !defined
> USE_STRTOD_FOR_FROM_CHARS
>    // Either long double is the same as double, or we can't use strtold.
>    // In the latter case, this might give an incorrect result (e.g. values
>    // out of range of double give an error, even if they fit in long
> double).
> @@ -1329,13 +1325,23 @@
> _ZSt10from_charsPKcS0_RDF128_St12chars_format(const char* first,
>                                               __ieee128& value,
>                                               chars_format fmt) noexcept
>  __attribute__((alias
> ("_ZSt10from_charsPKcS0_Ru9__ieee128St12chars_format")));
> -#elif defined(USE_STRTOF128_FOR_FROM_CHARS)
> +#else
>  from_chars_result
>  from_chars(const char* first, const char* last, _Float128& value,
>            chars_format fmt) noexcept
>  {
> +#ifdef USE_STRTOF128_FOR_FROM_CHARS
>    // fast_float doesn't support IEEE binary128 format, but we can use
> strtold.
>    return from_chars_strtod(first, last, value, fmt);
> +#else
> +  // Read a long double. This might give an incorrect result (e.g. values
> +  // out of range of long double give an error, even if they fit in
> _Float128).
> +  long double ldbl_val;
> +  auto res = std::from_chars(first, last, ldbl_val, fmt);
> +  if (res.ec == errc{})
> +    value = ldbl_val;
> +  return res;
> +#endif
>  }
>  #endif
>
> --
> 2.40.1
>
>