Implement likely/unlikely intrinsics #26429
Conversation
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
|
👍 for removing some temporaries. |
|
blocked on something? |
|
@seanmonstar not that I know of. |
|
@dotdash can I just squash the commit into the previous one? Splitting the commit and then squashing seems like a fair bit of work that amounts to nothing actually changing. |
Previously, all function calls would store the result to a temporary stack slot. While this would often be optimized out, it prevents the branch hinting from taking effect. Now function calls that return a scalar have their return values, well, returned directly, avoiding the redundant store/load pair that was there before. The return types it does this for are fairly limited due to issues when doing it for other types.
This is essentially the same change as in `intrinsics`. Returns a value or `()` depending on whether or not the type is immediate.
My earlier changes caused the result to be incorrect when both the input and output types were immediate. This has been fixed. I also extended the code to handle int -> ptr and ptr -> int casts as simple cases, since it's fairly easy to do. I changed the one place that was transmuting a function pointer to an integer to just use a cast, it's more clear this way.
This fixes up calling foreign functions so they return the value the function returns, same as other calls. It also refactors the as-datum function-call code to handle more cases and to be in a better place.
This handles block expressions based on the kind of their trailing
expression (if there is one), rather than always using DPS. This is to
allow `unsafe { likely(foo) }` to work as expected, but also reduces the
number of temporary allocas in some cases.
Also adds a codegen test to make sure the intrinsics are being
codegenned in a way that means they are having an effect.
This commit also fixes a small bug I noticed in the translation of
overflow checking that meant the branch hints weren't actually taking
effect.
|
☔ The latest upstream changes (presumably #26479) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@Aatch this all seems pretty sane. I had some small questions and nits, but this can basically be r=pnkfelix'ed post rebase. |
|
@pnkfelix cool, though some recent changes have thrown a spanner in the works that I'm having trouble dealing with nicely. So depending on how different the code ends up being, I may get you to re-review. The change in |
|
@Aatch what's the status? |
|
@gankro blocked on some weird codegen issue that only occurs in a test. I'm actually going to do the immediate-value changes from this PR in another branch then just add the intrinsics on top, as they are trivial, but only functional with said changes. Unfortunately, life-stuff has meant I haven't had much time recently to work on it. I'll close this PR in the meantime. |
Implementation for RFC 1131 (rust-lang/rfcs#1131). Tracking issue #26179.
Most of the work here is actually improvements/changes to code generation to ensure that the branch hints actually take effect. LLVM runs the lowering pass pretty early in the pipeline, and requires the result of the intrinsic call to be used directly in the branch. The existing code would cause the return value of the intrinsic to go into, and then back out of, a stack slot.
The main changes are avoiding the temporary stack slots for functions that return immediate values and a similar change for blocks that have a trailing expression. The change to blocks is to be able to handle this case:
unsafe { likely(foo) }./cc @rust-lang/compiler