From 45197887cb7bd7fd3148249f49e396de625d9a25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 27 May 2025 20:18:39 +0000 Subject: [PATCH 1/5] move `MaybeInitializedPlaces` computation to where it's used This dataflow analysis is only used by `liveness::trace`. We move it there to make it lazy. --- compiler/rustc_borrowck/src/lib.rs | 9 +-------- compiler/rustc_borrowck/src/nll.rs | 6 +----- compiler/rustc_borrowck/src/type_check/liveness/mod.rs | 7 ++----- compiler/rustc_borrowck/src/type_check/liveness/trace.rs | 8 +++++--- compiler/rustc_borrowck/src/type_check/mod.rs | 8 ++------ 5 files changed, 11 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 676cb618b725b..2bd0ffd143be8 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -40,9 +40,7 @@ use rustc_middle::ty::{ self, ParamEnv, RegionVid, Ty, TyCtxt, TypeFoldable, TypeVisitable, TypingMode, fold_regions, }; use rustc_middle::{bug, span_bug}; -use rustc_mir_dataflow::impls::{ - EverInitializedPlaces, MaybeInitializedPlaces, MaybeUninitializedPlaces, -}; +use rustc_mir_dataflow::impls::{EverInitializedPlaces, MaybeUninitializedPlaces}; use rustc_mir_dataflow::move_paths::{ InitIndex, InitLocation, LookupResult, MoveData, MovePathIndex, }; @@ -324,10 +322,6 @@ fn do_mir_borrowck<'tcx>( let move_data = MoveData::gather_moves(body, tcx, |_| true); - let flow_inits = MaybeInitializedPlaces::new(tcx, body, &move_data) - .iterate_to_fixpoint(tcx, body, Some("borrowck")) - .into_results_cursor(body); - let locals_are_invalidated_at_exit = tcx.hir_body_owner_kind(def).is_fn_or_closure(); let borrow_set = BorrowSet::build(tcx, body, locals_are_invalidated_at_exit, &move_data); @@ -346,7 +340,6 @@ fn do_mir_borrowck<'tcx>( body, &promoted, &location_table, - flow_inits, &move_data, &borrow_set, consumer_options, diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index fe899bb054fa9..8664e99cae366 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -11,8 +11,6 @@ use rustc_middle::mir::pretty::{PrettyPrintMirOptions, dump_mir_with_options}; use rustc_middle::mir::{Body, PassWhere, Promoted, create_dump_file, dump_enabled, dump_mir}; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{self, TyCtxt}; -use rustc_mir_dataflow::ResultsCursor; -use rustc_mir_dataflow::impls::MaybeInitializedPlaces; use rustc_mir_dataflow::move_paths::MoveData; use rustc_mir_dataflow::points::DenseLocationMap; use rustc_session::config::MirIncludeSpans; @@ -75,14 +73,13 @@ pub(crate) fn replace_regions_in_mir<'tcx>( /// Computes the (non-lexical) regions from the input MIR. /// /// This may result in errors being reported. -pub(crate) fn compute_regions<'a, 'tcx>( +pub(crate) fn compute_regions<'tcx>( root_cx: &mut BorrowCheckRootCtxt<'tcx>, infcx: &BorrowckInferCtxt<'tcx>, universal_regions: UniversalRegions<'tcx>, body: &Body<'tcx>, promoted: &IndexSlice>, location_table: &PoloniusLocationTable, - flow_inits: ResultsCursor<'a, 'tcx, MaybeInitializedPlaces<'a, 'tcx>>, move_data: &MoveData<'tcx>, borrow_set: &BorrowSet<'tcx>, consumer_options: Option, @@ -112,7 +109,6 @@ pub(crate) fn compute_regions<'a, 'tcx>( location_table, borrow_set, &mut polonius_facts, - flow_inits, move_data, Rc::clone(&location_map), ); diff --git a/compiler/rustc_borrowck/src/type_check/liveness/mod.rs b/compiler/rustc_borrowck/src/type_check/liveness/mod.rs index b7a21cf48c8f7..ca1b850f7665d 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/mod.rs @@ -5,8 +5,6 @@ use rustc_middle::mir::{Body, Local, Location, SourceInfo}; use rustc_middle::span_bug; use rustc_middle::ty::relate::Relate; use rustc_middle::ty::{GenericArgsRef, Region, RegionVid, Ty, TyCtxt, TypeVisitable}; -use rustc_mir_dataflow::ResultsCursor; -use rustc_mir_dataflow::impls::MaybeInitializedPlaces; use rustc_mir_dataflow::move_paths::MoveData; use rustc_mir_dataflow::points::DenseLocationMap; use tracing::debug; @@ -28,10 +26,9 @@ mod trace; /// /// N.B., this computation requires normalization; therefore, it must be /// performed before -pub(super) fn generate<'a, 'tcx>( +pub(super) fn generate<'tcx>( typeck: &mut TypeChecker<'_, 'tcx>, location_map: &DenseLocationMap, - flow_inits: ResultsCursor<'a, 'tcx, MaybeInitializedPlaces<'a, 'tcx>>, move_data: &MoveData<'tcx>, ) { debug!("liveness::generate"); @@ -58,7 +55,7 @@ pub(super) fn generate<'a, 'tcx>( let (relevant_live_locals, boring_locals) = compute_relevant_live_locals(typeck.tcx(), &free_regions, typeck.body); - trace::trace(typeck, location_map, flow_inits, move_data, relevant_live_locals, boring_locals); + trace::trace(typeck, location_map, move_data, relevant_live_locals, boring_locals); // Mark regions that should be live where they appear within rvalues or within a call: like // args, regions, and types. diff --git a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs index 512288a0f7d85..9c00628f71b45 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs @@ -7,10 +7,10 @@ use rustc_middle::mir::{BasicBlock, Body, ConstraintCategory, HasLocalDecls, Loc use rustc_middle::traits::query::DropckOutlivesResult; use rustc_middle::ty::relate::Relate; use rustc_middle::ty::{Ty, TyCtxt, TypeVisitable, TypeVisitableExt}; -use rustc_mir_dataflow::ResultsCursor; use rustc_mir_dataflow::impls::MaybeInitializedPlaces; use rustc_mir_dataflow::move_paths::{HasMoveData, MoveData, MovePathIndex}; use rustc_mir_dataflow::points::{DenseLocationMap, PointIndex}; +use rustc_mir_dataflow::{Analysis, ResultsCursor}; use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span}; use rustc_trait_selection::error_reporting::InferCtxtErrorExt; use rustc_trait_selection::traits::ObligationCtxt; @@ -37,15 +37,17 @@ use crate::type_check::{NormalizeLocation, TypeChecker}; /// DROP-LIVE set are to the liveness sets for regions found in the /// `dropck_outlives` result of the variable's type (in particular, /// this respects `#[may_dangle]` annotations). -pub(super) fn trace<'a, 'tcx>( +pub(super) fn trace<'tcx>( typeck: &mut TypeChecker<'_, 'tcx>, location_map: &DenseLocationMap, - flow_inits: ResultsCursor<'a, 'tcx, MaybeInitializedPlaces<'a, 'tcx>>, move_data: &MoveData<'tcx>, relevant_live_locals: Vec, boring_locals: Vec, ) { let local_use_map = &LocalUseMap::build(&relevant_live_locals, location_map, typeck.body); + let flow_inits = MaybeInitializedPlaces::new(typeck.tcx(), typeck.body, move_data) + .iterate_to_fixpoint(typeck.tcx(), typeck.body, Some("borrowck")) + .into_results_cursor(typeck.body); let cx = LivenessContext { typeck, flow_inits, diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 8c51225712093..9dff03a8a709a 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -30,8 +30,6 @@ use rustc_middle::ty::{ TypeVisitableExt, UserArgs, UserTypeAnnotationIndex, fold_regions, }; use rustc_middle::{bug, span_bug}; -use rustc_mir_dataflow::ResultsCursor; -use rustc_mir_dataflow::impls::MaybeInitializedPlaces; use rustc_mir_dataflow::move_paths::MoveData; use rustc_mir_dataflow::points::DenseLocationMap; use rustc_span::def_id::CRATE_DEF_ID; @@ -97,10 +95,9 @@ mod relate_tys; /// - `location_table` -- for datalog polonius, the map between `Location`s and `RichLocation`s /// - `borrow_set` -- information about borrows occurring in `body` /// - `polonius_facts` -- when using Polonius, this is the generated set of Polonius facts -/// - `flow_inits` -- results of a maybe-init dataflow analysis /// - `move_data` -- move-data constructed when performing the maybe-init dataflow analysis /// - `location_map` -- map between MIR `Location` and `PointIndex` -pub(crate) fn type_check<'a, 'tcx>( +pub(crate) fn type_check<'tcx>( root_cx: &mut BorrowCheckRootCtxt<'tcx>, infcx: &BorrowckInferCtxt<'tcx>, body: &Body<'tcx>, @@ -109,7 +106,6 @@ pub(crate) fn type_check<'a, 'tcx>( location_table: &PoloniusLocationTable, borrow_set: &BorrowSet<'tcx>, polonius_facts: &mut Option, - flow_inits: ResultsCursor<'a, 'tcx, MaybeInitializedPlaces<'a, 'tcx>>, move_data: &MoveData<'tcx>, location_map: Rc, ) -> MirTypeckResults<'tcx> { @@ -167,7 +163,7 @@ pub(crate) fn type_check<'a, 'tcx>( typeck.equate_inputs_and_outputs(&normalized_inputs_and_output); typeck.check_signature_annotation(); - liveness::generate(&mut typeck, &location_map, flow_inits, move_data); + liveness::generate(&mut typeck, &location_map, move_data); let opaque_type_values = opaque_types::take_opaques_and_register_member_constraints(&mut typeck); From 10d39f52a94e25181609a415742a43cbccebc51e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 27 May 2025 20:21:28 +0000 Subject: [PATCH 2/5] remove unneeded lifetime --- .../src/type_check/liveness/trace.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs index 9c00628f71b45..151c1ef085044 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs @@ -67,7 +67,7 @@ pub(super) fn trace<'tcx>( } /// Contextual state for the type-liveness coroutine. -struct LivenessContext<'a, 'typeck, 'b, 'tcx> { +struct LivenessContext<'a, 'typeck, 'tcx> { /// Current type-checker, giving us our inference context etc. /// /// This also stores the body we're currently analyzing. @@ -84,7 +84,7 @@ struct LivenessContext<'a, 'typeck, 'b, 'tcx> { /// Results of dataflow tracking which variables (and paths) have been /// initialized. - flow_inits: ResultsCursor<'b, 'tcx, MaybeInitializedPlaces<'b, 'tcx>>, + flow_inits: ResultsCursor<'a, 'tcx, MaybeInitializedPlaces<'a, 'tcx>>, /// Index indicating where each variable is assigned, used, or /// dropped. @@ -96,8 +96,8 @@ struct DropData<'tcx> { region_constraint_data: Option<&'tcx QueryRegionConstraints<'tcx>>, } -struct LivenessResults<'a, 'typeck, 'b, 'tcx> { - cx: LivenessContext<'a, 'typeck, 'b, 'tcx>, +struct LivenessResults<'a, 'typeck, 'tcx> { + cx: LivenessContext<'a, 'typeck, 'tcx>, /// Set of points that define the current local. defs: DenseBitSet, @@ -118,8 +118,8 @@ struct LivenessResults<'a, 'typeck, 'b, 'tcx> { stack: Vec, } -impl<'a, 'typeck, 'b, 'tcx> LivenessResults<'a, 'typeck, 'b, 'tcx> { - fn new(cx: LivenessContext<'a, 'typeck, 'b, 'tcx>) -> Self { +impl<'a, 'typeck, 'tcx> LivenessResults<'a, 'typeck, 'tcx> { + fn new(cx: LivenessContext<'a, 'typeck, 'tcx>) -> Self { let num_points = cx.location_map.num_points(); LivenessResults { cx, @@ -461,10 +461,11 @@ impl<'a, 'typeck, 'b, 'tcx> LivenessResults<'a, 'typeck, 'b, 'tcx> { } } -impl<'tcx> LivenessContext<'_, '_, '_, 'tcx> { +impl<'tcx> LivenessContext<'_, '_, 'tcx> { fn body(&self) -> &Body<'tcx> { self.typeck.body } + /// Returns `true` if the local variable (or some part of it) is initialized at the current /// cursor position. Callers should call one of the `seek` methods immediately before to point /// the cursor to the desired location. From 430e23044914f759f5674cfae542a366f6cca7e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 27 May 2025 20:55:58 +0000 Subject: [PATCH 3/5] fast path: compute `MaybeInitializedPlaces` lazily Only drop-liveness checks for maybe-initializedness of move paths, and it does so only for the relevant live locals that have drop points. This adds a fast path by computing this dataflow analysis only when checking for such initializedness. This avoids this expensive computation for the common case. For example, it avoids computing initializedness for 20K locals in the `cranelift-codegen` benchmark, it has 7K relevant live locals but none with drop points. That saves 900ms on end-to-end compilation times. --- .../src/type_check/liveness/trace.rs | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs index 151c1ef085044..cf7bd9751e190 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs @@ -45,12 +45,9 @@ pub(super) fn trace<'tcx>( boring_locals: Vec, ) { let local_use_map = &LocalUseMap::build(&relevant_live_locals, location_map, typeck.body); - let flow_inits = MaybeInitializedPlaces::new(typeck.tcx(), typeck.body, move_data) - .iterate_to_fixpoint(typeck.tcx(), typeck.body, Some("borrowck")) - .into_results_cursor(typeck.body); let cx = LivenessContext { typeck, - flow_inits, + flow_inits: None, location_map, local_use_map, move_data, @@ -83,8 +80,8 @@ struct LivenessContext<'a, 'typeck, 'tcx> { drop_data: FxIndexMap, DropData<'tcx>>, /// Results of dataflow tracking which variables (and paths) have been - /// initialized. - flow_inits: ResultsCursor<'a, 'tcx, MaybeInitializedPlaces<'a, 'tcx>>, + /// initialized. Computed lazily when needed by drop-liveness. + flow_inits: Option>>, /// Index indicating where each variable is assigned, used, or /// dropped. @@ -461,6 +458,28 @@ impl<'a, 'typeck, 'tcx> LivenessResults<'a, 'typeck, 'tcx> { } } +impl<'a, 'typeck, 'tcx> LivenessContext<'a, 'typeck, 'tcx> { + /// Computes the `MaybeInitializedPlaces` dataflow analysis if it hasn't been done already. + /// + /// In practice, the results of this dataflow analysis are rarely needed but can be expensive to + /// compute on big functions, so we compute them lazily as a fast path when: + /// - there are relevant live locals + /// - there are drop points for these relevant live locals. + /// + /// This happens as part of the drop-liveness computation: it's the only place checking for + /// maybe-initializedness of `MovePathIndex`es. + fn flow_inits(&mut self) -> &mut ResultsCursor<'a, 'tcx, MaybeInitializedPlaces<'a, 'tcx>> { + self.flow_inits.get_or_insert_with(|| { + let tcx = self.typeck.tcx(); + let body = self.typeck.body; + let flow_inits = MaybeInitializedPlaces::new(tcx, body, self.move_data) + .iterate_to_fixpoint(tcx, body, Some("borrowck")) + .into_results_cursor(body); + flow_inits + }) + } +} + impl<'tcx> LivenessContext<'_, '_, 'tcx> { fn body(&self) -> &Body<'tcx> { self.typeck.body @@ -469,13 +488,14 @@ impl<'tcx> LivenessContext<'_, '_, 'tcx> { /// Returns `true` if the local variable (or some part of it) is initialized at the current /// cursor position. Callers should call one of the `seek` methods immediately before to point /// the cursor to the desired location. - fn initialized_at_curr_loc(&self, mpi: MovePathIndex) -> bool { - let state = self.flow_inits.get(); + fn initialized_at_curr_loc(&mut self, mpi: MovePathIndex) -> bool { + let flow_inits = self.flow_inits(); + let state = flow_inits.get(); if state.contains(mpi) { return true; } - let move_paths = &self.flow_inits.analysis().move_data().move_paths; + let move_paths = &flow_inits.analysis().move_data().move_paths; move_paths[mpi].find_descendant(move_paths, |mpi| state.contains(mpi)).is_some() } @@ -484,7 +504,8 @@ impl<'tcx> LivenessContext<'_, '_, 'tcx> { /// DROP of some local variable will have an effect -- note that /// drops, as they may unwind, are always terminators. fn initialized_at_terminator(&mut self, block: BasicBlock, mpi: MovePathIndex) -> bool { - self.flow_inits.seek_before_primary_effect(self.body().terminator_loc(block)); + let terminator_location = self.body().terminator_loc(block); + self.flow_inits().seek_before_primary_effect(terminator_location); self.initialized_at_curr_loc(mpi) } @@ -494,7 +515,8 @@ impl<'tcx> LivenessContext<'_, '_, 'tcx> { /// **Warning:** Does not account for the result of `Call` /// instructions. fn initialized_at_exit(&mut self, block: BasicBlock, mpi: MovePathIndex) -> bool { - self.flow_inits.seek_after_primary_effect(self.body().terminator_loc(block)); + let terminator_location = self.body().terminator_loc(block); + self.flow_inits().seek_after_primary_effect(terminator_location); self.initialized_at_curr_loc(mpi) } From 703e051f36013e620eece08151fd3f6e435fd278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 27 May 2025 21:17:01 +0000 Subject: [PATCH 4/5] add perf fixme for `MaybeInitializedPlaces` domain --- .../rustc_borrowck/src/type_check/liveness/trace.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs index cf7bd9751e190..5d30fa71e92c5 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs @@ -472,6 +472,18 @@ impl<'a, 'typeck, 'tcx> LivenessContext<'a, 'typeck, 'tcx> { self.flow_inits.get_or_insert_with(|| { let tcx = self.typeck.tcx(); let body = self.typeck.body; + // FIXME: reduce the `MaybeInitializedPlaces` domain to the useful `MovePath`s. + // + // This dataflow analysis computes maybe-initializedness of all move paths, which + // explains why it can be expensive on big functions. But this data is only used in + // drop-liveness. Therefore, most of the move paths computed here are ultimately unused, + // even if the results are computed lazily and "no relevant live locals with drop + // points" is the common case. + // + // So we only need the ones for 1) relevant live locals 2) that have drop points. That's + // a much, much smaller domain: in our benchmarks, when it's not zero (the most likely + // case), there are a few dozens compared to e.g. thousands or tens of thousands of + // locals and move paths. let flow_inits = MaybeInitializedPlaces::new(tcx, body, self.move_data) .iterate_to_fixpoint(tcx, body, Some("borrowck")) .into_results_cursor(body); From ddeae148c511d9a0c56f0a442dbefe3fa73213c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 27 May 2025 08:42:00 +0000 Subject: [PATCH 5/5] switch dataflow test to a maybe-uninit analysis This mir-opt test used the maybe-init dataflow analysis but it's now lazy and doesn't emit the graphviz graph when it's not needed anymore. --- tests/mir-opt/dataflow.main.maybe_init.borrowck.dot | 6 ------ tests/mir-opt/dataflow.main.maybe_uninit.borrowck.dot | 6 ++++++ tests/mir-opt/dataflow.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) delete mode 100644 tests/mir-opt/dataflow.main.maybe_init.borrowck.dot create mode 100644 tests/mir-opt/dataflow.main.maybe_uninit.borrowck.dot diff --git a/tests/mir-opt/dataflow.main.maybe_init.borrowck.dot b/tests/mir-opt/dataflow.main.maybe_init.borrowck.dot deleted file mode 100644 index 7c7d8921fb366..0000000000000 --- a/tests/mir-opt/dataflow.main.maybe_init.borrowck.dot +++ /dev/null @@ -1,6 +0,0 @@ -digraph graph_for_def_id_0_3 { - graph[fontname="Courier, monospace"]; - node[fontname="Courier, monospace"]; - edge[fontname="Courier, monospace"]; - bb_0[label=<
bb0
MIRSTATE
(on start){}
0_0 = const ()+_0
Treturn
(on end){_0}
>][shape="none"]; -} diff --git a/tests/mir-opt/dataflow.main.maybe_uninit.borrowck.dot b/tests/mir-opt/dataflow.main.maybe_uninit.borrowck.dot new file mode 100644 index 0000000000000..258404b8da782 --- /dev/null +++ b/tests/mir-opt/dataflow.main.maybe_uninit.borrowck.dot @@ -0,0 +1,6 @@ +digraph graph_for_def_id_0_3 { + graph[fontname="Courier, monospace"]; + node[fontname="Courier, monospace"]; + edge[fontname="Courier, monospace"]; + bb_0[label=<
bb0
MIRSTATE
(on start){_0}
0_0 = const ()-_0
Treturn
(on end){}
>][shape="none"]; +} diff --git a/tests/mir-opt/dataflow.rs b/tests/mir-opt/dataflow.rs index 3a28f5d47b9a5..5ed3da4c531a8 100644 --- a/tests/mir-opt/dataflow.rs +++ b/tests/mir-opt/dataflow.rs @@ -2,5 +2,5 @@ // Test graphviz dataflow output //@ compile-flags: -Z dump-mir=main -Z dump-mir-dataflow -// EMIT_MIR dataflow.main.maybe_init.borrowck.dot +// EMIT_MIR dataflow.main.maybe_uninit.borrowck.dot fn main() {}