[Bug libstdc++/107468] New: std::from_chars doesn't always round to nearest

[Bug libstdc++/107468] New: std::from_chars doesn't always round to nearest

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107468

            Bug ID: 107468
           Summary: std::from_chars doesn't always round to nearest
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
  Target Milestone: ---

#include <cfenv>
#include <charconv>
#include <iostream>
#include <string_view>

int
main()
{
  float f;
  char buf[] = "3.355447e+07";
  fesetround(FE_DOWNWARD);
  auto [ptr, ec] = std::from_chars(buf, buf + sizeof (buf) - 1, f,
std::chars_format::scientific);
  char buf2[128];
  auto [ptr2, ec2] = std::to_chars(buf2, buf2 + 128, f,
std::chars_format::fixed);
  std::cout << std::string_view(buf2, ptr2 - buf2) << std::endl;
}

33554470 isn't representable in IEEE single, only 33554468 and 33554472 are,
the former is 0x1.000012p+25 and the latter is 0x1.000014p+25 and the latter
has
0 as the least significant digit.  So, round to even should be 33554472 and
that is what happens without the fesetround(FE_DOWNWARD).

When fast_float isn't used, floating_from_chars.cc temporarily overrides the
rounding to nearest, but when it is used, it doesn't.
In most cases it is fine, because fast_float usually computes the mantissa and
exponent in integral code.  But it has two exceptions for this I think.
One is the fast path:
  // Next is Clinger's fast path.
  if (binary_format<T>::min_exponent_fast_path() <= pns.exponent &&
pns.exponent <= binary_format<T>::max_exponent_fast_path() && pns.mantissa
<=binary_format<T>::max_mantissa_fast_p
ath() && !pns.too_many_digits) {
    value = T(pns.mantissa);
    if (pns.exponent < 0) { value = value /
binary_format<T>::exact_power_of_ten(-pns.exponent); }
    else { value = value * binary_format<T>::exact_power_of_ten(pns.exponent);
}
    if (pns.negative) { value = -value; }
    return answer;
  }
I'm afraid we need to temporarily override rounding to nearest around the
multiplications in there.  And the other one is:
  T b;
  to_float(false, am_b, b);
  adjusted_mantissa theor = to_extended_halfway(b);
where I think we don't really need it but I don't understand the point of
jumping through the floating point format, it encodes am_b into an integer,
memcpys it to float/double (in to_float), then immediately memcpys it back to
an integer (in to_extended called from to_extended_halfway) and then unpacks it
with some adjustments.

[Bug libstdc++/107468] std::from_chars doesn't always round to nearest

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107468

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-11-01

[Bug libstdc++/107468] std::from_chars doesn't always round to nearest

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107468

--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Might be good to let upstream know about these issues

[Bug libstdc++/107468] std::from_chars doesn't always round to nearest

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107468

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:cb0ceeaee9e041aaac3edd089b07b439621d0f29

commit r13-3755-gcb0ceeaee9e041aaac3edd089b07b439621d0f29
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Nov 7 15:17:21 2022 +0100

    libstdc++: Update from latest fast_float [PR107468]

    The following patch updates from fast_float trunk.  That way
    it grabs two of the 4 LOCAL_PATCHES, some smaller tweaks, to_extended
    cleanups and most importantly fix for the incorrect rounding case,
    PR107468 aka https://github.com/fastfloat/fast_float/issues/149
    Using std::fegetround showed in benchmarks too slow, so instead of
    doing that the patch limits the fast path where it uses floating
    point multiplication rather than integral to cases where we can
    prove there will be no rounding (the multiplication will be exact, not
    just that the two multiplication or division operation arguments are
    exactly representable).

    2022-11-07  Jakub Jelinek  <jakub@redhat.com>

            PR libstdc++/107468
            * src/c++17/fast_float/MERGE: Adjust for merge from upstream.
            * src/c++17/fast_float/LOCAL_PATCHES: Remove commits that were
            upstreamed.
            * src/c++17/fast_float/README.md: Merge from fast_float
            662497742fea7055f0e0ee27e5a7ddc382c2c38e commit.
            * src/c++17/fast_float/fast_float.h: Likewise.
            * testsuite/20_util/from_chars/pr107468.cc: New test.

[Bug libstdc++/107468] std::from_chars doesn't always round to nearest

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107468

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed now on the trunk.
Dunno what to do for GCC 12, nothing, or do a limited backport (just the
Clinger's fast path related changes), something else?

[Bug libstdc++/107468] std::from_chars doesn't always round to nearest

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107468

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The limited backport would be good, I don't think we need to backport the trunk
sync though.

[Bug libstdc++/107468] std::from_chars doesn't always round to nearest

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107468

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:ec73b55c75baa16c1cf7482fa65928a8d45598d4

commit r13-4285-gec73b55c75baa16c1cf7482fa65928a8d45598d4
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Nov 24 10:38:42 2022 +0100

    libstdc++: Another merge from fast_float upstream [PR107468]

    Upstream fast_float came up with a cheaper test for
    fegetround () == FE_TONEAREST using one float addition, one subtraction
    and one comparison.  If we know we are rounding to nearest, we can use
    fast path in more cases as before.
    The following patch merges those changes into libstdc++.

    2022-11-24  Jakub Jelinek  <jakub@redhat.com>

            PR libstdc++/107468
            * src/c++17/fast_float/MERGE: Adjust for merge from upstream.
            * src/c++17/fast_float/fast_float.h: Merge from fast_float
            2ef9abbcf6a11958b6fa685a89d0150022e82e78 commit.

[Bug libstdc++/107468] std::from_chars doesn't always round to nearest

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107468

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:0061c84d2d85ca4c2ceca6116937f2d1fab6e10a

commit r12-9154-g0061c84d2d85ca4c2ceca6116937f2d1fab6e10a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Nov 7 15:17:21 2022 +0100

    libstdc++: Update from latest fast_float [PR107468]

    The following patch partially updates from fast_float trunk.
    That way it gets fix for the incorrect rounding case,
    PR107468 aka https://github.com/fastfloat/fast_float/issues/149
    Using std::fegetround showed in benchmarks too slow, so instead of
    doing that the patch limits the fast path where it uses floating
    point multiplication rather than integral to cases where we can
    prove there will be no rounding (the multiplication will be exact, not
    just that the two multiplication or division operation arguments are
    exactly representable).

    2022-11-07  Jakub Jelinek  <jakub@redhat.com>

            PR libstdc++/107468
            * src/c++17/fast_float/fast_float.h: Partial merge from fast_float
            662497742fea7055f0e0ee27e5a7ddc382c2c38e commit.
            * testsuite/20_util/from_chars/pr107468.cc: New test.

    (cherry picked from commit cb0ceeaee9e041aaac3edd089b07b439621d0f29)