Closed crlf0710 closed 4 years ago
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
Procedurally, I think this needs some tweaks. From a lang perspective, #[inline]
is just a hint (an implementation can completely ignore it) and thus even when used has no impact on observable behaviour.
I would thus say that "we should run a MIR inliner" is entirely a compiler question -- whether or not you do, vs getting LLVM to do it, should be semantically invisible and thus not need lang
approval.
So I think the only thing left under lang preview in the proposal is whether to lint on #[inline(always)]
that doesn't? (Or potentially stronger alternatives, like some attribute that would guarantee a particular semantic by giving hard errors if it doesn't happen.) If so, I would suggest focusing down to that.
A specific thing to clarify: "N/A -> heuristically determine whether to inline or not, left to compiler internals" appears to have been the case at least as far back as Rust 1.4.0. So it's unclear to me what this issue is proposing to change in that specific bullet.
@scottmcm Thanks! I've got the text updated according to the initial feedback here and on zulip.
Update from 2020-09-21:
Some summary of Zulip thread:
#[inline(required)]
might be a clearer way to force inlining, since it's a clear opt-in to this rule.Consensus from the meeting:
#[inline(always)]
will not result in inlining.#[inline(required)]
unless we are planning to deprecate #[inline(always)]
and transition to required
on some timeframe. We do not think it would be good to have always
and required
both exist on stable (undeprecated) indefinitely.Based on the above, we are going to recommend this MCP be closed because the next steps are all implementation and no RFC is required. Entering final comment period, let us know if you disagree or see a problem with that.
Closing per previous summary.
Proposal
Summary and problem statement
Change the semantics of
#[inline]
attribute.Currently in Rust
#[inline]
is following LLVM-style inline policies#[inline(always)]
->alwaysinline
#[inline]
->inlinehint
N/A -> N/A#[inline(never)]
-neverinline
However this is unnecessarily complex and hard to use.
It also causes compilation performance issues. (e.g. thousands copies of
Option::map
inrustc_middle
according tocargo-llvm-lines
, to ask llvm to examine the inlining possibilities one-by-one)Motivation, use-cases, and solution sketches
Inlining should ideally happen before monomorphization.
Inlining should ideally be made as deterministic as possible (respecting user's intention).
I propose replacing the semantics of these attributes to: **
#[inline(always)]
-> do inline, not just a hint (always invoke deterministic inlining before monomorphization), raise a warning or error when failing to do so. * Feedback: It was suggested that this become#[inline(required)]
, and the check be a lint.#[inline]
-> do inline, not just a hint (always invoke deterministic inlining before monomorphization), fallback to not inlining when failing to do so with good reason TBD: \<Language should identify and list the possible reasons.> (and maybe tag asalwaysinline
to ask llvm to try again). N/A -> keeping current behavior here: heuristically determine whether to inline or not, left to compiler internals (maybe invoke deterministic inlining before monomorphization,inlinehint
).#[inline(never)]
-> do not inline, not just a hint (do not invoke deterministic inlining before monomorphization,neverinline
to stop llvm from doing so too)This will not be a breaking change. There're performance impacts, but hopefully a positive one.
Prioritization
This is related to the "Targeted ergonomic wins and extensions" because it improves building experience. It is also relatively not a large change.
It was mentioned on zulip that the current implementation MIR-inliner is not fully ready. (Need to recheck the feasibility of using MIR inliner to provide the expected "deterministic inlining before monomorphization" behavior and timeframe approximation.)
Links and related work
inline
keyword is currently mainly used to workaround ODR in C++ language. It causes confusion to beginners too.Initial people involved
TBD
What happens now?
This issue is part of the experimental MCP process described in RFC 2936. Once this issue is filed, a Zulip topic will be opened for discussion, and the lang-team will review open MCPs in its weekly triage meetings. You should receive feedback within a week or two.
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
EDIT: