Skip to content

Commit 22c2d9d

Browse files
committed
Auto merge of rust-lang#89883 - Mark-Simulacrum:incr-verify-outline, r=nnethercote
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.
2 parents 7c4be43 + dc65b22 commit 22c2d9d

File tree

2 files changed

+74
-24
lines changed

2 files changed

+74
-24
lines changed

compiler/rustc_query_system/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#![feature(let_else)]
77
#![feature(min_specialization)]
88
#![feature(thread_local_const_init)]
9+
#![feature(extern_types)]
910

1011
#[macro_use]
1112
extern crate tracing;

compiler/rustc_query_system/src/query/plumbing.rs

+73-24
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use rustc_data_structures::sharded::{get_shard_index_by_hash, Sharded};
1818
use rustc_data_structures::sync::{Lock, LockGuard};
1919
use rustc_data_structures::thin_vec::ThinVec;
2020
use rustc_errors::{DiagnosticBuilder, FatalError};
21+
use rustc_session::Session;
2122
use rustc_span::{Span, DUMMY_SP};
2223
use std::cell::Cell;
2324
use std::collections::hash_map::Entry;
@@ -595,38 +596,86 @@ fn incremental_verify_ich<CTX, K, V: Debug>(
595596
debug!("END verify_ich({:?})", dep_node);
596597

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

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

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

616-
if old_in_panic {
617-
tcx.sess().struct_err("internal compiler error: re-entrant incremental verify failure, suppressing message")
618-
.emit();
619-
} else {
620-
tcx.sess().struct_err(&format!("internal compiler error: encountered incremental compilation error with {:?}", dep_node))
620+
impl<'a, T> From<&'a T> for DebugArg<'a>
621+
where
622+
T: std::fmt::Debug,
623+
{
624+
fn from(value: &'a T) -> DebugArg<'a> {
625+
DebugArg {
626+
value: unsafe { std::mem::transmute(value) },
627+
fmt: unsafe {
628+
std::mem::transmute(<T as std::fmt::Debug>::fmt as fn(_, _) -> std::fmt::Result)
629+
},
630+
}
631+
}
632+
}
633+
634+
impl std::fmt::Debug for DebugArg<'_> {
635+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
636+
(self.fmt)(self.value, f)
637+
}
638+
}
639+
640+
// Note that this is marked #[cold] and intentionally takes the equivalent of
641+
// `dyn Debug` for its arguments, as we want to avoid generating a bunch of
642+
// different implementations for LLVM to chew on (and filling up the final
643+
// binary, too).
644+
#[cold]
645+
fn incremental_verify_ich_cold(sess: &Session, dep_node: DebugArg<'_>, result: DebugArg<'_>) {
646+
let run_cmd = if let Some(crate_name) = &sess.opts.crate_name {
647+
format!("`cargo clean -p {}` or `cargo clean`", crate_name)
648+
} else {
649+
"`cargo clean`".to_string()
650+
};
651+
652+
// When we emit an error message and panic, we try to debug-print the `DepNode`
653+
// and query result. Unfortunately, this can cause us to run additional queries,
654+
// which may result in another fingerprint mismatch while we're in the middle
655+
// of processing this one. To avoid a double-panic (which kills the process
656+
// before we can print out the query static), we print out a terse
657+
// but 'safe' message if we detect a re-entrant call to this method.
658+
thread_local! {
659+
static INSIDE_VERIFY_PANIC: Cell<bool> = const { Cell::new(false) };
660+
};
661+
662+
let old_in_panic = INSIDE_VERIFY_PANIC.with(|in_panic| in_panic.replace(true));
663+
664+
if old_in_panic {
665+
sess.struct_err(
666+
"internal compiler error: re-entrant incremental verify failure, suppressing message",
667+
)
668+
.emit();
669+
} else {
670+
sess.struct_err(&format!("internal compiler error: encountered incremental compilation error with {:?}", dep_node))
621671
.help(&format!("This is a known issue with the compiler. Run {} to allow your project to compile", run_cmd))
622672
.note(&"Please follow the instructions below to create a bug report with the provided information")
623673
.note(&"See <https://github.com/rust-lang/rust/issues/84970> for more information")
624674
.emit();
625-
panic!("Found unstable fingerprints for {:?}: {:?}", dep_node, result);
626-
}
627-
628-
INSIDE_VERIFY_PANIC.with(|in_panic| in_panic.set(old_in_panic));
675+
panic!("Found unstable fingerprints for {:?}: {:?}", dep_node, result);
629676
}
677+
678+
INSIDE_VERIFY_PANIC.with(|in_panic| in_panic.set(old_in_panic));
630679
}
631680

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

0 commit comments

Comments
 (0)