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

s2d vs MSVC/Clang #157

Closed
ejb77 opened this issue May 13, 2020 · 3 comments
Closed

s2d vs MSVC/Clang #157

ejb77 opened this issue May 13, 2020 · 3 comments

Comments

@ejb77
Copy link

ejb77 commented May 13, 2020

i noticed that s2d disagrees with MSVC & CLang. Is this expected?

int main()
{
double d;
double e = 1.2999999999999999E+154;
s2d("1.2999999999999999E+154", &d);
assert(d==e); // assert occurs.
}

E

@ejb77
Copy link
Author

ejb77 commented May 14, 2020

d2s returns a number that is slightly different to what MSVC and CLang think they should be. d2s returns 1.2999999999999998e+154, the most insignificant bit is different.

(unsigned long long)&d,x 0x5fef06d5a4bf631e vs
(unsigned long long)&e,x 0x5fef06d5a4bf631f

@abolz
Copy link
Contributor

abolz commented May 15, 2020

This is due to an invalid shift value in multipleOfPowerOf2. I think a fix would be to change

ryu/ryu/s2d.c

Line 187 in 4cd47db

trailingZeros = e2 < e10 || multipleOfPowerOf2(m10, e2 - e10);

to

trailingZeros = e2 < e10 || (e2 - e10 < 64 && multipleOfPowerOf2(m10, e2 - e10));

and possibly adding an assert to multipleOfPowerOf2 to check the preconditions.

@ejb77
Copy link
Author

ejb77 commented May 21, 2020

thanks. this works great

@ejb77 ejb77 closed this as completed May 21, 2020
abolz added a commit to abolz/ryu that referenced this issue May 21, 2020
multipleOfPowerOf2 only works for exponents < 64.

In this case here we have 0 < m10 < 2^64, so m10 can never
be a multiple of 2^p if p >= 64.
ulfjack pushed a commit that referenced this issue May 22, 2020
multipleOfPowerOf2 only works for exponents < 64.

In this case here we have 0 < m10 < 2^64, so m10 can never
be a multiple of 2^p if p >= 64.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants