diff mbox series

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

Message ID 20230531122222.4116868-1-jwakely@redhat.com
State Accepted
Headers
Series [committed] libstdc++: Fix preprocessor conditions for std::from_chars [PR109921] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Jonathan Wakely May 31, 2023, 12:22 p.m. UTC 
  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.
---
 libstdc++-v3/src/c++17/floating_from_chars.cc | 20 ++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

Comments

Christophe Lyon June 1, 2023, 9:29 a.m. UTC | #1 
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
>
>
Jonathan Wakely June 1, 2023, 11:05 a.m. UTC | #2 
On Thu, 1 Jun 2023 at 10:30, Christophe Lyon via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> 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

This is now PR 110077.


>
> 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
> >
> >
Jonathan Wakely June 29, 2023, 12:49 p.m. UTC | #3 
On Thu, 1 Jun 2023 at 12:05, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:

> On Thu, 1 Jun 2023 at 10:30, Christophe Lyon via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > 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
>
> This is now PR 110077.
>

Hi Christophe,

Is this fixed for aarch64 now? I think it should be.



>
>
> >
> > 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
> > >
> > >
>
>
Christophe Lyon June 29, 2023, 1:25 p.m. UTC | #4 
On Thu, 29 Jun 2023 at 14:50, Jonathan Wakely <jwakely@redhat.com> wrote:

>
>
> On Thu, 1 Jun 2023 at 12:05, Jonathan Wakely <jwakely.gcc@gmail.com>
> wrote:
>
>> On Thu, 1 Jun 2023 at 10:30, Christophe Lyon via Libstdc++
>> <libstdc++@gcc.gnu.org> wrote:
>> >
>> > 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
>>
>> This is now PR 110077.
>>
>
> Hi Christophe,
>
> Is this fixed for aarch64 now? I think it should be.
>
> Hi Jonathan,

Yes, I know see
PASS: libstdc++-abi/abi_check

Thanks for fixing this.

Christophe



>
>>
>>
>> >
>> > 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
>> > >
>> > >
>>
>>
diff mbox series

Patch

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