Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<xlocnum>: Incorrect rounding when parsing long hexadecimal floating point numbers just above midpoints #3375

Closed
statementreply opened this issue Feb 1, 2023 · 3 comments · Fixed by #3364
Labels
bug Something isn't working fixed Something works now, yay!

Comments

@statementreply
Copy link
Contributor

statementreply commented Feb 1, 2023

Describe the bug
The hexadecimal code path of num_get::do_get parses the initial 768 significant digits and discards the remaining. When the remaining digits could affect rounding, it returns an incorrect result.

Command-line test case

D:\Temp>type test.cpp
#include <iostream>
#include <sstream>
#include <string>
using namespace std;

int main() {
    istringstream stream("0x1.00000000000008" + string(1000, '0') + "1p+0");
    stream >> hexfloat; // GH-1259
    double x;
    if (stream >> x) {
        cout << "actual:   " << hexfloat << x << "\n";
    } else {
        cout << "actual:   (failure)\n";
    }
    cout << "expected: 0x1.0000000000001p+0\n";
    return 0;
}
D:\Temp>cl /EHsc /W4 test.cpp
用于 x64 的 Microsoft (R) C/C++ 优化编译器 19.35.32213 版
版权所有(C) Microsoft Corporation。保留所有权利。

test.cpp
Microsoft (R) Incremental Linker Version 14.35.32213.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:test.exe
test.obj

D:\Temp>.\test
actual:   0x1.0000000000000p+0
expected: 0x1.0000000000001p+0

STL version

Microsoft Visual Studio Community 2022 (64 位) - Preview
版本 17.5.0 Preview 5.0
@frederick-vs-ja
Copy link
Contributor

I think ideally we should determine whether the reperesentation without the "sticky" part is exactly a midpoint. If not so, the "sticky" part should be discarded, otherwise, it should cause "adding one". But such determination might be a heavy work, and lightweight alternatives seem possible.

@statementreply
Copy link
Contributor Author

statementreply commented Feb 2, 2023

When the radix of the number system is even and >= 4, to round a number for rounding again to less significant digits later, possibly with different rounding modes, one of the possible algorithms is to add one if and only if sticky && (last_digit == 0 || last_digit == radix/2) (no carrying). For decimal numbers, this means increasing 0 to 1 and 5 to 6, but leaving [12346789] unchanged when discarding trailing digits.

num_get::do_get is rounding the decimal or hexadecimal number for conversion to binary. While it's not exactly the same problem, given the radix (10->2 and 16->2) and precision involved, the algorithm above should also work.

@frederick-vs-ja
Copy link
Contributor

one of the possible algorithms is to add one if and only if sticky && (last_digit == 0 || last_digit == radix/2) (no carrying)

Thank you! I do want to believe this is correct and thus we can drop the carrying logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Something works now, yay!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants