-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add an expect intrinsic #1131
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
Merged
Merged
Add an expect intrinsic #1131
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
- Feature Name: expect_intrinsic | ||
- Start Date: 2015-05-20 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
|
||
Provide an intrinsic function for hinting the likelyhood of branches being taken. | ||
|
||
# Motivation | ||
|
||
Branch prediction can have significant effects on the running time of some code. Especially tight | ||
inner loops which may be run millions of times. While in general programmers aren't able to | ||
effectively provide hints to the compiler, there are cases where the likelyhood of some branch | ||
being taken can be known. | ||
|
||
For example: in arbitrary-precision arithmetic, operations are often performed in a base that is | ||
equal to `2^word_size`. The most basic division algorithm, "Schoolbook Division", has a step that | ||
will be taken in `2/B` cases (where `B` is the base the numbers are in), given random input. On a | ||
32-bit processor that is approximately one in two billion cases, for 64-bit it's one in 18 | ||
quintillion cases. | ||
|
||
# Detailed design | ||
|
||
Implement an `expect` intrinsic with the signature: `fn(bool, bool) -> bool`. The first argument is | ||
the condition being tested, the second argument is the expected result. The return value is the | ||
same as the first argument, meaning that `if foo == bar { .. }` can be simply replaced with | ||
`if expect(foo == bar, false) { .. }`. | ||
|
||
The expected value is required to be a constant value. | ||
|
||
# Drawbacks | ||
|
||
The second argument is required to be a constant value, which can't be easily expressed. | ||
|
||
# Alternatives | ||
|
||
Provide a pair of intrinsics `likely` and `unlikely`, these are the same as `expect` just with | ||
`true` and `false` substituted in for the expected value, respectively. | ||
|
||
# Unresolved questions | ||
|
||
None. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I assume you mean to use something like
llvm.expect.i8
and converting the booleans to i8s, but this could be included in the Detailed Design.Likewise, when I tried this myself by creating
extern fn expect_i8
etc functions, linking against llvm, I couldn't find the optimizations in the asm. It'd be nice to see the improvements that such a function could possibly bring in this RFC.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.
@seanmonstar actually, you can use
llvm.expect.i1
. As for improvements, that's hard to show since benchmark code tends to warm up the prediction table pretty quickly. It also isn't always visible in the asm if the code was already structured such that the blocks were ordered appropriately.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.
Right, but we don't have an
i1
, the smallest in rust isi8
.True about the benchmarks. I meant the compiled asm having the jump altered, before and after using
expect
.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.
@seanmonstar actually,
bool
s are now treated asi1
s when generating LLVM IR.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.
Ah, very cool. That confusion may be even more reason to include the implementation details.
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.
Oh, and I left it out of the detailed design because I don't think the precise implementation details for a specific backend are important. Just the semantics of the intrinsic.