From 9fb6c16e9cbf10b1a8d0e1d910221f25b4e7521f Mon Sep 17 00:00:00 2001 From: b-naber Date: Thu, 29 Jun 2023 19:14:18 +0000 Subject: [PATCH] make BorrowedLocalsResultsCursor compatible with #108293 --- .../src/impls/live_borrows.rs | 89 ++++++++----------- .../src/impls/storage_liveness.rs | 23 +++-- compiler/rustc_mir_transform/src/generator.rs | 24 +++-- 3 files changed, 59 insertions(+), 77 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/impls/live_borrows.rs b/compiler/rustc_mir_dataflow/src/impls/live_borrows.rs index df54d71531cd6..4f64bb4178e3b 100644 --- a/compiler/rustc_mir_dataflow/src/impls/live_borrows.rs +++ b/compiler/rustc_mir_dataflow/src/impls/live_borrows.rs @@ -242,11 +242,9 @@ use super::*; -use crate::framework::{Analysis, Results, ResultsCursor}; +use crate::framework::{Analysis, Results, ResultsClonedCursor, ResultsCursor}; use crate::impls::MaybeBorrowedLocals; -use crate::{ - AnalysisDomain, Backward, CallReturnPlaces, GenKill, GenKillAnalysis, ResultsRefCursor, -}; +use crate::{AnalysisDomain, Backward, CallReturnPlaces, CloneAnalysis, GenKill, GenKillAnalysis}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::graph; use rustc_data_structures::graph::implementation::{Graph, NodeIndex}; @@ -257,7 +255,6 @@ use rustc_middle::mir::*; use rustc_middle::ty::TypeVisitable; use rustc_middle::ty::{self, Ty, TypeSuperVisitable}; -use std::cell::RefCell; use std::ops::{ControlFlow, Deref, DerefMut}; // FIXME Properly determine a reasonable value @@ -890,7 +887,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BorrowDependencies<'a, 'tcx> { } } -pub struct BorrowedLocalsResults<'a, 'mir, 'tcx> { +pub struct BorrowedLocalsResults<'mir, 'tcx> { /// the results of the liveness analysis of `LiveBorrows` borrows_analysis_results: Results<'tcx, LiveBorrows<'mir, 'tcx>>, @@ -900,36 +897,26 @@ pub struct BorrowedLocalsResults<'a, 'mir, 'tcx> { /// to the set of `Local`s that are borrowed through those references, pointers or composite values. borrowed_local_to_locals_to_keep_alive: FxHashMap>, - /// The results cursor of the `MaybeBorrowedLocals` analysis. Needed as an upper bound, since + /// The results of the `MaybeBorrowedLocals` analysis. Needed as an upper bound, since /// to ensure soundness the `LiveBorrows` analysis would keep more `Local`s alive than /// strictly necessary. - maybe_borrowed_locals_results_cursor: RefCell< - ResultsCursor<'mir, 'tcx, MaybeBorrowedLocals, &'a Results<'tcx, MaybeBorrowedLocals>>, - >, + maybe_borrowed_locals_results: Results<'tcx, MaybeBorrowedLocals>, } -impl<'a, 'mir, 'tcx> BorrowedLocalsResults<'a, 'mir, 'tcx> +impl<'mir, 'tcx> BorrowedLocalsResults<'mir, 'tcx> where 'tcx: 'mir, - 'tcx: 'a, { fn new( borrows_analysis_results: Results<'tcx, LiveBorrows<'mir, 'tcx>>, - maybe_borrowed_locals_results_cursor: ResultsCursor< - 'mir, - 'tcx, - MaybeBorrowedLocals, - &'a Results<'tcx, MaybeBorrowedLocals>, - >, + maybe_borrowed_locals_results: Results<'tcx, MaybeBorrowedLocals>, dep_graph: BorrowDepGraph, ) -> Self { let borrowed_local_to_locals_to_keep_alive = Self::get_locals_to_keep_alive_map(dep_graph); Self { borrows_analysis_results, borrowed_local_to_locals_to_keep_alive, - maybe_borrowed_locals_results_cursor: RefCell::new( - maybe_borrowed_locals_results_cursor, - ), + maybe_borrowed_locals_results, } } @@ -1051,17 +1038,12 @@ where /// The function gets the results of the borrowed locals analysis in this module. See the module /// doc-comment for information on what exactly this analysis does. -#[instrument(skip(tcx, maybe_borrowed_locals_cursor, body), level = "debug")] -pub fn get_borrowed_locals_results<'a, 'mir, 'tcx>( +#[instrument(skip(tcx, maybe_borrowed_locals, body), level = "debug")] +pub fn get_borrowed_locals_results<'mir, 'tcx>( body: &'mir Body<'tcx>, tcx: TyCtxt<'tcx>, - maybe_borrowed_locals_cursor: ResultsCursor< - 'mir, - 'tcx, - MaybeBorrowedLocals, - &'a Results<'tcx, MaybeBorrowedLocals>, - >, -) -> BorrowedLocalsResults<'a, 'mir, 'tcx> { + maybe_borrowed_locals: Results<'tcx, MaybeBorrowedLocals>, +) -> BorrowedLocalsResults<'mir, 'tcx> { debug!("body: {:#?}", body); let mut borrow_deps = BorrowDependencies::new(body.local_decls(), tcx); @@ -1111,11 +1093,7 @@ pub fn get_borrowed_locals_results<'a, 'mir, 'tcx>( let live_borrows_results = live_borrows.into_engine(tcx, body).pass_name("borrowed_locals").iterate_to_fixpoint(); - BorrowedLocalsResults::new( - live_borrows_results, - maybe_borrowed_locals_cursor, - borrow_deps.dep_graph, - ) + BorrowedLocalsResults::new(live_borrows_results, maybe_borrowed_locals, borrow_deps.dep_graph) } /// The `ResultsCursor` equivalent for the borrowed locals analysis. Since this analysis doesn't @@ -1123,7 +1101,16 @@ pub fn get_borrowed_locals_results<'a, 'mir, 'tcx>( /// the `get` method without the need for any prior 'seek' calls. pub struct BorrowedLocalsResultsCursor<'a, 'mir, 'tcx> { // The cursor for the liveness analysis performed by `LiveBorrows` - borrows_analysis_cursor: ResultsRefCursor<'a, 'mir, 'tcx, LiveBorrows<'mir, 'tcx>>, + borrows_analysis_cursor: ResultsCursor< + 'mir, + 'tcx, + LiveBorrows<'mir, 'tcx>, + Results< + 'tcx, + LiveBorrows<'mir, 'tcx>, + &'a rustc_index::IndexVec>, + >, + >, // Maps each `Local` corresponding to a reference or pointer to the set of `Local`s // that are borrowed through the ref/ptr. Additionally contains entries for `Local`s @@ -1132,14 +1119,13 @@ pub struct BorrowedLocalsResultsCursor<'a, 'mir, 'tcx> { borrowed_local_to_locals_to_keep_alive: &'a FxHashMap>, // the cursor of the conservative borrowed locals analysis - maybe_borrowed_locals_results_cursor: &'a RefCell< - ResultsCursor<'mir, 'tcx, MaybeBorrowedLocals, &'a Results<'tcx, MaybeBorrowedLocals>>, - >, + maybe_borrowed_locals_results_cursor: ResultsClonedCursor<'a, 'mir, 'tcx, MaybeBorrowedLocals>, } impl<'a, 'mir, 'tcx> BorrowedLocalsResultsCursor<'a, 'mir, 'tcx> { - pub fn new(body: &'mir Body<'tcx>, results: &'a BorrowedLocalsResults<'a, 'mir, 'tcx>) -> Self { - let mut cursor = ResultsCursor::new(body, &results.borrows_analysis_results); + pub fn new(body: &'mir Body<'tcx>, results: &'a BorrowedLocalsResults<'mir, 'tcx>) -> Self { + let mut cursor = + ResultsCursor::new(body, results.borrows_analysis_results.clone_analysis()); // We don't care about the order of the blocks, only about the result at a given location. // This statement is necessary since we're performing a backward analysis in `LiveBorrows`, @@ -1149,7 +1135,10 @@ impl<'a, 'mir, 'tcx> BorrowedLocalsResultsCursor<'a, 'mir, 'tcx> { Self { borrows_analysis_cursor: cursor, borrowed_local_to_locals_to_keep_alive: &results.borrowed_local_to_locals_to_keep_alive, - maybe_borrowed_locals_results_cursor: &results.maybe_borrowed_locals_results_cursor, + maybe_borrowed_locals_results_cursor: ResultsClonedCursor::new( + body, + results.maybe_borrowed_locals_results.clone_analysis(), + ), } } @@ -1178,11 +1167,9 @@ impl<'a, 'mir, 'tcx> BorrowedLocalsResultsCursor<'a, 'mir, 'tcx> { // use results of conservative analysis as an "upper bound" on the borrowed locals. This // is necessary since to guarantee soundness for this analysis we would have to keep // more `Local`s alive than strictly necessary. - let mut maybe_borrowed_locals_cursor = - self.maybe_borrowed_locals_results_cursor.borrow_mut(); - maybe_borrowed_locals_cursor.allow_unreachable(); - maybe_borrowed_locals_cursor.seek_before_primary_effect(loc); - let upper_bound_borrowed_locals = maybe_borrowed_locals_cursor.get(); + self.maybe_borrowed_locals_results_cursor.allow_unreachable(); + self.maybe_borrowed_locals_results_cursor.seek_before_primary_effect(loc); + let upper_bound_borrowed_locals = self.maybe_borrowed_locals_results_cursor.get(); borrowed_locals.intersect(upper_bound_borrowed_locals); debug!(?borrowed_locals); @@ -1225,7 +1212,7 @@ impl<'mir, 'tcx> LiveBorrows<'mir, 'tcx> { } } -impl<'mir, 'tcx> crate::CloneAnalysis for LiveBorrows<'mir, 'tcx> { +impl<'mir, 'tcx> CloneAnalysis for LiveBorrows<'mir, 'tcx> { fn clone_analysis(&self) -> Self { self.clone() } @@ -1251,7 +1238,7 @@ impl<'a, 'tcx> GenKillAnalysis<'tcx> for LiveBorrows<'a, 'tcx> { #[instrument(skip(self, trans), level = "debug")] fn statement_effect( - &self, + &mut self, trans: &mut impl GenKill, statement: &mir::Statement<'tcx>, location: Location, @@ -1261,7 +1248,7 @@ impl<'a, 'tcx> GenKillAnalysis<'tcx> for LiveBorrows<'a, 'tcx> { #[instrument(skip(self, trans), level = "debug")] fn terminator_effect( - &self, + &mut self, trans: &mut impl GenKill, terminator: &mir::Terminator<'tcx>, location: Location, @@ -1270,7 +1257,7 @@ impl<'a, 'tcx> GenKillAnalysis<'tcx> for LiveBorrows<'a, 'tcx> { } fn call_return_effect( - &self, + &mut self, _trans: &mut impl GenKill, _block: mir::BasicBlock, _return_places: CallReturnPlaces<'_, 'tcx>, diff --git a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs index 90e13187a38b0..31aae40eac89d 100644 --- a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs @@ -1,6 +1,6 @@ pub use super::*; -use crate::impls::{BorrowedLocalsResults, BorrowedLocalsResultsCursor}; +use crate::impls::BorrowedLocalsResultsCursor; use crate::{CallReturnPlaces, GenKill}; use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::*; @@ -151,18 +151,15 @@ impl<'tcx> crate::GenKillAnalysis<'tcx> for MaybeStorageDead { /// given location; i.e. whether its storage can go away without being observed. pub struct MaybeRequiresStorage<'a, 'mir, 'tcx> { body: &'mir Body<'tcx>, - borrowed_locals: RefCell>, + borrowed_locals_cursor: BorrowedLocalsResultsCursor<'a, 'mir, 'tcx>, } impl<'a, 'mir, 'tcx> MaybeRequiresStorage<'a, 'mir, 'tcx> { pub fn new( body: &'mir Body<'tcx>, - borrowed_locals: &'a BorrowedLocalsResults<'a, 'mir, 'tcx>, + borrowed_locals_cursor: BorrowedLocalsResultsCursor<'a, 'mir, 'tcx>, ) -> Self { - MaybeRequiresStorage { - body, - borrowed_locals: RefCell::new(BorrowedLocalsResultsCursor::new(body, borrowed_locals)), - } + MaybeRequiresStorage { body, borrowed_locals_cursor } } } @@ -195,7 +192,7 @@ impl<'a, 'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'a, ' loc: Location, ) { // If a place is borrowed in a statement, it needs storage for that statement. - let borrowed_locals_at_loc = self.borrowed_locals.borrow_mut().get(loc); + let borrowed_locals_at_loc = self.borrowed_locals_cursor.get(loc); for i in 0..self.body.local_decls().len() { let local = Local::from_usize(i); if borrowed_locals_at_loc.contains(local) { @@ -246,7 +243,7 @@ impl<'a, 'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'a, ' loc: Location, ) { // If a place is borrowed in a statement, it needs storage for that statement. - let borrowed_locals_at_loc = self.borrowed_locals.borrow_mut().get(loc); + let borrowed_locals_at_loc = self.borrowed_locals_cursor.get(loc); for i in 0..self.body.local_decls().len() { let local = Local::from_usize(i); if borrowed_locals_at_loc.contains(local) { @@ -359,14 +356,14 @@ impl<'a, 'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'a, ' impl<'a, 'mir, 'tcx> MaybeRequiresStorage<'a, 'mir, 'tcx> { /// Kill locals that are fully moved and have not been borrowed. fn check_for_move(&mut self, trans: &mut impl GenKill, loc: Location) { - let body = self.borrowed_locals.body(); - let mut visitor = MoveVisitor { trans, borrowed_locals: &mut self.borrowed_locals }; + let body = self.body; + let mut visitor = MoveVisitor { trans, borrowed_locals: &mut self.borrowed_locals_cursor }; visitor.visit_location(body, loc); } } struct MoveVisitor<'a, 'b, 'mir, 'tcx, T> { - borrowed_locals: &'a RefCell>, + borrowed_locals: &'a mut BorrowedLocalsResultsCursor<'b, 'mir, 'tcx>, trans: &'a mut T, } @@ -377,7 +374,7 @@ where #[instrument(skip(self), level = "debug")] fn visit_local(&mut self, local: Local, context: PlaceContext, loc: Location) { if PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) == context { - let borrowed_locals = self.borrowed_locals.borrow_mut().get(loc); + let borrowed_locals = self.borrowed_locals.get(loc); debug!(?borrowed_locals); if !borrowed_locals.contains(local) { self.trans.kill(local); diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs index a579a9ad12138..1426e12cbfaa7 100644 --- a/compiler/rustc_mir_transform/src/generator.rs +++ b/compiler/rustc_mir_transform/src/generator.rs @@ -594,25 +594,26 @@ fn locals_live_across_suspend_points<'tcx>( .iterate_to_fixpoint() .into_results_cursor(body_ref); + // conservative upper bound on borrowed locals that is needed in the `LiveBorrows` analysis let borrowed_locals_results = MaybeBorrowedLocals.into_engine(tcx, body_ref).pass_name("generator").iterate_to_fixpoint(); - let borrowed_locals_cursor = - rustc_mir_dataflow::ResultsCursor::new(body_ref, &borrowed_locals_results); - - let mut blc = rustc_mir_dataflow::ResultsCursor::new(body_ref, &borrowed_locals_results); // Calculate the locals that are live due to outstanding references or pointers. - let live_borrows_results = get_borrowed_locals_results(body_ref, tcx, borrowed_locals_cursor); + let live_borrows_results = get_borrowed_locals_results(body_ref, tcx, borrowed_locals_results); + let mut live_borrows_cursor = BorrowedLocalsResultsCursor::new(body_ref, &live_borrows_results); // Calculate the MIR locals that we actually need to keep storage around // for. - let requires_storage_results = MaybeRequiresStorage::new(body, &live_borrows_results) - .into_engine(tcx, body_ref) - .iterate_to_fixpoint(); + let mut requires_storage_results = MaybeRequiresStorage::new( + body, + BorrowedLocalsResultsCursor::new(body_ref, &live_borrows_results), + ) + .into_engine(tcx, body_ref) + .iterate_to_fixpoint(); let mut requires_storage_cursor = - rustc_mir_dataflow::ResultsCursor::new(body_ref, &requires_storage_results); + rustc_mir_dataflow::ResultsRefCursor::new(body_ref, &mut requires_storage_results); // Calculate the liveness of MIR locals ignoring borrows. let mut liveness = MaybeLiveLocals @@ -646,8 +647,6 @@ fn locals_live_across_suspend_points<'tcx>( // forever. Note that the final liveness is still bounded by the storage liveness // of the local, which happens using the `intersect` operation below. let live_borrowed_locals = live_borrows_cursor.get(loc); - blc.seek_before_primary_effect(loc); - let old_borrowed_locals = blc.get(); let mut live_locals_stmt: BitSet<_> = BitSet::new_empty(body.local_decls.len()); liveness.seek_before_primary_effect(loc); @@ -658,7 +657,6 @@ fn locals_live_across_suspend_points<'tcx>( storage_req.union(requires_storage_cursor.get()); debug!(?live_borrowed_locals); - debug!(?old_borrowed_locals); debug!(?live_locals_stmt); debug!(?storage_req); @@ -770,7 +768,7 @@ fn compute_storage_conflicts<'a, 'mir, 'tcx>( body: &'mir Body<'tcx>, saved_locals: &GeneratorSavedLocals, always_live_locals: BitSet, - requires_storage: rustc_mir_dataflow::Results<'tcx, MaybeRequiresStorage<'a, 'mir, 'tcx>>, + mut requires_storage: rustc_mir_dataflow::Results<'tcx, MaybeRequiresStorage<'a, 'mir, 'tcx>>, ) -> BitMatrix { assert_eq!(body.local_decls.len(), saved_locals.domain_size());