-
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 all commits
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,56 @@ | ||
- Feature Name: expect_intrinsic | ||
- Start Date: 2015-05-20 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
|
||
Provide a pair of intrinsic functions 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 a pair of intrinsics `likely` and `unlikely`, both with signature `fn(bool) -> bool` | ||
which hint at the probability of the passed value being true or false. Specifically, `likely` hints | ||
to the compiler that the passed value is likely to be true, and `unlikely` hints that it is likely | ||
to be false. Both functions simply return the value they are passed. | ||
|
||
The primary reason for this design is that it reflects common usage of this general feature in many | ||
C and C++ projects, most of which define simple `LIKELY` and `UNLIKELY` macros around the gcc | ||
`__builtin_expect` intrinsic. It also provides the most flexibility, allowing branches on any | ||
condition to be hinted at, even if the process that produced the branched-upon value is | ||
complex. For why an equivalent to `__builtin_expect` is not being exposed, see the Alternatives | ||
section. | ||
|
||
There are no observable changes in behaviour from use of these intrinsics. It is valid to implement | ||
these intrinsics simply as the identity function. Though it is expected that the intrinsics provide | ||
information to the optimizer, that information is not guaranteed to change the decisions the | ||
optimiser makes. | ||
|
||
# Drawbacks | ||
|
||
The intrinsics cannot be used to hint at arms in `match` expressions. However, given that hints | ||
would need to be variants, a simple intrinsic would not be sufficient for those purposes. | ||
|
||
# Alternatives | ||
|
||
Expose an `expect` intrinsic. This is what gcc/clang does with `__builtin_expect`. However there is | ||
a restriction that the second argument be a constant value, a requirement that is not easily | ||
expressible in Rust code. The split into `likely` and `unlikely` intrinsics reflects the strategy | ||
we have used for similar restrictions like the ordering constraint of the atomic intrinsics. | ||
|
||
# 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.