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

[Jonathan Wakely <jwakely@redhat.com>]

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

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