Skip to content
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

Make small functions implicitly #[inline] #78120

Closed
jonas-schievink opened this issue Oct 19, 2020 · 6 comments
Closed

Make small functions implicitly #[inline] #78120

jonas-schievink opened this issue Oct 19, 2020 · 6 comments
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt-inlining Area: MIR inlining C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jonas-schievink
Copy link
Contributor

jonas-schievink commented Oct 19, 2020

It is very common to clutter libraries with #[inline] annotations on small/trivial functions to ensure that they optimize well. It shouldn't really be necessary to do this by hand so often. Instead, the compiler could use its inlining heuristic (which already exists for the MIR inliner) to decide when it is likely to be beneficial to make a function inlineable.

This was previously attempted in #70550

@jonas-schievink jonas-schievink added I-slow Issue: Problems and improvements with respect to performance of generated code. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html labels Oct 19, 2020
@thomcc
Copy link
Member

thomcc commented Oct 23, 2020

I'd appreciate if there were a way to get the current behavior then — something between #[inline(never)] and #[inline]... Avoiding #[inline] can substantially improve compile times.

That said supporting this complicates this issue.

@thomcc
Copy link
Member

thomcc commented Nov 4, 2020

I stand by what I said here — but at the same time I did just go through the (tedious) process of manually adding this to a crate that had very few #[inline] annotations in rusqlite/rusqlite#834

@jonas-schievink
Copy link
Contributor Author

I wonder if doing this plus MIR inlining might not end up improving compile times. This is only about small functions, after all.

@camelid camelid added the A-mir-opt-inlining Area: MIR inlining label Feb 7, 2021
@mati865
Copy link
Contributor

mati865 commented Mar 11, 2021

What is the current status on this (or how to push it forward)?

Looking at #70550 the MIR inliner seems to already have what it needs but it's not enabled yet.

@bjorn3
Copy link
Member

bjorn3 commented Mar 12, 2021

We do not currentlt store the MIR for non-#[inline] and non-generic functions in the crate metadata as necessary for inlining cross-crate.

@tmiasko
Copy link
Contributor

tmiasko commented Nov 14, 2023

Implemented in #116505.

@tmiasko tmiasko closed this as completed Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt-inlining Area: MIR inlining C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants