Skip to content

Commit

Permalink
make BorrowedLocalsResultsCursor compatible with rust-lang#108293
Browse files Browse the repository at this point in the history
  • Loading branch information
b-naber committed Jun 29, 2023
1 parent 2208981 commit 9fb6c16
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 77 deletions.
89 changes: 38 additions & 51 deletions compiler/rustc_mir_dataflow/src/impls/live_borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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
Expand Down Expand Up @@ -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>>,

Expand All @@ -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<Local, FxHashSet<Local>>,

/// 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,
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1111,19 +1093,24 @@ 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
/// require convergence, we expose the set of borrowed `Local`s for a `Location` directly via
/// 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<BasicBlock, BitSet<Local>>,
>,
>,

// 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
Expand All @@ -1132,14 +1119,13 @@ pub struct BorrowedLocalsResultsCursor<'a, 'mir, 'tcx> {
borrowed_local_to_locals_to_keep_alive: &'a FxHashMap<Local, FxHashSet<Local>>,

// 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`,
Expand All @@ -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(),
),
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()
}
Expand All @@ -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<Self::Idx>,
statement: &mir::Statement<'tcx>,
location: Location,
Expand All @@ -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<Self::Idx>,
terminator: &mir::Terminator<'tcx>,
location: Location,
Expand All @@ -1270,7 +1257,7 @@ impl<'a, 'tcx> GenKillAnalysis<'tcx> for LiveBorrows<'a, 'tcx> {
}

fn call_return_effect(
&self,
&mut self,
_trans: &mut impl GenKill<Self::Idx>,
_block: mir::BasicBlock,
_return_places: CallReturnPlaces<'_, 'tcx>,
Expand Down
23 changes: 10 additions & 13 deletions compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs
Original file line number Diff line number Diff line change
@@ -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::*;
Expand Down Expand Up @@ -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<BorrowedLocalsResultsCursor<'a, 'mir, 'tcx>>,
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 }
}
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<Local>, 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<BorrowedLocalsResultsCursor<'b, 'mir, 'tcx>>,
borrowed_locals: &'a mut BorrowedLocalsResultsCursor<'b, 'mir, 'tcx>,
trans: &'a mut T,
}

Expand All @@ -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);
Expand Down
24 changes: 11 additions & 13 deletions compiler/rustc_mir_transform/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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);

Expand Down Expand Up @@ -770,7 +768,7 @@ fn compute_storage_conflicts<'a, 'mir, 'tcx>(
body: &'mir Body<'tcx>,
saved_locals: &GeneratorSavedLocals,
always_live_locals: BitSet<Local>,
requires_storage: rustc_mir_dataflow::Results<'tcx, MaybeRequiresStorage<'a, 'mir, 'tcx>>,
mut requires_storage: rustc_mir_dataflow::Results<'tcx, MaybeRequiresStorage<'a, 'mir, 'tcx>>,
) -> BitMatrix<GeneratorSavedLocal, GeneratorSavedLocal> {
assert_eq!(body.local_decls.len(), saved_locals.domain_size());

Expand Down

0 comments on commit 9fb6c16

Please sign in to comment.