-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Implement rem(float, float, RoundNearest) in Julia
#42380
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
|
Out if curiosity, any performance numbers to share? |
Good idea! Seems to do pretty well (first here is this implementation): |
|
Any idea why this is faster? Does a |
None whatsoever |
|
This is perhaps one case where it would be fine to use the llvm intrinsic (since all implementations should return the same value), but having our own probably makes sense. Thanks @ararslan! |
|
I was looking into doing this directly with LLVM initially but AFAICT the available LLVM intrinsics don't have the |
|
Only |
I have a local implementation of that one too, will be doing a bit more testing then making a PR. 🙂 |
|
Don't we still have |
They don't? What do they use. |
|
It's not specified in the LLVM documentation but based on experimentation LLVM's |
|
Oh right, LLVM doesn't offer a |
|
you may need some sort of copyright acknowledgement? |
9d4df8f to
88a962b
Compare
|
Needs tests? |
|
There are existing tests for these methods which I assumed were sufficient but I can always add more. |
|
Test failures:
All of these platforms successfully ran the numbers tests, which is where |
|
It would be nice if we could get rid of some of the bit twiddling and use plain Julia functions ( |
|
I don't quite understand why it does |
|
Ah, because of the ties-to-even. |
One of the few remaining uses of openlibm is implementing `rem` with `RoundNearest` for `Float32` and `Float64`. This commit translates the msun libm implementations of `__ieee754_remainder` and `___ieee754_remainderf` to Julia to avoid relying on openlibm.
88a962b to
bc13ba2
Compare
|
Made some changes, current state:
Summary of test failures:
|
Also make sure that signed zero is checked in the test
|
Simon's feedback was incorporated and the only CI failure is FreeBSD, which is a known issue, so I'll go ahead and merge. Thanks! |
|
Ended up very clean and nice! |
One of the few remaining uses of openlibm is implementing `rem` with `RoundNearest` for `Float32` and `Float64`. This commit translates the msun libm implementations of `__ieee754_remainder` and `___ieee754_remainderf` to Julia to avoid relying on openlibm.
One of the few remaining uses of openlibm is implementing `rem` with `RoundNearest` for `Float32` and `Float64`. This commit translates the msun libm implementations of `__ieee754_remainder` and `___ieee754_remainderf` to Julia to avoid relying on openlibm.
One of the few remaining uses of openlibm is implementing
remwithRoundNearestforFloat32andFloat64. This commit translates the msun libm implementations ofremainderandremainderfto Julia to avoid relying on openlibm or the local system libm.The implementations of the
Float32andFloat64methods are quite similar and could be combined with a bit more work. I'd be happy to do that but would need some guidance for some of the less obvious magic numbers.Checks off the
rembox in #26434.