-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add native julia fmod #47501
Add native julia fmod #47501
Conversation
Co-authored-by: Alex Arslan <ararslan@comcast.net>
Co-authored-by: Alex Arslan <ararslan@comcast.net>
I think this looks ready! I'll merge in a few days if no one objects. |
Co-authored-by: Alex Arslan <ararslan@comcast.net>
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.
Some extremely pedantic comments, some may be useful.
Co-authored-by: Alex Arslan <ararslan@comcast.net>
Co-authored-by: Alex Arslan <ararslan@comcast.net>
Co-authored-by: Alex Arslan <ararslan@comcast.net>
Co-authored-by: Alex Arslan <ararslan@comcast.net>
Do we want this in 1.9? |
Imo yes, it fixes a perf regression |
should we merge this now? |
I believe so, if we find a way to fix the effects of the bitwise operations without breaking other things we can change here. |
Merge? |
* Add native julia rem Co-authored-by: Alex Arslan <ararslan@comcast.net> (cherry picked from commit cf5ae03)
Should fix #46467
Based on LLVMs libc implementation. About 5x faster than openlibm and 10% slower than glibc.
Tested on all values of Float16 and a large amount of Float32 and 64.
The amount of tests might be a bit much, since it's the exact same codepath independent of type. But maybe I should add some explicit Float16 tests. That function there tests all possible combinations but it's a bit too long.