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

Int vs float comparisons in Julia #9030

Merged
merged 2 commits into from
Nov 20, 2014

Conversation

simonbyrne
Copy link
Contributor

The first commit (a5de252) implements Int64/UInt64 vs Float32/Float64 comparisons in pure Julia, removing the C++ code from src/intrinisics.cpp. From what I can tell, this should emit identical LLVM code as before for Float64; for Float32, the methods should now work without promotion to Float64.

The second commit (62828b7) makes a small modification to fix #9021. Any comparisons with constant integers (e.g. x > 0) should now be inlined into a single float comparison. Is there anyway I could craft this into a test?

@JeffBezanson
Copy link
Member

Oo, nice code reduction.

@JeffBezanson
Copy link
Member

Maybe we should declare these @inline? @vtjnash what do you think?

@StefanKarpinski
Copy link
Member

This is very nice.

@StefanKarpinski
Copy link
Member

Declaring @inline seems like a good idea since that's effectively what using an intrinsic does.

@JeffBezanson
Copy link
Member

To be more specific, while these functions are inlineable as-is, I worry that a large function with many calls to these could hit a size threshold that prevents further inlining.

@vtjnash
Copy link
Member

vtjnash commented Nov 18, 2014

I generally recommend against forcing inlining. Our current heuristic is pretty generous already, so when you hit that limit you probably really want to stop inlining. One case where that might be false is when you have a large if statement that you know will rarely be triggered and the speed of that code is highly critical. Of the user has redefined some of these internal functions to be large, it is possible that we don't actually want to be inlining here at all. In addition to the size penalty in the AST and resulting code, my understanding is that some of the llvm passes are O(n^2) in the lines of code, so it is best not to make things excessively large

@simonbyrne
Copy link
Contributor Author

What's the call here: to @inline or not to @inline?

@eschnett
Copy link
Contributor

I tend to use @inline only if I know that inlining will expose further optimization opportunities, e.g. evaluating an if condition at compile time, of if the function is very small (just a wrapper around another function call). Otherwise, it's usually best to leave it to the compiler.

How many machine instructions are generated from the comparison functions? If it's fewer than for a function call, then we should use @inline.

JeffBezanson added a commit that referenced this pull request Nov 20, 2014
Int vs float comparisons in Julia
@JeffBezanson JeffBezanson merged commit 7ddcd23 into JuliaLang:master Nov 20, 2014
@JeffBezanson
Copy link
Member

Can always add it later :)

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.

Float comparisons vs 0
5 participants