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

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

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

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