-
-
Notifications
You must be signed in to change notification settings - Fork 742
std.math: proper support for 64-bit real in exp() #3285
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
Conversation
// Invalid and div by zero shouldn't be affected. | ||
assert(!f.invalid); | ||
assert(!f.divByZero); | ||
assert(feqrel(x, pair[1]) >= real.mant_dig-13); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
13 lowest mantissa bits allowed to diverge - 13 has been chosen based on an LDC run on Linux x64 with LLVM 3.6 llvm.exp.f80
intrinsic (supposedly following libm
implementation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add as comment in code
Please explain what is meant by "proper" 64 bit support. Does it mean reducing precision from 80 bits for intermediate results to 64 bits? |
I assume this allows targets whose |
I thought the changes are pretty self-explaining, but here's a quick summary: The 1st commit fixes a serious bug in There are other places assuming 80-bit reals, so that's only a start for platforms/targets without x87 support. |
48111c8
to
c85f529
Compare
For subnormal diffs, the old code used the whole ushort containing the exponent bits as exponent, without masking and shifting. This seems to have worked for 80-bit reals so far and the single unit- test was only enabled for x87 reals.
I've added some appropriate comments and tightened the tests again, this time allowing a relative diff of max 2 mantissa bits (3 ulp). That limit is declared as variable so that other backends like LDC may easily relax them. |
Auto-merge toggled on |
std.math: proper support for 64-bit real in exp()
I've just had another look, and I found something wrong, see comment. |
[ 0x1p+80L, real.infinity ], // far overflow | ||
[ real.infinity, real.infinity ], | ||
[-0x1.6p+9L, 0x1.44a3824e5285fp-1016L ], // near underflow | ||
[-0x1.64p+9L, 0x0.06f84920bb2d3p-1022L ], // near underflow - subnormal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this (albeit on 64bit) with -mlong-double-64
and get slightly different result to here. I even double-checked against libm, and it agrees with me.
expected = 0x0.06f84920bb2d300p-1022 (6.05799464199889e-310)
std.math = 0x0.06f84920bb2d400p-1022 (6.05799464199894e-310)
libm = 0x0.06f84920bb2d400p-1022 (6.05799464199894e-310)
Ping on the failing unittest! |
Regression raised https://issues.dlang.org/show_bug.cgi?id=14732 |
No description provided.