-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
[wip] Tweak cgu partitioning #71349
[wip] Tweak cgu partitioning #71349
Conversation
23a0804
to
ded2710
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit ded271016b03b48bc73843e36d3c1958c242e24d with merge 0c9e1e2a364ddefe107a4988cacd5900ca232245... |
☀️ Try build successful - checks-azure |
Queued 0c9e1e2a364ddefe107a4988cacd5900ca232245 with parent 4ca5fd2, future comparison URL. |
Finished benchmarking try commit 0c9e1e2a364ddefe107a4988cacd5900ca232245, comparison URL. |
as the server that is running perf do not have that many threads I think that the biggest gains is from getting functions calling the same inline marked functions in the same CGU as described in #65281 (comment). |
also this can be part of the solution for #69382 |
ded2710
to
9c97268
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit 9c9726815023031f5fd4e45e9f97a5160e8a8abf with merge 54729123272d6aeceda10eedcdae76fec44d5337... |
☀️ Try build successful - checks-azure |
Queued 54729123272d6aeceda10eedcdae76fec44d5337 with parent e83f756, future comparison URL. |
Remember that there is also a terminator per basic block that is not part of the statements. |
Finished benchmarking try commit 54729123272d6aeceda10eedcdae76fec44d5337, comparison URL. |
There are absolutely no changes in performance. |
9c97268
to
5c4c00a
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit 7b86e2a with merge 35fdb7107589ab8f0cce89e7a5790f12c40ca662... |
☀️ Try build successful - checks-azure |
Queued 35fdb7107589ab8f0cce89e7a5790f12c40ca662 with parent 0aa6751, future comparison URL. |
Finished benchmarking try commit 35fdb7107589ab8f0cce89e7a5790f12c40ca662, comparison URL. |
| StatementKind::Nop => false, | ||
_ => true, | ||
}) | ||
.count() |
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.
shall there maybe be something like
rust/src/librustc_mir/transform/inline.rs
Lines 319 to 373 in 963bf52
let term = blk.terminator(); | |
let mut is_drop = false; | |
match term.kind { | |
TerminatorKind::Drop { ref location, target, unwind } | |
| TerminatorKind::DropAndReplace { ref location, target, unwind, .. } => { | |
is_drop = true; | |
work_list.push(target); | |
// If the location doesn't actually need dropping, treat it like | |
// a regular goto. | |
let ty = location.ty(callee_body, tcx).subst(tcx, callsite.substs).ty; | |
if ty.needs_drop(tcx, param_env) { | |
cost += CALL_PENALTY; | |
if let Some(unwind) = unwind { | |
cost += LANDINGPAD_PENALTY; | |
work_list.push(unwind); | |
} | |
} else { | |
cost += INSTR_COST; | |
} | |
} | |
TerminatorKind::Unreachable | TerminatorKind::Call { destination: None, .. } | |
if first_block => | |
{ | |
// If the function always diverges, don't inline | |
// unless the cost is zero | |
threshold = 0; | |
} | |
TerminatorKind::Call { func: Operand::Constant(ref f), cleanup, .. } => { | |
if let ty::FnDef(def_id, _) = f.literal.ty.kind { | |
// Don't give intrinsics the extra penalty for calls | |
let f = tcx.fn_sig(def_id); | |
if f.abi() == Abi::RustIntrinsic || f.abi() == Abi::PlatformIntrinsic { | |
cost += INSTR_COST; | |
} else { | |
cost += CALL_PENALTY; | |
} | |
} else { | |
cost += CALL_PENALTY; | |
} | |
if cleanup.is_some() { | |
cost += LANDINGPAD_PENALTY; | |
} | |
} | |
TerminatorKind::Assert { cleanup, .. } => { | |
cost += CALL_PENALTY; | |
if cleanup.is_some() { | |
cost += LANDINGPAD_PENALTY; | |
} | |
} | |
TerminatorKind::Resume => cost += RESUME_PENALTY, | |
_ => cost += INSTR_COST, | |
} |
or at least a +1 so the terminator is counted in some way
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.
can maybe be simplified to only add extra for landingpads(10 llvm ir instructions) and resume (9 llvm ir instructions)
r? @ghost