Skip to content

More CGU cleanups #112369

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jun 10, 2023
Merged

More CGU cleanups #112369

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0a1cd5b
Remove some unnecessary `&mut`s.
nnethercote Jun 2, 2023
9fd6d97
Improve sorting in `debug_dump`.
nnethercote Jun 2, 2023
392045b
Make the two loops in `internalize_symbols` have the same form.
nnethercote Jun 5, 2023
fe3b646
Merge the two loops in `internalize_symbols`.
nnethercote Jun 5, 2023
8dbb347
Split loop in `place_inlined_mono_item`.
nnethercote Jun 6, 2023
1defd30
Remove `PlacedRootMonoItems::roots`.
nnethercote Jun 6, 2023
8533456
Move `mono_item_placement` construction.
nnethercote Jun 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 19 additions & 30 deletions compiler/rustc_monomorphize/src/partitioning.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,21 +486,7 @@ fn internalize_symbols<'tcx>(
mono_item_placements: FxHashMap<MonoItem<'tcx>, MonoItemPlacement>,
internalization_candidates: FxHashSet<MonoItem<'tcx>>,
) {
if codegen_units.len() == 1 {
// Fast path for when there is only one codegen unit. In this case we
// can internalize all candidates, since there is nowhere else they
// could be used from.
for cgu in codegen_units {
for (item, linkage_and_visibility) in cgu.items_mut() {
if !internalization_candidates.contains(item) {
// This item is no candidate for internalizing, so skip it.
continue;
}
*linkage_and_visibility = (Linkage::Internal, Visibility::Default);
}
}
return;
}
let single_codegen_unit = codegen_units.len() == 1;

// For each internalization candidates in each codegen unit, check if it is
// used from outside its defining codegen unit.
Expand All @@ -512,21 +498,24 @@ fn internalize_symbols<'tcx>(
// This item is no candidate for internalizing, so skip it.
continue;
}
debug_assert_eq!(mono_item_placements[item], home_cgu);

if let Some(user_items) = cx.usage_map.get_user_items(*item) {
if user_items
.iter()
.filter_map(|user_item| {
// Some user mono items might not have been
// instantiated. We can safely ignore those.
mono_item_placements.get(user_item)
})
.any(|placement| *placement != home_cgu)
{
// Found a user from another CGU, so skip to the next item
// without marking this one as internal.
continue;

if !single_codegen_unit {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe just leave some kind of comment here (could copy from the deleted one) about why it's ok to skip this check if there's only one CGU

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt like the "Found a user from another CGU" comment below covered the reasoning sufficiently here.

debug_assert_eq!(mono_item_placements[item], home_cgu);

if let Some(user_items) = cx.usage_map.get_user_items(*item) {
if user_items
.iter()
.filter_map(|user_item| {
// Some user mono items might not have been
// instantiated. We can safely ignore those.
mono_item_placements.get(user_item)
})
.any(|placement| *placement != home_cgu)
{
// Found a user from another CGU, so skip to the next item
// without marking this one as internal.
continue;
}
}
}

Expand Down