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

Optimize for LLVM 17 #11

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Optimize for LLVM 17 #11

merged 1 commit into from
Feb 21, 2024

Conversation

chriselrod
Copy link
Collaborator

@chriselrod chriselrod commented Jan 27, 2024

Julia master is on LLVM 16, so no real rush here.
The !unpredictable should stop the cmov converter from converting the cmov into a branch.

This PR is a follow up to #10
Basically, that PR shows that disabling the cmov-converter yields a huge performance boost here. The cmov-converted converts branch-free cmovs into branches, following the logic that if branch prediction rates are good, the branches are faster.

In real world use, binary search's branches are totally unpredictable: for each split, the chance is 50/50 whether the answer is in the upper or lower half.

Note, however, that if you have a microbenchmark where you're searching for the same number in the same vector over and over again, the branch predictor can probably memorize that exact sequence, so you'll need to do much more rigorous benchmark to replicate real-world use, where you're not searching for the same number in the same vector every time (if you are doing that, stop! Calculate it once, and save the answer!).

Because of that, we really do want cmov, which the README benchmarks added in #10 demonstrate: we go from roughly 85 microseconds to just 26 microseconds via using cmov!

This PR adds unpredictable metadata to the select statement.

Using __builtin_unpredictable in Clang 17 or newer adds this metadata to the LLVM IR, and also causes cmovs to be preserved. So hopefully adding the metadata like this PR does will work for Julia, once we upgrade to LLVM 17.

Converting cmovs to branches is a bad idea only when branches are hard to predict, so telling LLVM the branch/select is hard to predict is the ideal solution.

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (591b366) 0.00% compared to head (38b267e) 0.00%.
Report is 6 commits behind head on main.

Files Patch % Lines
src/FindFirstFunctions.jl 0.00% 3 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main     #11   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          1       1           
  Lines         43      45    +2     
=====================================
- Misses        43      45    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chriselrod
Copy link
Collaborator Author

Need to try this PR on Julia master following JuliaLang/julia#53070 merging.

@chriselrod
Copy link
Collaborator Author

Need to try this PR on Julia master following JuliaLang/julia#53070 merging.

I just checked out the PR and tried it. Works as intended.

@chriselrod chriselrod merged commit 9ce565c into main Feb 21, 2024
8 of 9 checks passed
@chriselrod chriselrod deleted the llvm17prep branch February 21, 2024 19:47
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.

1 participant