Auto merge of #108293 - Jarcho:mut_analyses, r=eholk

Take MIR dataflow analyses by mutable reference

The main motivation here is any analysis requiring dynamically sized scratch memory to work. One concrete example would be pointer target tracking, where tracking the results of a dereference can result in multiple possible targets. This leads to processing multi-level dereferences requiring the ability to handle a changing number of potential targets per step. A (simplified) function for this would be `fn apply_deref(potential_targets: &mut Vec<Target>)` which would use the scratch space contained in the analysis to send arguments and receive the results.

The alternative to this would be to wrap everything in a `RefCell`, which is what `MaybeRequiresStorage` currently does. This comes with a small perf cost and loses the compiler's guarantee that we don't try to take multiple borrows at the same time.

For the implementation:
* `AnalysisResults` is an unfortunate requirement to avoid an unconstrained type parameter error.
* `CloneAnalysis` could just be `Clone` instead, but that would result in more work than is required to have multiple cursors over the same result set.
* `ResultsVisitor` now takes the results type on in each function as there's no other way to have access to the analysis without cloning it. This could use an associated type rather than a type parameter, but the current approach makes it easier to not care about the type when it's not necessary.
* `MaybeRequiresStorage` now no longer uses a `RefCell`, but the graphviz formatter now does. It could be removed, but that would require even more changes and doesn't really seem necessary.
This commit is contained in:
bors
2023-06-08 23:58:44 +00:00
19 changed files with 491 additions and 288 deletions

View File

@@ -10,6 +10,7 @@ use rustc_middle::mir::*;
/// At present, this is used as a very limited form of alias analysis. For example,
/// `MaybeBorrowedLocals` is used to compute which locals are live during a yield expression for
/// immovable generators.
#[derive(Clone, Copy)]
pub struct MaybeBorrowedLocals;
impl MaybeBorrowedLocals {
@@ -36,7 +37,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeBorrowedLocals {
type Idx = Local;
fn statement_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
statement: &mir::Statement<'tcx>,
location: Location,
@@ -45,7 +46,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeBorrowedLocals {
}
fn terminator_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
terminator: &mir::Terminator<'tcx>,
location: Location,
@@ -54,7 +55,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeBorrowedLocals {
}
fn call_return_effect(
&self,
&mut self,
_trans: &mut impl GenKill<Self::Idx>,
_block: mir::BasicBlock,
_return_places: CallReturnPlaces<'_, 'tcx>,

View File

@@ -21,6 +21,7 @@ use crate::{Analysis, AnalysisDomain, Backward, CallReturnPlaces, GenKill, GenKi
/// [`MaybeBorrowedLocals`]: super::MaybeBorrowedLocals
/// [flow-test]: https://github.com/rust-lang/rust/blob/a08c47310c7d49cbdc5d7afb38408ba519967ecd/src/test/ui/mir-dataflow/liveness-ptr.rs
/// [liveness]: https://en.wikipedia.org/wiki/Live_variable_analysis
#[derive(Clone, Copy)]
pub struct MaybeLiveLocals;
impl<'tcx> AnalysisDomain<'tcx> for MaybeLiveLocals {
@@ -43,7 +44,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeLiveLocals {
type Idx = Local;
fn statement_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
statement: &mir::Statement<'tcx>,
location: Location,
@@ -52,7 +53,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeLiveLocals {
}
fn terminator_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
terminator: &mir::Terminator<'tcx>,
location: Location,
@@ -61,7 +62,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeLiveLocals {
}
fn call_return_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_block: mir::BasicBlock,
return_places: CallReturnPlaces<'_, 'tcx>,
@@ -74,7 +75,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeLiveLocals {
}
fn yield_resume_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_resume_block: mir::BasicBlock,
resume_place: mir::Place<'tcx>,
@@ -215,6 +216,7 @@ impl DefUse {
/// This is basically written for dead store elimination and nothing else.
///
/// All of the caveats of `MaybeLiveLocals` apply.
#[derive(Clone, Copy)]
pub struct MaybeTransitiveLiveLocals<'a> {
always_live: &'a BitSet<Local>,
}
@@ -247,7 +249,7 @@ impl<'a, 'tcx> AnalysisDomain<'tcx> for MaybeTransitiveLiveLocals<'a> {
impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> {
fn apply_statement_effect(
&self,
&mut self,
trans: &mut Self::Domain,
statement: &mir::Statement<'tcx>,
location: Location,
@@ -282,7 +284,7 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> {
}
fn apply_terminator_effect(
&self,
&mut self,
trans: &mut Self::Domain,
terminator: &mir::Terminator<'tcx>,
location: Location,
@@ -291,7 +293,7 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> {
}
fn apply_call_return_effect(
&self,
&mut self,
trans: &mut Self::Domain,
_block: mir::BasicBlock,
return_places: CallReturnPlaces<'_, 'tcx>,
@@ -304,7 +306,7 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> {
}
fn apply_yield_resume_effect(
&self,
&mut self,
trans: &mut Self::Domain,
_resume_block: mir::BasicBlock,
resume_place: mir::Place<'tcx>,

View File

@@ -306,7 +306,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
type Idx = MovePathIndex;
fn statement_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
statement: &mir::Statement<'tcx>,
location: Location,
@@ -329,7 +329,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
}
fn terminator_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
terminator: &mir::Terminator<'tcx>,
location: Location,
@@ -351,7 +351,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
}
fn call_return_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_block: mir::BasicBlock,
return_places: CallReturnPlaces<'_, 'tcx>,
@@ -372,7 +372,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
}
fn switch_int_edge_effects<G: GenKill<Self::Idx>>(
&self,
&mut self,
block: mir::BasicBlock,
discr: &mir::Operand<'tcx>,
edge_effects: &mut impl SwitchIntEdgeEffects<G>,
@@ -442,7 +442,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> {
type Idx = MovePathIndex;
fn statement_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_statement: &mir::Statement<'tcx>,
location: Location,
@@ -456,7 +456,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> {
}
fn terminator_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_terminator: &mir::Terminator<'tcx>,
location: Location,
@@ -467,7 +467,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> {
}
fn call_return_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_block: mir::BasicBlock,
return_places: CallReturnPlaces<'_, 'tcx>,
@@ -488,7 +488,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> {
}
fn switch_int_edge_effects<G: GenKill<Self::Idx>>(
&self,
&mut self,
block: mir::BasicBlock,
discr: &mir::Operand<'tcx>,
edge_effects: &mut impl SwitchIntEdgeEffects<G>,
@@ -562,7 +562,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for DefinitelyInitializedPlaces<'_, 'tcx> {
type Idx = MovePathIndex;
fn statement_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_statement: &mir::Statement<'tcx>,
location: Location,
@@ -573,7 +573,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for DefinitelyInitializedPlaces<'_, 'tcx> {
}
fn terminator_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_terminator: &mir::Terminator<'tcx>,
location: Location,
@@ -584,7 +584,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for DefinitelyInitializedPlaces<'_, 'tcx> {
}
fn call_return_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_block: mir::BasicBlock,
return_places: CallReturnPlaces<'_, 'tcx>,
@@ -627,7 +627,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for EverInitializedPlaces<'_, 'tcx> {
#[instrument(skip(self, trans), level = "debug")]
fn statement_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
stmt: &mir::Statement<'tcx>,
location: Location,
@@ -651,7 +651,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for EverInitializedPlaces<'_, 'tcx> {
#[instrument(skip(self, trans, _terminator), level = "debug")]
fn terminator_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_terminator: &mir::Terminator<'tcx>,
location: Location,
@@ -672,7 +672,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for EverInitializedPlaces<'_, 'tcx> {
}
fn call_return_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
block: mir::BasicBlock,
_return_places: CallReturnPlaces<'_, 'tcx>,

View File

@@ -1,10 +1,9 @@
pub use super::*;
use crate::{CallReturnPlaces, GenKill, Results, ResultsRefCursor};
use crate::{CallReturnPlaces, GenKill, ResultsClonedCursor};
use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use std::borrow::Cow;
use std::cell::RefCell;
#[derive(Clone)]
pub struct MaybeStorageLive<'a> {
@@ -17,6 +16,12 @@ impl<'a> MaybeStorageLive<'a> {
}
}
impl crate::CloneAnalysis for MaybeStorageLive<'_> {
fn clone_analysis(&self) -> Self {
self.clone()
}
}
impl<'tcx, 'a> crate::AnalysisDomain<'tcx> for MaybeStorageLive<'a> {
type Domain = BitSet<Local>;
@@ -43,7 +48,7 @@ impl<'tcx, 'a> crate::GenKillAnalysis<'tcx> for MaybeStorageLive<'a> {
type Idx = Local;
fn statement_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
stmt: &mir::Statement<'tcx>,
_: Location,
@@ -56,7 +61,7 @@ impl<'tcx, 'a> crate::GenKillAnalysis<'tcx> for MaybeStorageLive<'a> {
}
fn terminator_effect(
&self,
&mut self,
_trans: &mut impl GenKill<Self::Idx>,
_: &mir::Terminator<'tcx>,
_: Location,
@@ -65,7 +70,7 @@ impl<'tcx, 'a> crate::GenKillAnalysis<'tcx> for MaybeStorageLive<'a> {
}
fn call_return_effect(
&self,
&mut self,
_trans: &mut impl GenKill<Self::Idx>,
_block: BasicBlock,
_return_places: CallReturnPlaces<'_, 'tcx>,
@@ -110,7 +115,7 @@ impl<'tcx> crate::GenKillAnalysis<'tcx> for MaybeStorageDead {
type Idx = Local;
fn statement_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
stmt: &mir::Statement<'tcx>,
_: Location,
@@ -123,7 +128,7 @@ impl<'tcx> crate::GenKillAnalysis<'tcx> for MaybeStorageDead {
}
fn terminator_effect(
&self,
&mut self,
_trans: &mut impl GenKill<Self::Idx>,
_: &mir::Terminator<'tcx>,
_: Location,
@@ -132,7 +137,7 @@ impl<'tcx> crate::GenKillAnalysis<'tcx> for MaybeStorageDead {
}
fn call_return_effect(
&self,
&mut self,
_trans: &mut impl GenKill<Self::Idx>,
_block: BasicBlock,
_return_places: CallReturnPlaces<'_, 'tcx>,
@@ -141,28 +146,28 @@ impl<'tcx> crate::GenKillAnalysis<'tcx> for MaybeStorageDead {
}
}
type BorrowedLocalsResults<'a, 'tcx> = ResultsRefCursor<'a, 'a, 'tcx, MaybeBorrowedLocals>;
type BorrowedLocalsResults<'res, 'mir, 'tcx> =
ResultsClonedCursor<'res, 'mir, 'tcx, MaybeBorrowedLocals>;
/// Dataflow analysis that determines whether each local requires storage at a
/// given location; i.e. whether its storage can go away without being observed.
pub struct MaybeRequiresStorage<'mir, 'tcx> {
body: &'mir Body<'tcx>,
borrowed_locals: RefCell<BorrowedLocalsResults<'mir, 'tcx>>,
pub struct MaybeRequiresStorage<'res, 'mir, 'tcx> {
borrowed_locals: BorrowedLocalsResults<'res, 'mir, 'tcx>,
}
impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> {
pub fn new(
body: &'mir Body<'tcx>,
borrowed_locals: &'mir Results<'tcx, MaybeBorrowedLocals>,
) -> Self {
MaybeRequiresStorage {
body,
borrowed_locals: RefCell::new(ResultsRefCursor::new(&body, borrowed_locals)),
}
impl<'res, 'mir, 'tcx> MaybeRequiresStorage<'res, 'mir, 'tcx> {
pub fn new(borrowed_locals: BorrowedLocalsResults<'res, 'mir, 'tcx>) -> Self {
MaybeRequiresStorage { borrowed_locals }
}
}
impl<'mir, 'tcx> crate::AnalysisDomain<'tcx> for MaybeRequiresStorage<'mir, 'tcx> {
impl crate::CloneAnalysis for MaybeRequiresStorage<'_, '_, '_> {
fn clone_analysis(&self) -> Self {
Self { borrowed_locals: self.borrowed_locals.new_cursor() }
}
}
impl<'tcx> crate::AnalysisDomain<'tcx> for MaybeRequiresStorage<'_, '_, 'tcx> {
type Domain = BitSet<Local>;
const NAME: &'static str = "requires_storage";
@@ -181,17 +186,17 @@ impl<'mir, 'tcx> crate::AnalysisDomain<'tcx> for MaybeRequiresStorage<'mir, 'tcx
}
}
impl<'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tcx> {
impl<'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'_, '_, 'tcx> {
type Idx = Local;
fn before_statement_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
stmt: &mir::Statement<'tcx>,
loc: Location,
) {
// If a place is borrowed in a statement, it needs storage for that statement.
self.borrowed_locals.borrow().analysis().statement_effect(trans, stmt, loc);
self.borrowed_locals.mut_analysis().statement_effect(trans, stmt, loc);
match &stmt.kind {
StatementKind::StorageDead(l) => trans.kill(*l),
@@ -218,7 +223,7 @@ impl<'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tc
}
fn statement_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_: &mir::Statement<'tcx>,
loc: Location,
@@ -229,13 +234,13 @@ impl<'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tc
}
fn before_terminator_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
terminator: &mir::Terminator<'tcx>,
loc: Location,
) {
// If a place is borrowed in a terminator, it needs storage for that terminator.
self.borrowed_locals.borrow().analysis().terminator_effect(trans, terminator, loc);
self.borrowed_locals.mut_analysis().terminator_effect(trans, terminator, loc);
match &terminator.kind {
TerminatorKind::Call { destination, .. } => {
@@ -282,7 +287,7 @@ impl<'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tc
}
fn terminator_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
terminator: &mir::Terminator<'tcx>,
loc: Location,
@@ -321,7 +326,7 @@ impl<'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tc
}
fn call_return_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_block: BasicBlock,
return_places: CallReturnPlaces<'_, 'tcx>,
@@ -330,7 +335,7 @@ impl<'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tc
}
fn yield_resume_effect(
&self,
&mut self,
trans: &mut impl GenKill<Self::Idx>,
_resume_block: BasicBlock,
resume_place: mir::Place<'tcx>,
@@ -339,28 +344,28 @@ impl<'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tc
}
}
impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> {
impl<'tcx> MaybeRequiresStorage<'_, '_, 'tcx> {
/// Kill locals that are fully moved and have not been borrowed.
fn check_for_move(&self, trans: &mut impl GenKill<Local>, loc: Location) {
let mut visitor = MoveVisitor { trans, borrowed_locals: &self.borrowed_locals };
visitor.visit_location(&self.body, loc);
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 };
visitor.visit_location(body, loc);
}
}
struct MoveVisitor<'a, 'mir, 'tcx, T> {
borrowed_locals: &'a RefCell<BorrowedLocalsResults<'mir, 'tcx>>,
struct MoveVisitor<'a, 'res, 'mir, 'tcx, T> {
borrowed_locals: &'a mut BorrowedLocalsResults<'res, 'mir, 'tcx>,
trans: &'a mut T,
}
impl<'a, 'mir, 'tcx, T> Visitor<'tcx> for MoveVisitor<'a, 'mir, 'tcx, T>
impl<'tcx, T> Visitor<'tcx> for MoveVisitor<'_, '_, '_, 'tcx, T>
where
T: GenKill<Local>,
{
fn visit_local(&mut self, local: Local, context: PlaceContext, loc: Location) {
if PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) == context {
let mut borrowed_locals = self.borrowed_locals.borrow_mut();
borrowed_locals.seek_before_primary_effect(loc);
if !borrowed_locals.contains(local) {
self.borrowed_locals.seek_before_primary_effect(loc);
if !self.borrowed_locals.contains(local) {
self.trans.kill(local);
}
}