Skip to content

Commit

Permalink
Auto merge of #89883 - Mark-Simulacrum:incr-verify-outline, r=nnether…
Browse files Browse the repository at this point in the history
…cote

Manually outline error on incremental_verify_ich

This reduces codegen for rustc_query_impl by 169k lines of LLVM IR, representing
a 1.2% improvement. This code should be fairly cold, so hopefully this has minimal
performance impact.
  • Loading branch information
bors committed Nov 23, 2021
2 parents 7c4be43 + dc65b22 commit 22c2d9d
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 24 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_query_system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#![feature(let_else)]
#![feature(min_specialization)]
#![feature(thread_local_const_init)]
#![feature(extern_types)]

#[macro_use]
extern crate tracing;
Expand Down
97 changes: 73 additions & 24 deletions compiler/rustc_query_system/src/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use rustc_data_structures::sharded::{get_shard_index_by_hash, Sharded};
use rustc_data_structures::sync::{Lock, LockGuard};
use rustc_data_structures::thin_vec::ThinVec;
use rustc_errors::{DiagnosticBuilder, FatalError};
use rustc_session::Session;
use rustc_span::{Span, DUMMY_SP};
use std::cell::Cell;
use std::collections::hash_map::Entry;
Expand Down Expand Up @@ -595,38 +596,86 @@ fn incremental_verify_ich<CTX, K, V: Debug>(
debug!("END verify_ich({:?})", dep_node);

if Some(new_hash) != old_hash {
let run_cmd = if let Some(crate_name) = &tcx.sess().opts.crate_name {
format!("`cargo clean -p {}` or `cargo clean`", crate_name)
} else {
"`cargo clean`".to_string()
};
incremental_verify_ich_cold(tcx.sess(), DebugArg::from(&dep_node), DebugArg::from(&result));
}
}

// When we emit an error message and panic, we try to debug-print the `DepNode`
// and query result. Unforunately, this can cause us to run additional queries,
// which may result in another fingerprint mismatch while we're in the middle
// of processing this one. To avoid a double-panic (which kills the process
// before we can print out the query static), we print out a terse
// but 'safe' message if we detect a re-entrant call to this method.
thread_local! {
static INSIDE_VERIFY_PANIC: Cell<bool> = const { Cell::new(false) };
};
// This DebugArg business is largely a mirror of std::fmt::ArgumentV1, which is
// currently not exposed publicly.
//
// The PR which added this attempted to use `&dyn Debug` instead, but that
// showed statistically significant worse compiler performance. It's not
// actually clear what the cause there was -- the code should be cold. If this
// can be replaced with `&dyn Debug` with on perf impact, then it probably
// should be.
extern "C" {
type Opaque;
}

let old_in_panic = INSIDE_VERIFY_PANIC.with(|in_panic| in_panic.replace(true));
struct DebugArg<'a> {
value: &'a Opaque,
fmt: fn(&Opaque, &mut std::fmt::Formatter<'_>) -> std::fmt::Result,
}

if old_in_panic {
tcx.sess().struct_err("internal compiler error: re-entrant incremental verify failure, suppressing message")
.emit();
} else {
tcx.sess().struct_err(&format!("internal compiler error: encountered incremental compilation error with {:?}", dep_node))
impl<'a, T> From<&'a T> for DebugArg<'a>
where
T: std::fmt::Debug,
{
fn from(value: &'a T) -> DebugArg<'a> {
DebugArg {
value: unsafe { std::mem::transmute(value) },
fmt: unsafe {
std::mem::transmute(<T as std::fmt::Debug>::fmt as fn(_, _) -> std::fmt::Result)
},
}
}
}

impl std::fmt::Debug for DebugArg<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
(self.fmt)(self.value, f)
}
}

// Note that this is marked #[cold] and intentionally takes the equivalent of
// `dyn Debug` for its arguments, as we want to avoid generating a bunch of
// different implementations for LLVM to chew on (and filling up the final
// binary, too).
#[cold]
fn incremental_verify_ich_cold(sess: &Session, dep_node: DebugArg<'_>, result: DebugArg<'_>) {
let run_cmd = if let Some(crate_name) = &sess.opts.crate_name {
format!("`cargo clean -p {}` or `cargo clean`", crate_name)
} else {
"`cargo clean`".to_string()
};

// When we emit an error message and panic, we try to debug-print the `DepNode`
// and query result. Unfortunately, this can cause us to run additional queries,
// which may result in another fingerprint mismatch while we're in the middle
// of processing this one. To avoid a double-panic (which kills the process
// before we can print out the query static), we print out a terse
// but 'safe' message if we detect a re-entrant call to this method.
thread_local! {
static INSIDE_VERIFY_PANIC: Cell<bool> = const { Cell::new(false) };
};

let old_in_panic = INSIDE_VERIFY_PANIC.with(|in_panic| in_panic.replace(true));

if old_in_panic {
sess.struct_err(
"internal compiler error: re-entrant incremental verify failure, suppressing message",
)
.emit();
} else {
sess.struct_err(&format!("internal compiler error: encountered incremental compilation error with {:?}", dep_node))
.help(&format!("This is a known issue with the compiler. Run {} to allow your project to compile", run_cmd))
.note(&"Please follow the instructions below to create a bug report with the provided information")
.note(&"See <https://github.com/rust-lang/rust/issues/84970> for more information")
.emit();
panic!("Found unstable fingerprints for {:?}: {:?}", dep_node, result);
}

INSIDE_VERIFY_PANIC.with(|in_panic| in_panic.set(old_in_panic));
panic!("Found unstable fingerprints for {:?}: {:?}", dep_node, result);
}

INSIDE_VERIFY_PANIC.with(|in_panic| in_panic.set(old_in_panic));
}

/// Ensure that either this query has all green inputs or been executed.
Expand Down

0 comments on commit 22c2d9d

Please sign in to comment.