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

native frexp function #8022

Merged
merged 1 commit into from
Aug 16, 2014
Merged

native frexp function #8022

merged 1 commit into from
Aug 16, 2014

Conversation

simonbyrne
Copy link
Contributor

This implements the frexp function in native julia. It is roughly 10x faster than calling the libm function (as it avoids passing by reference), and doesn't require keeping a global array around (so no threading issues).

@JeffBezanson
Copy link
Member

💯 !!

@johnmyleswhite
Copy link
Member

@simonbyrne, world famous floating-point badass.

JeffBezanson added a commit that referenced this pull request Aug 16, 2014
@JeffBezanson JeffBezanson merged commit 3ddb0d7 into JuliaLang:master Aug 16, 2014
@quinnj
Copy link
Member

quinnj commented Aug 17, 2014

This is awesome! I actually started a version grisu based on frexp that aimed to avoid a whole bunch of code from the native library, but the performance gains were minor. I'll have to resurrect that since it was BigFloat compatible as well. Thanks @simonbyrne!

@tkelman
Copy link
Contributor

tkelman commented Aug 21, 2014

Apparently these are the first hex floats in Base. MSVC can't handle these yet, so #7761 can't bootstrap any more after rebasing this in. Can we just spell these out?

@simonbyrne simonbyrne deleted the frexp branch August 21, 2014 07:25
@simonbyrne
Copy link
Contributor Author

@tkelman Sure. Maybe keep it in as a comment though, so it's clear why it multiplies by 18014398509481984.0.

@tkelman
Copy link
Contributor

tkelman commented Aug 21, 2014

Like this? tkelman@a72ccd9

@simonbyrne
Copy link
Contributor Author

Yes, that's what I had in mind.

@ivarne
Copy link
Member

ivarne commented Aug 21, 2014

I think something like

# x *= 0x1p54 # normalise significand
x *= 1.8014398509481984e16 # normalise significand

is clearer, but that seems like a matter of taste.

@StefanKarpinski
Copy link
Member

The hex version is infinitely clearer IMO, but that might just be me.

@ivarne
Copy link
Member

ivarne commented Aug 21, 2014

I also prefer hex when it contains lots of 0 and F. I meant that it was clearer to have the full hex version as a comment above, not in a comment on the side.

@tknopp
Copy link
Contributor

tknopp commented Aug 21, 2014

Although the hex float version is of course clearer, I think it should be ok to make a comment like @ivarne proposed. It makes @tkelman life a lot simpler for now and we should really appreciate that he moves on the MSVC support.

If there are various places like this we would have to rethink how to proceed, but a single hex-float should get a gentle exception.

By the way, #6349 is the issue that MSVC does not support hex parsing. Maybe this could be added to the comment.

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

Successfully merging this pull request may close these issues.

8 participants