From 2dcc828863ed8515df3a0efac507535581d162de Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Tue, 10 Oct 2023 09:39:47 +0800 Subject: [PATCH 1/6] use env variable to control thread ids in rustc_log --- compiler/rustc_log/src/lib.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_log/src/lib.rs b/compiler/rustc_log/src/lib.rs index 3e43bcb3b8f6d..0c9ec556549fa 100644 --- a/compiler/rustc_log/src/lib.rs +++ b/compiler/rustc_log/src/lib.rs @@ -74,6 +74,11 @@ pub fn init_env_logger(env: &str) -> Result<(), Error> { Some(v) => &v != "0", }; + let verbose_thread_ids = match env::var_os(String::from(env) + "_THREAD_IDS") { + None => false, + Some(v) => &v == "1", + }; + let layer = tracing_tree::HierarchicalLayer::default() .with_writer(io::stderr) .with_indent_lines(true) @@ -81,9 +86,9 @@ pub fn init_env_logger(env: &str) -> Result<(), Error> { .with_targets(true) .with_verbose_exit(verbose_entry_exit) .with_verbose_entry(verbose_entry_exit) - .with_indent_amount(2); - #[cfg(all(parallel_compiler, debug_assertions))] - let layer = layer.with_thread_ids(true).with_thread_names(true); + .with_indent_amount(2) + .with_thread_ids(verbose_thread_ids) + .with_thread_names(verbose_thread_ids); let subscriber = tracing_subscriber::Registry::default().with(filter).with(layer); match env::var(format!("{env}_BACKTRACE")) { From f214497d22588c95b94f8ef89ca2db7d71424c36 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 10 Oct 2023 15:44:33 +1100 Subject: [PATCH 2/6] coverage: Replace `ShortCircuitPreorder` with a single function Instead of defining a named struct, we can use `std::iter::from_fn` and store intermediate state in a closure. --- .../rustc_mir_transform/src/coverage/graph.rs | 72 +++++-------------- 1 file changed, 17 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index 812633348e3bd..c94e0baf8be81 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -1,8 +1,9 @@ +use rustc_data_structures::captures::Captures; use rustc_data_structures::graph::dominators::{self, Dominators}; use rustc_data_structures::graph::{self, GraphSuccessors, WithNumNodes, WithStartNode}; use rustc_index::bit_set::BitSet; use rustc_index::{IndexSlice, IndexVec}; -use rustc_middle::mir::{self, BasicBlock, BasicBlockData, Terminator, TerminatorKind}; +use rustc_middle::mir::{self, BasicBlock, Terminator, TerminatorKind}; use std::cmp::Ordering; use std::ops::{Index, IndexMut}; @@ -80,10 +81,9 @@ impl CoverageGraph { // intentionally omits unwind paths. // FIXME(#78544): MIR InstrumentCoverage: Improve coverage of `#[should_panic]` tests and // `catch_unwind()` handlers. - let mir_cfg_without_unwind = ShortCircuitPreorder::new(&mir_body, bcb_filtered_successors); let mut basic_blocks = Vec::new(); - for (bb, data) in mir_cfg_without_unwind { + for bb in short_circuit_preorder(mir_body, bcb_filtered_successors) { if let Some(last) = basic_blocks.last() { let predecessors = &mir_body.basic_blocks.predecessors()[bb]; if predecessors.len() > 1 || !predecessors.contains(last) { @@ -109,7 +109,7 @@ impl CoverageGraph { } basic_blocks.push(bb); - let term = data.terminator(); + let term = mir_body[bb].terminator(); match term.kind { TerminatorKind::Return { .. } @@ -553,66 +553,28 @@ pub(super) fn find_loop_backedges( backedges } -pub struct ShortCircuitPreorder< - 'a, - 'tcx, - F: Fn(&'a mir::Body<'tcx>, &'a TerminatorKind<'tcx>) -> Box + 'a>, -> { +fn short_circuit_preorder<'a, 'tcx, F, Iter>( body: &'a mir::Body<'tcx>, - visited: BitSet, - worklist: Vec, filtered_successors: F, -} - -impl< - 'a, - 'tcx, - F: Fn(&'a mir::Body<'tcx>, &'a TerminatorKind<'tcx>) -> Box + 'a>, -> ShortCircuitPreorder<'a, 'tcx, F> -{ - pub fn new( - body: &'a mir::Body<'tcx>, - filtered_successors: F, - ) -> ShortCircuitPreorder<'a, 'tcx, F> { - let worklist = vec![mir::START_BLOCK]; - - ShortCircuitPreorder { - body, - visited: BitSet::new_empty(body.basic_blocks.len()), - worklist, - filtered_successors, - } - } -} - -impl< - 'a, - 'tcx, - F: Fn(&'a mir::Body<'tcx>, &'a TerminatorKind<'tcx>) -> Box + 'a>, -> Iterator for ShortCircuitPreorder<'a, 'tcx, F> +) -> impl Iterator + Captures<'a> + Captures<'tcx> +where + F: Fn(&'a mir::Body<'tcx>, &'a TerminatorKind<'tcx>) -> Iter, + Iter: Iterator, { - type Item = (BasicBlock, &'a BasicBlockData<'tcx>); + let mut visited = BitSet::new_empty(body.basic_blocks.len()); + let mut worklist = vec![mir::START_BLOCK]; - fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { - while let Some(idx) = self.worklist.pop() { - if !self.visited.insert(idx) { + std::iter::from_fn(move || { + while let Some(bb) = worklist.pop() { + if !visited.insert(bb) { continue; } - let data = &self.body[idx]; - - if let Some(ref term) = data.terminator { - self.worklist.extend((self.filtered_successors)(&self.body, &term.kind)); - } + worklist.extend(filtered_successors(body, &body[bb].terminator().kind)); - return Some((idx, data)); + return Some(bb); } None - } - - fn size_hint(&self) -> (usize, Option) { - let size = self.body.basic_blocks.len() - self.visited.count(); - (size, Some(size)) - } + }) } From 5d629457fd3bb580b87800f7c2858ddfe3f1c0ac Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 10 Oct 2023 15:09:04 +1100 Subject: [PATCH 3/6] coverage: Unbox and simplify `bcb_filtered_successors` This function already has access to the MIR body, so instead of taking a reference to a terminator, it's simpler and easier to pass in a basic block index. There is no need to box the returned iterator if we instead add appropriate lifetime captures, since `short_circuit_preorder` is now generic over the type of iterator it expects. We can also greatly simplify the function's implementation by observing that the only difference between its two cases is whether we take all of a BB's successors, or just the first one. --- .../rustc_mir_transform/src/coverage/graph.rs | 50 +++++++++---------- .../rustc_mir_transform/src/coverage/tests.rs | 2 +- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index c94e0baf8be81..39027d21f066a 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -3,7 +3,7 @@ use rustc_data_structures::graph::dominators::{self, Dominators}; use rustc_data_structures::graph::{self, GraphSuccessors, WithNumNodes, WithStartNode}; use rustc_index::bit_set::BitSet; use rustc_index::{IndexSlice, IndexVec}; -use rustc_middle::mir::{self, BasicBlock, Terminator, TerminatorKind}; +use rustc_middle::mir::{self, BasicBlock, TerminatorKind}; use std::cmp::Ordering; use std::ops::{Index, IndexMut}; @@ -37,9 +37,8 @@ impl CoverageGraph { } let bcb_data = &bcbs[bcb]; let mut bcb_successors = Vec::new(); - for successor in - bcb_filtered_successors(&mir_body, &bcb_data.terminator(mir_body).kind) - .filter_map(|successor_bb| bb_to_bcb[successor_bb]) + for successor in bcb_filtered_successors(&mir_body, bcb_data.last_bb()) + .filter_map(|successor_bb| bb_to_bcb[successor_bb]) { if !seen[successor] { seen[successor] = true; @@ -316,11 +315,6 @@ impl BasicCoverageBlockData { pub fn last_bb(&self) -> BasicBlock { *self.basic_blocks.last().unwrap() } - - #[inline(always)] - pub fn terminator<'a, 'tcx>(&self, mir_body: &'a mir::Body<'tcx>) -> &'a Terminator<'tcx> { - &mir_body[self.last_bb()].terminator() - } } /// Represents a successor from a branching BasicCoverageBlock (such as the arms of a `SwitchInt`) @@ -362,26 +356,28 @@ impl std::fmt::Debug for BcbBranch { } } -// Returns the `Terminator`s non-unwind successors. +// Returns the subset of a block's successors that are relevant to the coverage +// graph, i.e. those that do not represent unwinds or unreachable branches. // FIXME(#78544): MIR InstrumentCoverage: Improve coverage of `#[should_panic]` tests and // `catch_unwind()` handlers. fn bcb_filtered_successors<'a, 'tcx>( body: &'a mir::Body<'tcx>, - term_kind: &'a TerminatorKind<'tcx>, -) -> Box + 'a> { - Box::new( - match &term_kind { - // SwitchInt successors are never unwind, and all of them should be traversed. - TerminatorKind::SwitchInt { ref targets, .. } => { - None.into_iter().chain(targets.all_targets().into_iter().copied()) - } - // For all other kinds, return only the first successor, if any, and ignore unwinds. - // NOTE: `chain(&[])` is required to coerce the `option::iter` (from - // `next().into_iter()`) into the `mir::Successors` aliased type. - _ => term_kind.successors().next().into_iter().chain((&[]).into_iter().copied()), - } - .filter(move |&successor| body[successor].terminator().kind != TerminatorKind::Unreachable), - ) + bb: BasicBlock, +) -> impl Iterator + Captures<'a> + Captures<'tcx> { + let terminator = body[bb].terminator(); + + let take_n_successors = match terminator.kind { + // SwitchInt successors are never unwinds, so all of them should be traversed. + TerminatorKind::SwitchInt { .. } => usize::MAX, + // For all other kinds, return only the first successor (if any), ignoring any + // unwind successors. + _ => 1, + }; + + terminator + .successors() + .take(take_n_successors) + .filter(move |&successor| body[successor].terminator().kind != TerminatorKind::Unreachable) } /// Maintains separate worklists for each loop in the BasicCoverageBlock CFG, plus one for the @@ -558,7 +554,7 @@ fn short_circuit_preorder<'a, 'tcx, F, Iter>( filtered_successors: F, ) -> impl Iterator + Captures<'a> + Captures<'tcx> where - F: Fn(&'a mir::Body<'tcx>, &'a TerminatorKind<'tcx>) -> Iter, + F: Fn(&'a mir::Body<'tcx>, BasicBlock) -> Iter, Iter: Iterator, { let mut visited = BitSet::new_empty(body.basic_blocks.len()); @@ -570,7 +566,7 @@ where continue; } - worklist.extend(filtered_successors(body, &body[bb].terminator().kind)); + worklist.extend(filtered_successors(body, bb)); return Some(bb); } diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index 7476d3ce9272a..ee7cb791c8180 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -241,7 +241,7 @@ fn print_coverage_graphviz( " {:?} [label=\"{:?}: {}\"];\n{}", bcb, bcb, - bcb_data.terminator(mir_body).kind.name(), + mir_body[bcb_data.last_bb()].terminator().kind.name(), basic_coverage_blocks .successors(bcb) .map(|successor| { format!(" {:?} -> {:?};", bcb, successor) }) From 2de454637f56c678e33f4de83d50a5b27b46e56a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 10 Oct 2023 11:08:47 +0200 Subject: [PATCH 4/6] -Zmir-enable-passes: document that this may enable unsound passes --- compiler/rustc_session/src/options.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index c1424db600eff..81de3db6d0dd8 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1610,9 +1610,10 @@ options! { "emit Retagging MIR statements, interpreted e.g., by miri; implies -Zmir-opt-level=0 \ (default: no)"), mir_enable_passes: Vec<(String, bool)> = (Vec::new(), parse_list_with_polarity, [TRACKED], - "use like `-Zmir-enable-passes=+DestinationPropagation,-InstSimplify`. Forces the specified passes to be \ - enabled, overriding all other checks. Passes that are not specified are enabled or \ - disabled by other flags as usual."), + "use like `-Zmir-enable-passes=+DestinationPropagation,-InstSimplify`. Forces the \ + specified passes to be enabled, overriding all other checks. In particular, this will \ + enable unsound (known-buggy and hence usually disabled) passes without further warning! \ + Passes that are not specified are enabled or disabled by other flags as usual."), mir_include_spans: bool = (false, parse_bool, [UNTRACKED], "use line numbers relative to the function in mir pretty printing"), mir_keep_place_mention: bool = (false, parse_bool, [TRACKED], From d805b265dbc254cb9b8cee8c9a1eff9cd161a87a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 10 Oct 2023 11:17:27 +0200 Subject: [PATCH 5/6] add some comments explaining why MIR opts are marked as unsound --- compiler/rustc_mir_transform/src/early_otherwise_branch.rs | 1 + compiler/rustc_mir_transform/src/large_enums.rs | 3 +++ compiler/rustc_mir_transform/src/nrvo.rs | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs index 319fb4eaf3eca..6eb6cb069fec0 100644 --- a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs +++ b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs @@ -95,6 +95,7 @@ pub struct EarlyOtherwiseBranch; impl<'tcx> MirPass<'tcx> for EarlyOtherwiseBranch { fn is_enabled(&self, sess: &rustc_session::Session) -> bool { + // unsound: https://github.com/rust-lang/rust/issues/95162 sess.mir_opt_level() >= 3 && sess.opts.unstable_opts.unsound_mir_opts } diff --git a/compiler/rustc_mir_transform/src/large_enums.rs b/compiler/rustc_mir_transform/src/large_enums.rs index 886ff760481e0..0a8b13d6677ad 100644 --- a/compiler/rustc_mir_transform/src/large_enums.rs +++ b/compiler/rustc_mir_transform/src/large_enums.rs @@ -30,6 +30,9 @@ pub struct EnumSizeOpt { impl<'tcx> MirPass<'tcx> for EnumSizeOpt { fn is_enabled(&self, sess: &Session) -> bool { + // There are some differences in behavior on wasm and ARM that are not properly + // understood, so we conservatively treat this optimization as unsound: + // https://github.com/rust-lang/rust/pull/85158#issuecomment-1101836457 sess.opts.unstable_opts.unsound_mir_opts || sess.mir_opt_level() >= 3 } fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { diff --git a/compiler/rustc_mir_transform/src/nrvo.rs b/compiler/rustc_mir_transform/src/nrvo.rs index e1298b0654f2c..ff309bd10ec82 100644 --- a/compiler/rustc_mir_transform/src/nrvo.rs +++ b/compiler/rustc_mir_transform/src/nrvo.rs @@ -34,7 +34,7 @@ pub struct RenameReturnPlace; impl<'tcx> MirPass<'tcx> for RenameReturnPlace { fn is_enabled(&self, sess: &rustc_session::Session) -> bool { - // #111005 + // unsound: #111005 sess.mir_opt_level() > 0 && sess.opts.unstable_opts.unsound_mir_opts } From c70ef36f2cd7e505022a534a9a9bb9f472cf3610 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 10 Oct 2023 09:55:22 +0000 Subject: [PATCH 6/6] reorder files in solve --- .../src/solve/{eval_ctxt.rs => eval_ctxt/mod.rs} | 0 compiler/rustc_trait_selection/src/solve/mod.rs | 3 --- .../src/solve/{ => project_goals}/inherent_projection.rs | 0 .../src/solve/{project_goals.rs => project_goals/mod.rs} | 4 ++++ .../src/solve/{ => project_goals}/opaques.rs | 2 +- .../src/solve/{ => project_goals}/weak_types.rs | 0 .../src/solve/{search_graph/mod.rs => search_graph.rs} | 0 7 files changed, 5 insertions(+), 4 deletions(-) rename compiler/rustc_trait_selection/src/solve/{eval_ctxt.rs => eval_ctxt/mod.rs} (100%) rename compiler/rustc_trait_selection/src/solve/{ => project_goals}/inherent_projection.rs (100%) rename compiler/rustc_trait_selection/src/solve/{project_goals.rs => project_goals/mod.rs} (99%) rename compiler/rustc_trait_selection/src/solve/{ => project_goals}/opaques.rs (98%) rename compiler/rustc_trait_selection/src/solve/{ => project_goals}/weak_types.rs (100%) rename compiler/rustc_trait_selection/src/solve/{search_graph/mod.rs => search_graph.rs} (100%) diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs similarity index 100% rename from compiler/rustc_trait_selection/src/solve/eval_ctxt.rs rename to compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs index 77a3b5e128457..d45fe102805ad 100644 --- a/compiler/rustc_trait_selection/src/solve/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/mod.rs @@ -32,14 +32,11 @@ mod assembly; mod canonicalize; mod eval_ctxt; mod fulfill; -mod inherent_projection; pub mod inspect; mod normalize; -mod opaques; mod project_goals; mod search_graph; mod trait_goals; -mod weak_types; pub use eval_ctxt::{ EvalCtxt, GenerateProofTree, InferCtxtEvalExt, InferCtxtSelectExt, UseGlobalCache, diff --git a/compiler/rustc_trait_selection/src/solve/inherent_projection.rs b/compiler/rustc_trait_selection/src/solve/project_goals/inherent_projection.rs similarity index 100% rename from compiler/rustc_trait_selection/src/solve/inherent_projection.rs rename to compiler/rustc_trait_selection/src/solve/project_goals/inherent_projection.rs diff --git a/compiler/rustc_trait_selection/src/solve/project_goals.rs b/compiler/rustc_trait_selection/src/solve/project_goals/mod.rs similarity index 99% rename from compiler/rustc_trait_selection/src/solve/project_goals.rs rename to compiler/rustc_trait_selection/src/solve/project_goals/mod.rs index 339a3e738467c..2c000293f268d 100644 --- a/compiler/rustc_trait_selection/src/solve/project_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/project_goals/mod.rs @@ -18,6 +18,10 @@ use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_middle::ty::{ToPredicate, TypeVisitableExt}; use rustc_span::{sym, ErrorGuaranteed, DUMMY_SP}; +mod inherent_projection; +mod opaques; +mod weak_types; + impl<'tcx> EvalCtxt<'_, 'tcx> { #[instrument(level = "debug", skip(self), ret)] pub(super) fn compute_projection_goal( diff --git a/compiler/rustc_trait_selection/src/solve/opaques.rs b/compiler/rustc_trait_selection/src/solve/project_goals/opaques.rs similarity index 98% rename from compiler/rustc_trait_selection/src/solve/opaques.rs rename to compiler/rustc_trait_selection/src/solve/project_goals/opaques.rs index f08adc0208b57..ebd129f32b919 100644 --- a/compiler/rustc_trait_selection/src/solve/opaques.rs +++ b/compiler/rustc_trait_selection/src/solve/project_goals/opaques.rs @@ -7,7 +7,7 @@ use rustc_middle::traits::Reveal; use rustc_middle::ty; use rustc_middle::ty::util::NotUniqueParam; -use super::{EvalCtxt, SolverMode}; +use crate::solve::{EvalCtxt, SolverMode}; impl<'tcx> EvalCtxt<'_, 'tcx> { pub(super) fn normalize_opaque_type( diff --git a/compiler/rustc_trait_selection/src/solve/weak_types.rs b/compiler/rustc_trait_selection/src/solve/project_goals/weak_types.rs similarity index 100% rename from compiler/rustc_trait_selection/src/solve/weak_types.rs rename to compiler/rustc_trait_selection/src/solve/project_goals/weak_types.rs diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs b/compiler/rustc_trait_selection/src/solve/search_graph.rs similarity index 100% rename from compiler/rustc_trait_selection/src/solve/search_graph/mod.rs rename to compiler/rustc_trait_selection/src/solve/search_graph.rs