Skip to content

Refactor passes and pass execution to be more parallel #58679

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

Merged
merged 9 commits into from
Mar 9, 2019
1 change: 1 addition & 0 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ define_dep_nodes!( <'tcx>
[eval_always] CoherenceInherentImplOverlapCheck,
[] CoherenceCheckTrait(DefId),
[eval_always] PrivacyAccessLevels(CrateNum),
[eval_always] CheckPrivateInPublic(CrateNum),
[eval_always] Analysis(CrateNum),

// Represents the MIR for a fn; also used as the task node for
Expand Down
6 changes: 0 additions & 6 deletions src/librustc/hir/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,12 +344,6 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckAttrVisitor<'a, 'tcx> {
}
}

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
for &module in tcx.hir().krate().modules.keys() {
tcx.ensure().check_mod_attrs(tcx.hir().local_def_id(module));
}
}

fn is_c_like_enum(item: &hir::Item) -> bool {
if let hir::ItemKind::Enum(ref def, _) = item.node {
for variant in &def.variants {
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use syntax::util::parser::ExprPrecedence;
use crate::ty::AdtKind;
use crate::ty::query::Providers;

use rustc_data_structures::sync::{ParallelIterator, par_iter, Send, Sync};
use rustc_data_structures::sync::{par_for_each_in, Send, Sync};
use rustc_data_structures::thin_vec::ThinVec;

use serialize::{self, Encoder, Encodable, Decoder, Decodable};
Expand Down Expand Up @@ -782,15 +782,15 @@ impl Crate {
where V: itemlikevisit::ParItemLikeVisitor<'hir> + Sync + Send
{
parallel!({
par_iter(&self.items).for_each(|(_, item)| {
par_for_each_in(&self.items, |(_, item)| {
visitor.visit_item(item);
});
}, {
par_iter(&self.trait_items).for_each(|(_, trait_item)| {
par_for_each_in(&self.trait_items, |(_, trait_item)| {
visitor.visit_trait_item(trait_item);
});
}, {
par_iter(&self.impl_items).for_each(|(_, impl_item)| {
par_for_each_in(&self.impl_items, |(_, impl_item)| {
visitor.visit_impl_item(impl_item);
});
});
Expand Down
6 changes: 0 additions & 6 deletions src/librustc/middle/intrinsicck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ use syntax_pos::Span;
use crate::hir::intravisit::{self, Visitor, NestedVisitorMap};
use crate::hir;

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
for &module in tcx.hir().krate().modules.keys() {
tcx.ensure().check_mod_intrinsics(tcx.hir().local_def_id(module));
}
}

fn check_mod_intrinsics<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) {
tcx.hir().visit_item_likes_in_module(
module_def_id,
Expand Down
6 changes: 0 additions & 6 deletions src/librustc/middle/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,6 @@ fn check_mod_liveness<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) {
tcx.hir().visit_item_likes_in_module(module_def_id, &mut IrMaps::new(tcx).as_deep_visitor());
}

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
for &module in tcx.hir().krate().modules.keys() {
tcx.ensure().check_mod_liveness(tcx.hir().local_def_id(module));
}
}

pub fn provide(providers: &mut Providers<'_>) {
*providers = Providers {
check_mod_liveness,
Expand Down
6 changes: 0 additions & 6 deletions src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,12 +456,6 @@ impl<'a, 'tcx> Index<'tcx> {
}
}

pub fn check_unstable_api_usage<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
for &module in tcx.hir().krate().modules.keys() {
tcx.ensure().check_mod_unstable_api_usage(tcx.hir().local_def_id(module));
}
}

/// Cross-references the feature names of unstable APIs with enabled
/// features and possibly prints errors.
fn check_mod_unstable_api_usage<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) {
Expand Down
6 changes: 6 additions & 0 deletions src/librustc/ty/query/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,12 @@ impl<'tcx> QueryDescription<'tcx> for queries::privacy_access_levels<'tcx> {
}
}

impl<'tcx> QueryDescription<'tcx> for queries::check_private_in_public<'tcx> {
fn describe(_: TyCtxt<'_, '_, '_>, _: CrateNum) -> Cow<'static, str> {
"checking for private elements in public interfaces".into()
}
}

impl<'tcx> QueryDescription<'tcx> for queries::typeck_item_bodies<'tcx> {
fn describe(_: TyCtxt<'_, '_, '_>, _: CrateNum) -> Cow<'static, str> {
"type-checking all item bodies".into()
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/ty/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,9 @@ define_queries! { <'tcx>
[] fn check_match: CheckMatch(DefId)
-> Result<(), ErrorReported>,

/// Performs the privacy check and computes "access levels".
/// Performs part of the privacy check and computes "access levels".
[] fn privacy_access_levels: PrivacyAccessLevels(CrateNum) -> Lrc<AccessLevels>,
[] fn check_private_in_public: CheckPrivateInPublic(CrateNum) -> (),
},

Other {
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>,
force!(crate_inherent_impls_overlap_check, LOCAL_CRATE)
},
DepKind::PrivacyAccessLevels => { force!(privacy_access_levels, LOCAL_CRATE); }
DepKind::CheckPrivateInPublic => { force!(check_private_in_public, LOCAL_CRATE); }
DepKind::MirBuilt => { force!(mir_built, def_id!()); }
DepKind::MirConstQualif => { force!(mir_const_qualif, def_id!()); }
DepKind::MirConst => { force!(mir_const, def_id!()); }
Expand Down
61 changes: 54 additions & 7 deletions src/librustc_data_structures/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ cfg_if! {
}

use std::ops::Add;
use std::panic::{resume_unwind, catch_unwind, AssertUnwindSafe};

#[derive(Debug)]
pub struct Atomic<T: Copy>(Cell<T>);
Expand Down Expand Up @@ -130,7 +131,21 @@ cfg_if! {
#[macro_export]
macro_rules! parallel {
($($blocks:tt),*) => {
$($blocks)*;
// We catch panics here ensuring that all the blocks execute.
// This makes behavior consistent with the parallel compiler.
let mut panic = None;
$(
if let Err(p) = ::std::panic::catch_unwind(
::std::panic::AssertUnwindSafe(|| $blocks)
) {
if panic.is_none() {
panic = Some(p);
}
}
)*
if let Some(panic) = panic {
::std::panic::resume_unwind(panic);
}
Copy link
Member

Choose a reason for hiding this comment

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

What's going on here? Is this to make behaviour more consistent with parallel execution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Otherwise the single threaded compiler will stop at the first fatal error and the test output will diverge from the parallel compiler.

}
}

Expand All @@ -140,6 +155,26 @@ cfg_if! {
t.into_iter()
}

pub fn par_for_each_in<T: IntoIterator>(
t: T,
for_each:
impl Fn(<<T as IntoIterator>::IntoIter as Iterator>::Item) + Sync + Send
) {
// We catch panics here ensuring that all the loop iterations execute.
// This makes behavior consistent with the parallel compiler.
let mut panic = None;
t.into_iter().for_each(|i| {
if let Err(p) = catch_unwind(AssertUnwindSafe(|| for_each(i))) {
if panic.is_none() {
panic = Some(p);
}
}
});
if let Some(panic) = panic {
resume_unwind(panic);
}
}

pub type MetadataRef = OwningRef<Box<dyn Erased>, [u8]>;

pub use std::rc::Rc as Lrc;
Expand Down Expand Up @@ -278,23 +313,26 @@ cfg_if! {
use std::thread;
pub use rayon::{join, scope};

/// Runs a list of blocks in parallel. The first block is executed immediately on
/// the current thread. Use that for the longest running block.
#[macro_export]
macro_rules! parallel {
(impl [$($c:tt,)*] [$block:tt $(, $rest:tt)*]) => {
parallel!(impl [$block, $($c,)*] [$($rest),*])
(impl $fblock:tt [$($c:tt,)*] [$block:tt $(, $rest:tt)*]) => {
parallel!(impl $fblock [$block, $($c,)*] [$($rest),*])
};
(impl [$($blocks:tt,)*] []) => {
(impl $fblock:tt [$($blocks:tt,)*] []) => {
::rustc_data_structures::sync::scope(|s| {
$(
s.spawn(|_| $blocks);
)*
$fblock;
})
};
($($blocks:tt),*) => {
// Reverse the order of the blocks since Rayon executes them in reverse order
($fblock:tt, $($blocks:tt),*) => {
// Reverse the order of the later blocks since Rayon executes them in reverse order
// when using a single thread. This ensures the execution order matches that
// of a single threaded rustc
parallel!(impl [] [$($blocks),*]);
parallel!(impl $fblock [] [$($blocks),*]);
};
}

Expand All @@ -307,6 +345,15 @@ cfg_if! {
t.into_par_iter()
}

pub fn par_for_each_in<T: IntoParallelIterator>(
t: T,
for_each: impl Fn(
<<T as IntoParallelIterator>::Iter as ParallelIterator>::Item
) + Sync + Send
) {
t.into_par_iter().for_each(for_each)
}

pub type MetadataRef = OwningRef<Box<dyn Erased + Send + Sync>, [u8]>;

/// This makes locks panic if they are already held.
Expand Down
98 changes: 54 additions & 44 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use rustc_borrowck as borrowck;
use rustc_codegen_utils::codegen_backend::CodegenBackend;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::stable_hasher::StableHasher;
use rustc_data_structures::sync::Lrc;
use rustc_data_structures::sync::{Lrc, ParallelIterator, par_iter};
use rustc_incremental;
use rustc_metadata::creader::CrateLoader;
use rustc_metadata::cstore::{self, CStore};
Expand Down Expand Up @@ -191,51 +191,50 @@ fn analysis<'tcx>(

let sess = tcx.sess;

parallel!({
time(sess, "looking for entry point", || {
middle::entry::find_entry_point(tcx)
});
time(sess, "misc checking 1", || {
parallel!({
time(sess, "looking for entry point", || {
middle::entry::find_entry_point(tcx)
});

time(sess, "looking for plugin registrar", || {
plugin::build::find_plugin_registrar(tcx)
});
time(sess, "looking for plugin registrar", || {
plugin::build::find_plugin_registrar(tcx)
});

time(sess, "looking for derive registrar", || {
proc_macro_decls::find(tcx)
});
}, {
time(sess, "loop checking", || loops::check_crate(tcx));
}, {
time(sess, "attribute checking", || {
hir::check_attr::check_crate(tcx)
});
}, {
time(sess, "stability checking", || {
stability::check_unstable_api_usage(tcx)
time(sess, "looking for derive registrar", || {
proc_macro_decls::find(tcx)
});
}, {
par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| {
tcx.ensure().check_mod_loops(tcx.hir().local_def_id(module));
tcx.ensure().check_mod_attrs(tcx.hir().local_def_id(module));
tcx.ensure().check_mod_unstable_api_usage(tcx.hir().local_def_id(module));
});
});
});

// passes are timed inside typeck
typeck::check_crate(tcx)?;

time(sess, "misc checking", || {
time(sess, "misc checking 2", || {
parallel!({
time(sess, "rvalue promotion", || {
rvalue_promotion::check_crate(tcx)
});
}, {
time(sess, "intrinsic checking", || {
middle::intrinsicck::check_crate(tcx)
time(sess, "rvalue promotion + match checking", || {
tcx.par_body_owners(|def_id| {
tcx.ensure().const_is_rvalue_promotable_to_static(def_id);
tcx.ensure().check_match(def_id);
});
});
}, {
time(sess, "match checking", || mir::matchck_crate(tcx));
}, {
// this must run before MIR dump, because
// "not all control paths return a value" is reported here.
//
// maybe move the check to a MIR pass?
time(sess, "liveness checking", || {
middle::liveness::check_crate(tcx)
time(sess, "liveness checking + intrinsic checking", || {
par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| {
// this must run before MIR dump, because
// "not all control paths return a value" is reported here.
//
// maybe move the check to a MIR pass?
tcx.ensure().check_mod_liveness(tcx.hir().local_def_id(module));

tcx.ensure().check_mod_intrinsics(tcx.hir().local_def_id(module));
});
});
});
});
Expand Down Expand Up @@ -276,19 +275,30 @@ fn analysis<'tcx>(
return Err(ErrorReported);
}

time(sess, "misc checking", || {
time(sess, "misc checking 3", || {
parallel!({
time(sess, "privacy checking", || {
rustc_privacy::check_crate(tcx)
time(sess, "privacy access levels", || {
tcx.ensure().privacy_access_levels(LOCAL_CRATE);
});
}, {
time(sess, "death checking", || middle::dead::check_crate(tcx));
}, {
time(sess, "unused lib feature checking", || {
stability::check_unused_or_stable_features(tcx)
parallel!({
time(sess, "private in public", || {
tcx.ensure().check_private_in_public(LOCAL_CRATE);
});
}, {
time(sess, "death checking", || middle::dead::check_crate(tcx));
}, {
time(sess, "unused lib feature checking", || {
stability::check_unused_or_stable_features(tcx)
});
}, {
time(sess, "lint checking", || lint::check_crate(tcx));
});
}, {
time(sess, "lint checking", || lint::check_crate(tcx));
time(sess, "privacy checking modules", || {
par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| {
tcx.ensure().check_mod_privacy(tcx.hir().local_def_id(module));
});
});
});
});

Expand Down
7 changes: 0 additions & 7 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,6 @@ use std::slice;
use syntax::ptr::P;
use syntax_pos::{Span, DUMMY_SP, MultiSpan};

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
for def_id in tcx.body_owners() {
tcx.ensure().check_match(def_id);
}
tcx.sess.abort_if_errors();
}

pub(crate) fn check_match<'a, 'tcx>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId,
Expand Down
1 change: 0 additions & 1 deletion src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
mod _match;
mod check_match;

pub use self::check_match::check_crate;
pub(crate) use self::check_match::check_match;

use crate::const_eval::{const_field, const_variant_index};
Expand Down
1 change: 0 additions & 1 deletion src/librustc_mir/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ pub mod interpret;
pub mod monomorphize;
pub mod const_eval;

pub use hair::pattern::check_crate as matchck_crate;
use rustc::ty::query::Providers;

pub fn provide(providers: &mut Providers<'_>) {
Expand Down
Loading