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.

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

Couple of refactorings to cg_ssa::base::codegen_crate #97062

Merged
merged 2 commits into from
May 19, 2022
Merged

Couple of refactorings to cg_ssa::base::codegen_crate #97062

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c1f16c
Move cgu_reuse a bit earlier
bjorn3 May 13, 2022
a06deb5
Compute pre_compiled_cgus more eagerly
bjorn3 May 13, 2022
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
57 changes: 24 additions & 33 deletions compiler/rustc_codegen_ssa/src/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ use rustc_attr as attr;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::profiling::{get_resident_set_size, print_time_passes_entry};

use rustc_data_structures::sync::par_iter;
Copy link
Member

Choose a reason for hiding this comment

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

where is par_iter called in cfg(not(parallel_compiler))?

Copy link
Contributor

@klensy klensy May 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think it moved because in other case in will fail to compile, the reason is cfg!(parallel_compiler). If return back #[cfg(parallel_compiler)] all should work (at least i remembering something like that when i touched that part of code here #93787).

Copy link
Member

Choose a reason for hiding this comment

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

My bad, I didn't see the par_iter call in the hidden section between the diffs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I switched from #[cfg] to cfg!() to ensure that changes to the code are always typechecked and not just when explicitly running in the parallel_compiler configuration.

#[cfg(parallel_compiler)]
use rustc_data_structures::sync::{par_iter, ParallelIterator};
use rustc_data_structures::sync::ParallelIterator;
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::lang_items::LangItem;
Expand Down Expand Up @@ -607,6 +608,14 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
second_half.iter().rev().interleave(first_half).copied().collect()
};

// Calculate the CGU reuse
let cgu_reuse = tcx.sess.time("find_cgu_reuse", || {
codegen_units.iter().map(|cgu| determine_cgu_reuse(tcx, &cgu)).collect::<Vec<_>>()
});

let mut total_codegen_time = Duration::new(0, 0);
let start_rss = tcx.sess.time_passes().then(|| get_resident_set_size());

// The non-parallel compiler can only translate codegen units to LLVM IR
// on a single thread, leading to a staircase effect where the N LLVM
// threads have to wait on the single codegen threads to generate work
Expand All @@ -617,8 +626,7 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
// This likely is a temporary measure. Once we don't have to support the
// non-parallel compiler anymore, we can compile CGUs end-to-end in
// parallel and get rid of the complicated scheduling logic.
#[cfg(parallel_compiler)]
let pre_compile_cgus = |cgu_reuse: &[CguReuse]| {
let mut pre_compiled_cgus = if cfg!(parallel_compiler) {
tcx.sess.time("compile_first_CGU_batch", || {
// Try to find one CGU to compile per thread.
let cgus: Vec<_> = cgu_reuse
Expand All @@ -638,48 +646,31 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
})
.collect();

(pre_compiled_cgus, start_time.elapsed())
total_codegen_time += start_time.elapsed();

pre_compiled_cgus
})
} else {
FxHashMap::default()
};

#[cfg(not(parallel_compiler))]
let pre_compile_cgus = |_: &[CguReuse]| (FxHashMap::default(), Duration::new(0, 0));

let mut cgu_reuse = Vec::new();
let mut pre_compiled_cgus: Option<FxHashMap<usize, _>> = None;
let mut total_codegen_time = Duration::new(0, 0);
let start_rss = tcx.sess.time_passes().then(|| get_resident_set_size());

for (i, cgu) in codegen_units.iter().enumerate() {
ongoing_codegen.wait_for_signal_to_codegen_item();
ongoing_codegen.check_for_errors(tcx.sess);

// Do some setup work in the first iteration
if pre_compiled_cgus.is_none() {
// Calculate the CGU reuse
cgu_reuse = tcx.sess.time("find_cgu_reuse", || {
codegen_units.iter().map(|cgu| determine_cgu_reuse(tcx, &cgu)).collect()
});
// Pre compile some CGUs
let (compiled_cgus, codegen_time) = pre_compile_cgus(&cgu_reuse);
pre_compiled_cgus = Some(compiled_cgus);
total_codegen_time += codegen_time;
}

let cgu_reuse = cgu_reuse[i];
tcx.sess.cgu_reuse_tracker.set_actual_reuse(cgu.name().as_str(), cgu_reuse);

match cgu_reuse {
CguReuse::No => {
let (module, cost) =
if let Some(cgu) = pre_compiled_cgus.as_mut().unwrap().remove(&i) {
cgu
} else {
let start_time = Instant::now();
let module = backend.compile_codegen_unit(tcx, cgu.name());
total_codegen_time += start_time.elapsed();
module
};
let (module, cost) = if let Some(cgu) = pre_compiled_cgus.remove(&i) {
cgu
} else {
let start_time = Instant::now();
let module = backend.compile_codegen_unit(tcx, cgu.name());
total_codegen_time += start_time.elapsed();
module
};
// This will unwind if there are errors, which triggers our `AbortCodegenOnDrop`
// guard. Unfortunately, just skipping the `submit_codegened_module_to_llvm` makes
// compilation hang on post-monomorphization errors.
Expand Down