-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Don't store locals that have been moved from in generators #61922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
7262e64
4a8a552
9969417
11b09e7
aee1357
d8ed2e7
a68e2c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,13 @@ | ||
pub use super::*; | ||
|
||
use rustc::mir::*; | ||
use rustc::mir::visit::{ | ||
PlaceContext, Visitor, NonMutatingUseContext, | ||
}; | ||
use std::cell::RefCell; | ||
use crate::dataflow::BitDenotation; | ||
use crate::dataflow::HaveBeenBorrowedLocals; | ||
use crate::dataflow::{DataflowResults, DataflowResultsCursor, DataflowResultsRefCursor}; | ||
|
||
#[derive(Copy, Clone)] | ||
pub struct MaybeStorageLive<'a, 'tcx> { | ||
|
@@ -63,3 +69,125 @@ impl<'a, 'tcx> BottomValue for MaybeStorageLive<'a, 'tcx> { | |
/// bottom = dead | ||
const BOTTOM_VALUE: bool = false; | ||
} | ||
|
||
/// Dataflow analysis that determines whether each local requires storage at a | ||
/// given location; i.e. whether its storage can go away without being observed. | ||
/// | ||
/// In the case of a movable generator, borrowed_locals can be `None` and we | ||
/// will not consider borrows in this pass. This relies on the fact that we only | ||
/// use this pass at yield points for these generators. | ||
pub struct RequiresStorage<'mir, 'tcx, 'b> { | ||
body: &'mir Body<'tcx>, | ||
borrowed_locals: | ||
RefCell<DataflowResultsRefCursor<'mir, 'tcx, 'b, HaveBeenBorrowedLocals<'mir, 'tcx>>>, | ||
} | ||
|
||
impl<'mir, 'tcx: 'mir, 'b> RequiresStorage<'mir, 'tcx, 'b> { | ||
pub fn new( | ||
body: &'mir Body<'tcx>, | ||
borrowed_locals: &'b DataflowResults<'tcx, HaveBeenBorrowedLocals<'mir, 'tcx>>, | ||
) -> Self { | ||
RequiresStorage { | ||
body, | ||
borrowed_locals: RefCell::new(DataflowResultsCursor::new(borrowed_locals, body)), | ||
} | ||
} | ||
|
||
pub fn body(&self) -> &Body<'tcx> { | ||
self.body | ||
} | ||
} | ||
|
||
impl<'mir, 'tcx, 'b> BitDenotation<'tcx> for RequiresStorage<'mir, 'tcx, 'b> { | ||
type Idx = Local; | ||
fn name() -> &'static str { "requires_storage" } | ||
fn bits_per_block(&self) -> usize { | ||
self.body.local_decls.len() | ||
} | ||
|
||
fn start_block_effect(&self, _sets: &mut BitSet<Local>) { | ||
// Nothing is live on function entry | ||
} | ||
|
||
fn statement_effect(&self, | ||
sets: &mut GenKillSet<Local>, | ||
loc: Location) { | ||
self.check_for_move(sets, loc); | ||
self.check_for_borrow(sets, loc); | ||
|
||
let stmt = &self.body[loc.block].statements[loc.statement_index]; | ||
match stmt.kind { | ||
StatementKind::StorageLive(l) => sets.gen(l), | ||
StatementKind::StorageDead(l) => sets.kill(l), | ||
StatementKind::Assign(ref place, _) | ||
| StatementKind::SetDiscriminant { ref place, .. } => { | ||
place.base_local().map(|l| sets.gen(l)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this correct if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe so, because we don't kill the local if it has ever had its address taken. So there's no need to track pointers and derefs so we can gen it again. |
||
} | ||
StatementKind::InlineAsm(box InlineAsm { ref outputs, .. }) => { | ||
for p in &**outputs { | ||
p.base_local().map(|l| sets.gen(l)); | ||
} | ||
} | ||
_ => (), | ||
} | ||
} | ||
|
||
fn terminator_effect(&self, | ||
sets: &mut GenKillSet<Local>, | ||
loc: Location) { | ||
self.check_for_move(sets, loc); | ||
self.check_for_borrow(sets, loc); | ||
} | ||
|
||
fn propagate_call_return( | ||
&self, | ||
in_out: &mut BitSet<Local>, | ||
_call_bb: mir::BasicBlock, | ||
_dest_bb: mir::BasicBlock, | ||
dest_place: &mir::Place<'tcx>, | ||
) { | ||
dest_place.base_local().map(|l| in_out.insert(l)); | ||
} | ||
} | ||
|
||
impl<'mir, 'tcx, 'b> RequiresStorage<'mir, 'tcx, 'b> { | ||
/// Kill locals that are fully moved and have not been borrowed. | ||
fn check_for_move(&self, sets: &mut GenKillSet<Local>, loc: Location) { | ||
let mut visitor = MoveVisitor { | ||
sets, | ||
borrowed_locals: &self.borrowed_locals, | ||
}; | ||
visitor.visit_location(self.body, loc); | ||
} | ||
|
||
/// Gen locals that are newly borrowed. This includes borrowing any part of | ||
/// a local (we rely on this behavior of `HaveBeenBorrowedLocals`). | ||
fn check_for_borrow(&self, sets: &mut GenKillSet<Local>, loc: Location) { | ||
let mut borrowed_locals = self.borrowed_locals.borrow_mut(); | ||
borrowed_locals.seek(loc); | ||
borrowed_locals.each_gen_bit(|l| sets.gen(l)); | ||
} | ||
} | ||
|
||
impl<'mir, 'tcx, 'b> BottomValue for RequiresStorage<'mir, 'tcx, 'b> { | ||
/// bottom = dead | ||
const BOTTOM_VALUE: bool = false; | ||
} | ||
|
||
struct MoveVisitor<'a, 'b, 'mir, 'tcx> { | ||
borrowed_locals: | ||
&'a RefCell<DataflowResultsRefCursor<'mir, 'tcx, 'b, HaveBeenBorrowedLocals<'mir, 'tcx>>>, | ||
sets: &'a mut GenKillSet<Local>, | ||
} | ||
|
||
impl<'a, 'b, 'mir: 'a, 'tcx> Visitor<'tcx> for MoveVisitor<'a, 'b, 'mir, 'tcx> { | ||
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(loc); | ||
if !borrowed_locals.contains(*local) { | ||
self.sets.kill(*local); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ use std::io; | |
use std::path::PathBuf; | ||
use std::usize; | ||
|
||
pub use self::impls::{MaybeStorageLive}; | ||
pub use self::impls::{MaybeStorageLive, RequiresStorage}; | ||
pub use self::impls::{MaybeInitializedPlaces, MaybeUninitializedPlaces}; | ||
pub use self::impls::DefinitelyInitializedPlaces; | ||
pub use self::impls::EverInitializedPlaces; | ||
|
@@ -344,6 +344,99 @@ pub(crate) trait DataflowResultsConsumer<'a, 'tcx: 'a> { | |
fn body(&self) -> &'a Body<'tcx>; | ||
} | ||
|
||
/// Allows iterating dataflow results in a flexible and reasonably fast way. | ||
pub struct DataflowResultsCursor<'mir, 'tcx, BD, DR = DataflowResults<'tcx, BD>> | ||
where | ||
BD: BitDenotation<'tcx>, | ||
DR: Borrow<DataflowResults<'tcx, BD>>, | ||
{ | ||
flow_state: FlowAtLocation<'tcx, BD, DR>, | ||
|
||
// The statement (or terminator) whose effect has been reconstructed in | ||
// flow_state. | ||
curr_loc: Option<Location>, | ||
|
||
body: &'mir Body<'tcx>, | ||
} | ||
|
||
pub type DataflowResultsRefCursor<'mir, 'tcx, 'flow, BD> = | ||
DataflowResultsCursor<'mir, 'tcx, BD, &'flow DataflowResults<'tcx, BD>>; | ||
tmandry marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
impl<'mir, 'tcx, BD, DR> DataflowResultsCursor<'mir, 'tcx, BD, DR> | ||
where | ||
BD: BitDenotation<'tcx>, | ||
DR: Borrow<DataflowResults<'tcx, BD>>, | ||
{ | ||
pub fn new(result: DR, body: &'mir Body<'tcx>) -> Self { | ||
DataflowResultsCursor { | ||
flow_state: FlowAtLocation::new(result), | ||
curr_loc: None, | ||
body, | ||
} | ||
} | ||
|
||
/// Seek to the given location in MIR. This method is fast if you are | ||
/// traversing your MIR statements in order. | ||
/// | ||
/// After calling `seek`, the current state will reflect all effects up to | ||
/// and including the `before_statement_effect` of the statement at location | ||
/// `loc`. The `statement_effect` of the statement at `loc` will be | ||
/// available as the current effect (see e.g. `each_gen_bit`). | ||
/// | ||
/// If `loc.statement_index` equals the number of statements in the block, | ||
/// we will reconstruct the terminator effect in the same way as described | ||
/// above. | ||
pub fn seek(&mut self, loc: Location) { | ||
if self.curr_loc.map(|cur| loc == cur).unwrap_or(false) { | ||
return; | ||
} | ||
|
||
let start_index; | ||
let should_reset = match self.curr_loc { | ||
None => true, | ||
Some(cur) | ||
if loc.block != cur.block || loc.statement_index < cur.statement_index => true, | ||
_ => false, | ||
}; | ||
if should_reset { | ||
self.flow_state.reset_to_entry_of(loc.block); | ||
start_index = 0; | ||
} else { | ||
let curr_loc = self.curr_loc.unwrap(); | ||
start_index = curr_loc.statement_index; | ||
// Apply the effect from the last seek to the current state. | ||
self.flow_state.apply_local_effect(curr_loc); | ||
} | ||
|
||
for stmt in start_index..loc.statement_index { | ||
let mut stmt_loc = loc; | ||
stmt_loc.statement_index = stmt; | ||
self.flow_state.reconstruct_statement_effect(stmt_loc); | ||
self.flow_state.apply_local_effect(stmt_loc); | ||
} | ||
|
||
if loc.statement_index == self.body[loc.block].statements.len() { | ||
self.flow_state.reconstruct_terminator_effect(loc); | ||
} else { | ||
self.flow_state.reconstruct_statement_effect(loc); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has slightly different semantics than I think you should not call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, PTAL. I wish the implementation were more elegant. |
||
self.curr_loc = Some(loc); | ||
} | ||
|
||
/// Return whether the current state contains bit `x`. | ||
pub fn contains(&self, x: BD::Idx) -> bool { | ||
self.flow_state.contains(x) | ||
} | ||
|
||
/// Iterate over each `gen` bit in the current effect (invoke `seek` first). | ||
pub fn each_gen_bit<F>(&self, f: F) | ||
where | ||
F: FnMut(BD::Idx), | ||
{ | ||
self.flow_state.each_gen_bit(f) | ||
} | ||
} | ||
|
||
pub fn state_for_location<'tcx, T: BitDenotation<'tcx>>(loc: Location, | ||
analysis: &T, | ||
result: &DataflowResults<'tcx, T>, | ||
|
Uh oh!
There was an error while loading. Please reload this page.