-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Consider inexpensive inlining criteria first #82491
Conversation
Refactor inlining decisions so that inexpensive criteria are considered first: 1. Based on code generation attributes. 2. Based on MIR availability (examines call graph). 3. Based on MIR body.
r? @lcnr (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
// a lower `HirId` than the callee. This ensures that the callee will | ||
// not inline us. This trick only works without incremental compilation. | ||
// So don't do it if that is enabled. | ||
if !self.tcx.dep_graph.is_fully_enabled() && self.hir_id < callee_hir_id { |
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.
is that check even necessary?
I would expect us to now prevent this thanks to mir_callgraph_reachable
apart from that r=me
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.
It is valuable as an optimization to avoid using mir_callgraph_reachable
which is way more expensive in comparison.
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 wait, we use this to always return Ok(())
, even if we have a cycle in the call graph.
That's clever ✨
@bors r+ rollup |
📌 Commit 6d5c0c1 has been approved by |
Consider inexpensive inlining criteria first Refactor inlining decisions so that inexpensive criteria are considered first: 1. Based on code generation attributes. 2. Based on MIR availability (examines call graph). 3. Based on MIR body.
Consider inexpensive inlining criteria first Refactor inlining decisions so that inexpensive criteria are considered first: 1. Based on code generation attributes. 2. Based on MIR availability (examines call graph). 3. Based on MIR body.
…laumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#81940 (Stabilize str_split_once) - rust-lang#82165 (Reword labels on E0308 involving async fn return type) - rust-lang#82456 (Replaced some unwrap_or and map_or with lazy variants) - rust-lang#82491 (Consider inexpensive inlining criteria first) - rust-lang#82506 (Properly account for non-shorthand pattern field in unused variable lint) - rust-lang#82535 (Set codegen thread names) - rust-lang#82545 (rustdoc: add optional woff2 versions of FiraSans.) - rust-lang#82549 (Revert "Update normalize.css to 8.0.1") Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Refactor inlining decisions so that inexpensive criteria are considered first: