Allow inlining for Atomic*::from_ptr by tgross35 · Pull Request #138804 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation5 Commits1 Checks6 Files changed
Conversation
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
[ Show hidden characters]({{ revealButtonHref }})
Currently this cannot be inlined, which among other things means it can't be used in compiler-builtins 1. These are trivial functions that should be inlineable, so add #[inline].
r? @RalfJung
Currently this cannot be inlined, which among other things means it
can't be used in compiler-builtins [1]. These are trivial functions
that should be inlineable, so add #[inline].
[1]: rust-lang/compiler-builtins#790 (comment)
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
labels
📌 Commit eb2a2f8 has been approved by RalfJung
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Currently this cannot be inlined
I believe this statement to be wrong. If you call this function in an optimized build, you will see that it is currently inlined reliably: https://godbolt.org/z/YGE5T1Ghz
I am bringing this up in an attempt to be helpful; many contributors abuse the term "inline" to mean either the compiler optimization or LocalCopy instantiation without being clear which, and those concepts are almost entirely orthogonal in theory, but they are conflated (to our detriment) by the current compiler implementation.
Yeah it's the usual issue that "inline" is often used to mean "MIR-to-LLVM translation happens in the reverse dependencies of this crate". That's the property we actually need here, and #[inline] is how we get it. But LLVM can inline more things than that ofc. (The MIR inliner cannot, AFAIK?)
Or is that not right? Is the PR not actually going to help for the problem we're trying to solve?
(The MIR inliner cannot, AFAIK?)
The MIR inliner can inline anything that is #[inline], generic, or cross_crate_inlinable, which already includes the function in question.
Is the PR not actually going to help for the problem we're trying to solve?
This PR will fix the compile error that this was created in response to, because with this PR Atomic*::from_ptr will get LocalCopy codegen in all builds, not just optimized builds.
bors added a commit to rust-lang-ci/rust that referenced this pull request
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#138804 - tgross35:atomic-from-ptr-inline, r=RalfJung
Allow inlining for Atomic*::from_ptr
Currently this cannot be inlined, which among other things means it can't be used in compiler-builtins [1]. These are trivial functions that should be inlineable, so add #[inline].
[1]: rust-lang/compiler-builtins#790 (comment)
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request
…=RalfJung
Allow inlining for Atomic*::from_ptr
Currently this cannot be inlined, which among other things means it can't be used in compiler-builtins [1]. These are trivial functions that should be inlineable, so add #[inline].
[1]: rust-lang/compiler-builtins#790 (comment)
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the library team, which will review and decide on the PR/issue.