From db849dcdfa33863ec0bedb44decf14ab0c799d5c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 31 Oct 2024 11:02:22 +1100 Subject: [PATCH] Replace `BorrowckResults` with `Borrowck`. The results of most analyses end up in a `Results<'tcx, A>`, where `A` is the analysis. It's then possible to traverse the results via a `ResultsVisitor`, which relies on the `ResultsVisitable` trait. (That trait ends up using the same `apply_*` methods that were used when computing the analysis, albeit indirectly.) This pattern of "compute analysis results, then visit them" is common. But there is one exception. For borrow checking we compute three separate analyses (`Borrows`, `MaybeUninitializedPlaces`, and `EverInitializedPlaces`), combine them into a single `BorrowckResults`, and then do a single visit of that `BorrowckResults` with `MirBorrowckResults`. `BorrowckResults` is just different enough from `Results` that it requires the existence of `ResultsVisitable`, which abstracts over the traversal differences between `Results` and `BorrowckResults`. This commit changes things by introducing `Borrowck` and bundling the three borrowck analysis results into a standard `Results` instead of the exceptional `BorrowckResults`. Once that's done, the results can be visited like any other analysis results. `BorrowckResults` is removed, as is `impl ResultsVisitable for BorrowckResults`. (It's instructive to see how similar the added `impl Analysis for Borrowck` is to the removed `impl ResultsVisitable for BorrowckResults`. They're both doing exactly the same things.) Overall this increases the number of lines of code and might not seem like a win. But it enables the removal of `ResultsVisitable` in the next commit, which results in many simplifications. --- compiler/rustc_borrowck/src/dataflow.rs | 160 +++++++++++++----- compiler/rustc_borrowck/src/lib.rs | 79 +++++---- .../rustc_mir_dataflow/src/framework/mod.rs | 2 +- .../src/framework/results.rs | 4 +- compiler/rustc_mir_dataflow/src/lib.rs | 6 +- 5 files changed, 174 insertions(+), 77 deletions(-) diff --git a/compiler/rustc_borrowck/src/dataflow.rs b/compiler/rustc_borrowck/src/dataflow.rs index 89ff12c1479ae..0c27f387e97f6 100644 --- a/compiler/rustc_borrowck/src/dataflow.rs +++ b/compiler/rustc_borrowck/src/dataflow.rs @@ -1,93 +1,171 @@ +use std::fmt; + use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::graph; use rustc_index::bit_set::BitSet; -use rustc_middle::mir::{self, BasicBlock, Body, Location, Place, TerminatorEdges}; +use rustc_middle::mir::{ + self, BasicBlock, Body, CallReturnPlaces, Location, Place, TerminatorEdges, +}; use rustc_middle::ty::{RegionVid, TyCtxt}; use rustc_mir_dataflow::fmt::DebugWithContext; use rustc_mir_dataflow::impls::{EverInitializedPlaces, MaybeUninitializedPlaces}; -use rustc_mir_dataflow::{Analysis, Forward, GenKill, Results, ResultsVisitable}; +use rustc_mir_dataflow::{Analysis, GenKill, JoinSemiLattice, SwitchIntEdgeEffects}; use tracing::debug; use crate::{BorrowSet, PlaceConflictBias, PlaceExt, RegionInferenceContext, places_conflict}; -/// The results of the dataflow analyses used by the borrow checker. -pub(crate) struct BorrowckResults<'a, 'tcx> { - pub(crate) borrows: Results<'tcx, Borrows<'a, 'tcx>>, - pub(crate) uninits: Results<'tcx, MaybeUninitializedPlaces<'a, 'tcx>>, - pub(crate) ever_inits: Results<'tcx, EverInitializedPlaces<'a, 'tcx>>, -} - -/// The transient state of the dataflow analyses used by the borrow checker. -#[derive(Debug)] -pub(crate) struct BorrowckDomain<'a, 'tcx> { - pub(crate) borrows: as Analysis<'tcx>>::Domain, - pub(crate) uninits: as Analysis<'tcx>>::Domain, - pub(crate) ever_inits: as Analysis<'tcx>>::Domain, +// This analysis is different to most others. Its results aren't computed with +// `iterate_to_fixpoint`, but are instead composed from the results of three sub-analyses that are +// computed individually with `iterate_to_fixpoint`. +pub(crate) struct Borrowck<'a, 'tcx> { + pub(crate) borrows: Borrows<'a, 'tcx>, + pub(crate) uninits: MaybeUninitializedPlaces<'a, 'tcx>, + pub(crate) ever_inits: EverInitializedPlaces<'a, 'tcx>, } -impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> { - type Direction = Forward; +impl<'a, 'tcx> Analysis<'tcx> for Borrowck<'a, 'tcx> { type Domain = BorrowckDomain<'a, 'tcx>; + const NAME: &'static str = "borrowck"; + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { BorrowckDomain { - borrows: self.borrows.analysis.bottom_value(body), - uninits: self.uninits.analysis.bottom_value(body), - ever_inits: self.ever_inits.analysis.bottom_value(body), + borrows: self.borrows.bottom_value(body), + uninits: self.uninits.bottom_value(body), + ever_inits: self.ever_inits.bottom_value(body), } } - fn reset_to_block_entry(&self, state: &mut Self::Domain, block: BasicBlock) { - state.borrows.clone_from(self.borrows.entry_set_for_block(block)); - state.uninits.clone_from(self.uninits.entry_set_for_block(block)); - state.ever_inits.clone_from(self.ever_inits.entry_set_for_block(block)); + fn initialize_start_block(&self, _body: &mir::Body<'tcx>, _state: &mut Self::Domain) { + // This is only reachable from `iterate_to_fixpoint`, which this analysis doesn't use. + unreachable!(); } - fn reconstruct_before_statement_effect( + fn apply_before_statement_effect( &mut self, state: &mut Self::Domain, stmt: &mir::Statement<'tcx>, loc: Location, ) { - self.borrows.analysis.apply_before_statement_effect(&mut state.borrows, stmt, loc); - self.uninits.analysis.apply_before_statement_effect(&mut state.uninits, stmt, loc); - self.ever_inits.analysis.apply_before_statement_effect(&mut state.ever_inits, stmt, loc); + self.borrows.apply_before_statement_effect(&mut state.borrows, stmt, loc); + self.uninits.apply_before_statement_effect(&mut state.uninits, stmt, loc); + self.ever_inits.apply_before_statement_effect(&mut state.ever_inits, stmt, loc); } - fn reconstruct_statement_effect( + fn apply_statement_effect( &mut self, state: &mut Self::Domain, stmt: &mir::Statement<'tcx>, loc: Location, ) { - self.borrows.analysis.apply_statement_effect(&mut state.borrows, stmt, loc); - self.uninits.analysis.apply_statement_effect(&mut state.uninits, stmt, loc); - self.ever_inits.analysis.apply_statement_effect(&mut state.ever_inits, stmt, loc); + self.borrows.apply_statement_effect(&mut state.borrows, stmt, loc); + self.uninits.apply_statement_effect(&mut state.uninits, stmt, loc); + self.ever_inits.apply_statement_effect(&mut state.ever_inits, stmt, loc); } - fn reconstruct_before_terminator_effect( + fn apply_before_terminator_effect( &mut self, state: &mut Self::Domain, term: &mir::Terminator<'tcx>, loc: Location, ) { - self.borrows.analysis.apply_before_terminator_effect(&mut state.borrows, term, loc); - self.uninits.analysis.apply_before_terminator_effect(&mut state.uninits, term, loc); - self.ever_inits.analysis.apply_before_terminator_effect(&mut state.ever_inits, term, loc); + self.borrows.apply_before_terminator_effect(&mut state.borrows, term, loc); + self.uninits.apply_before_terminator_effect(&mut state.uninits, term, loc); + self.ever_inits.apply_before_terminator_effect(&mut state.ever_inits, term, loc); } - fn reconstruct_terminator_effect( + fn apply_terminator_effect<'mir>( &mut self, state: &mut Self::Domain, - term: &mir::Terminator<'tcx>, + term: &'mir mir::Terminator<'tcx>, loc: Location, + ) -> TerminatorEdges<'mir, 'tcx> { + self.borrows.apply_terminator_effect(&mut state.borrows, term, loc); + self.uninits.apply_terminator_effect(&mut state.uninits, term, loc); + self.ever_inits.apply_terminator_effect(&mut state.ever_inits, term, loc); + + // This return value doesn't matter. It's only used by `iterate_to_fixpoint`, which this + // analysis doesn't use. + TerminatorEdges::None + } + + fn apply_call_return_effect( + &mut self, + _state: &mut Self::Domain, + _block: BasicBlock, + _return_places: CallReturnPlaces<'_, 'tcx>, + ) { + // This is only reachable from `iterate_to_fixpoint`, which this analysis doesn't use. + unreachable!(); + } + + fn apply_switch_int_edge_effects( + &mut self, + _block: BasicBlock, + _discr: &mir::Operand<'tcx>, + _apply_edge_effects: &mut impl SwitchIntEdgeEffects, ) { - self.borrows.analysis.apply_terminator_effect(&mut state.borrows, term, loc); - self.uninits.analysis.apply_terminator_effect(&mut state.uninits, term, loc); - self.ever_inits.analysis.apply_terminator_effect(&mut state.ever_inits, term, loc); + // This is only reachable from `iterate_to_fixpoint`, which this analysis doesn't use. + unreachable!(); } } +impl JoinSemiLattice for BorrowckDomain<'_, '_> { + fn join(&mut self, _other: &Self) -> bool { + // This is only reachable from `iterate_to_fixpoint`, which this analysis doesn't use. + unreachable!(); + } +} + +impl<'tcx, C> DebugWithContext for BorrowckDomain<'_, 'tcx> +where + C: rustc_mir_dataflow::move_paths::HasMoveData<'tcx>, +{ + fn fmt_with(&self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("borrows: ")?; + self.borrows.fmt_with(ctxt, f)?; + f.write_str(" uninits: ")?; + self.uninits.fmt_with(ctxt, f)?; + f.write_str(" ever_inits: ")?; + self.ever_inits.fmt_with(ctxt, f)?; + Ok(()) + } + + fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self == old { + return Ok(()); + } + + if self.borrows != old.borrows { + f.write_str("borrows: ")?; + self.borrows.fmt_diff_with(&old.borrows, ctxt, f)?; + f.write_str("\n")?; + } + + if self.uninits != old.uninits { + f.write_str("uninits: ")?; + self.uninits.fmt_diff_with(&old.uninits, ctxt, f)?; + f.write_str("\n")?; + } + + if self.ever_inits != old.ever_inits { + f.write_str("ever_inits: ")?; + self.ever_inits.fmt_diff_with(&old.ever_inits, ctxt, f)?; + f.write_str("\n")?; + } + + Ok(()) + } +} + +/// The transient state of the dataflow analyses used by the borrow checker. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct BorrowckDomain<'a, 'tcx> { + pub(crate) borrows: as Analysis<'tcx>>::Domain, + pub(crate) uninits: as Analysis<'tcx>>::Domain, + pub(crate) ever_inits: as Analysis<'tcx>>::Domain, +} + rustc_index::newtype_index! { #[orderable] #[debug_format = "bw{}"] diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index efcee2899c6f9..4946ab28e23cd 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -36,13 +36,13 @@ use rustc_middle::mir::*; use rustc_middle::query::Providers; use rustc_middle::ty::{self, ParamEnv, RegionVid, TyCtxt, TypingMode}; use rustc_middle::{bug, span_bug}; -use rustc_mir_dataflow::Analysis; use rustc_mir_dataflow::impls::{ EverInitializedPlaces, MaybeInitializedPlaces, MaybeUninitializedPlaces, }; use rustc_mir_dataflow::move_paths::{ InitIndex, InitLocation, LookupResult, MoveData, MoveOutIndex, MovePathIndex, }; +use rustc_mir_dataflow::{Analysis, EntrySets, Results}; use rustc_session::lint::builtin::UNUSED_MUT; use rustc_span::{Span, Symbol}; use rustc_target::abi::FieldIdx; @@ -82,7 +82,7 @@ mod util; pub mod consumers; use borrow_set::{BorrowData, BorrowSet}; -use dataflow::{BorrowIndex, BorrowckDomain, BorrowckResults, Borrows}; +use dataflow::{BorrowIndex, Borrowck, BorrowckDomain, Borrows}; use nll::PoloniusOutput; use place_ext::PlaceExt; use places_conflict::{PlaceConflictBias, places_conflict}; @@ -221,6 +221,10 @@ fn do_mir_borrowck<'tcx>( consumer_options, ); + // `flow_inits` is large, so we drop it as soon as possible. This reduces + // peak memory usage significantly on some benchmarks. + drop(flow_inits); + // Dump MIR results into a file, if that is enabled. This let us // write unit-tests, as well as helping with debugging. nll::dump_nll_mir(&infcx, body, ®ioncx, &opt_closure_req, &borrow_set); @@ -236,27 +240,6 @@ fn do_mir_borrowck<'tcx>( &mut diags, ); - // The various `flow_*` structures can be large. We drop `flow_inits` here - // so it doesn't overlap with the others below. This reduces peak memory - // usage significantly on some benchmarks. - drop(flow_inits); - - let flow_borrows = Borrows::new(tcx, body, ®ioncx, &borrow_set).iterate_to_fixpoint( - tcx, - body, - Some("borrowck"), - ); - let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &move_data).iterate_to_fixpoint( - tcx, - body, - Some("borrowck"), - ); - let flow_ever_inits = EverInitializedPlaces::new(body, &move_data).iterate_to_fixpoint( - tcx, - body, - Some("borrowck"), - ); - let movable_coroutine = // The first argument is the coroutine type passed by value if let Some(local) = body.local_decls.raw.get(1) @@ -342,16 +325,11 @@ fn do_mir_borrowck<'tcx>( // Compute and report region errors, if any. mbcx.report_region_errors(nll_errors); - let mut results = BorrowckResults { - ever_inits: flow_ever_inits, - uninits: flow_uninits, - borrows: flow_borrows, - }; - + let mut flow_results = get_flow_results(tcx, body, &move_data, &borrow_set, ®ioncx); rustc_mir_dataflow::visit_results( body, traversal::reverse_postorder(body).map(|(bb, _)| bb), - &mut results, + &mut flow_results, &mut mbcx, ); @@ -434,6 +412,47 @@ fn do_mir_borrowck<'tcx>( (result, body_with_facts) } +fn get_flow_results<'a, 'tcx>( + tcx: TyCtxt<'tcx>, + body: &'a Body<'tcx>, + move_data: &'a MoveData<'tcx>, + borrow_set: &'a BorrowSet<'tcx>, + regioncx: &RegionInferenceContext<'tcx>, +) -> Results<'tcx, Borrowck<'a, 'tcx>> { + // We compute these three analyses individually, but them combine them into + // a single results so that `mbcx` can visit them all together. + let borrows = Borrows::new(tcx, body, regioncx, borrow_set).iterate_to_fixpoint( + tcx, + body, + Some("borrowck"), + ); + let uninits = MaybeUninitializedPlaces::new(tcx, body, move_data).iterate_to_fixpoint( + tcx, + body, + Some("borrowck"), + ); + let ever_inits = EverInitializedPlaces::new(body, move_data).iterate_to_fixpoint( + tcx, + body, + Some("borrowck"), + ); + + let analysis = Borrowck { + borrows: borrows.analysis, + uninits: uninits.analysis, + ever_inits: ever_inits.analysis, + }; + + assert_eq!(borrows.entry_sets.len(), uninits.entry_sets.len()); + assert_eq!(borrows.entry_sets.len(), ever_inits.entry_sets.len()); + let entry_sets: EntrySets<'_, Borrowck<'_, '_>> = + itertools::izip!(borrows.entry_sets, uninits.entry_sets, ever_inits.entry_sets) + .map(|(borrows, uninits, ever_inits)| BorrowckDomain { borrows, uninits, ever_inits }) + .collect(); + + Results { analysis, entry_sets } +} + pub struct BorrowckInferCtxt<'tcx> { pub(crate) infcx: InferCtxt<'tcx>, pub(crate) reg_var_to_origin: RefCell>, diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index 8f81da8bb0499..4ad681935285d 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -55,7 +55,7 @@ mod visitor; pub use self::cursor::ResultsCursor; pub use self::direction::{Backward, Direction, Forward}; pub use self::lattice::{JoinSemiLattice, MaybeReachable}; -pub use self::results::Results; +pub use self::results::{EntrySets, Results}; pub use self::visitor::{ResultsVisitable, ResultsVisitor, visit_results}; /// Analysis domains are all bitsets of various kinds. This trait holds diff --git a/compiler/rustc_mir_dataflow/src/framework/results.rs b/compiler/rustc_mir_dataflow/src/framework/results.rs index 366fcbf33ba3c..2269c4ec22696 100644 --- a/compiler/rustc_mir_dataflow/src/framework/results.rs +++ b/compiler/rustc_mir_dataflow/src/framework/results.rs @@ -18,7 +18,7 @@ use crate::errors::{ DuplicateValuesFor, PathMustEndInFilename, RequiresAnArgument, UnknownFormatter, }; -type EntrySets<'tcx, A> = IndexVec>::Domain>; +pub type EntrySets<'tcx, A> = IndexVec>::Domain>; /// A dataflow analysis that has converged to fixpoint. #[derive(Clone)] @@ -27,7 +27,7 @@ where A: Analysis<'tcx>, { pub analysis: A, - pub(super) entry_sets: EntrySets<'tcx, A>, + pub entry_sets: EntrySets<'tcx, A>, } impl<'tcx, A> Results<'tcx, A> diff --git a/compiler/rustc_mir_dataflow/src/lib.rs b/compiler/rustc_mir_dataflow/src/lib.rs index b404e3bfb72c3..8ebe7d593094e 100644 --- a/compiler/rustc_mir_dataflow/src/lib.rs +++ b/compiler/rustc_mir_dataflow/src/lib.rs @@ -18,9 +18,9 @@ pub use self::drop_flag_effects::{ move_path_children_matching, on_all_children_bits, on_lookup_result_bits, }; pub use self::framework::{ - Analysis, Backward, Direction, Forward, GenKill, JoinSemiLattice, MaybeReachable, Results, - ResultsCursor, ResultsVisitable, ResultsVisitor, SwitchIntEdgeEffects, fmt, graphviz, lattice, - visit_results, + Analysis, Backward, Direction, EntrySets, Forward, GenKill, JoinSemiLattice, MaybeReachable, + Results, ResultsCursor, ResultsVisitable, ResultsVisitor, SwitchIntEdgeEffects, fmt, graphviz, + lattice, visit_results, }; use self::move_paths::MoveData;