Skip to content

Commit

Permalink
Revert "Auto merge of #71956 - ecstatic-morse:remove-requires-storage…
Browse files Browse the repository at this point in the history
…-analysis, r=tmandry"

This reverts commit 458a3e7, reversing
changes made to d9417b3.
  • Loading branch information
Mark-Simulacrum committed Jun 13, 2020
1 parent 9da7d07 commit 4f75a62
Show file tree
Hide file tree
Showing 7 changed files with 372 additions and 254 deletions.
3 changes: 0 additions & 3 deletions src/librustc_mir/dataflow/impls/borrowed_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ impl<K> GenKillAnalysis<'tcx> for MaybeBorrowedLocals<K>
where
K: BorrowAnalysisKind<'tcx>,
{
// The generator transform relies on the fact that this analysis does **not** use "before"
// effects.

fn statement_effect(
&self,
trans: &mut impl GenKill<Self::Idx>,
Expand Down
118 changes: 0 additions & 118 deletions src/librustc_mir/dataflow/impls/init_locals.rs

This file was deleted.

4 changes: 1 addition & 3 deletions src/librustc_mir/dataflow/impls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,13 @@ use crate::dataflow::drop_flag_effects;

mod borrowed_locals;
pub(super) mod borrows;
mod init_locals;
mod liveness;
mod storage_liveness;

pub use self::borrowed_locals::{MaybeBorrowedLocals, MaybeMutBorrowedLocals};
pub use self::borrows::Borrows;
pub use self::init_locals::MaybeInitializedLocals;
pub use self::liveness::MaybeLiveLocals;
pub use self::storage_liveness::MaybeStorageLive;
pub use self::storage_liveness::{MaybeRequiresStorage, MaybeStorageLive};

/// `MaybeInitializedPlaces` tracks all places that might be
/// initialized upon reaching a particular point in the control flow
Expand Down
234 changes: 233 additions & 1 deletion src/librustc_mir/dataflow/impls/storage_liveness.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
pub use super::*;

use crate::dataflow::BottomValue;
use crate::dataflow::{self, GenKill};
use crate::dataflow::{self, GenKill, Results, ResultsRefCursor};
use crate::util::storage::AlwaysLiveLocals;
use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use std::cell::RefCell;

#[derive(Clone)]
pub struct MaybeStorageLive {
Expand Down Expand Up @@ -76,3 +78,233 @@ impl BottomValue for MaybeStorageLive {
/// bottom = dead
const BOTTOM_VALUE: bool = false;
}

type BorrowedLocalsResults<'a, 'tcx> = ResultsRefCursor<'a, 'a, '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>>,
}

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<'mir, 'tcx> dataflow::AnalysisDomain<'tcx> for MaybeRequiresStorage<'mir, 'tcx> {
type Idx = Local;

const NAME: &'static str = "requires_storage";

fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize {
body.local_decls.len()
}

fn initialize_start_block(&self, body: &mir::Body<'tcx>, on_entry: &mut BitSet<Self::Idx>) {
// The resume argument is live on function entry (we don't care about
// the `self` argument)
for arg in body.args_iter().skip(1) {
on_entry.insert(arg);
}
}
}

impl<'mir, 'tcx> dataflow::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tcx> {
fn before_statement_effect(
&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);

match &stmt.kind {
StatementKind::StorageDead(l) => trans.kill(*l),

// If a place is assigned to in a statement, it needs storage for that statement.
StatementKind::Assign(box (place, _))
| StatementKind::SetDiscriminant { box place, .. } => {
trans.gen(place.local);
}
StatementKind::LlvmInlineAsm(asm) => {
for place in &*asm.outputs {
trans.gen(place.local);
}
}

// Nothing to do for these. Match exhaustively so this fails to compile when new
// variants are added.
StatementKind::AscribeUserType(..)
| StatementKind::FakeRead(..)
| StatementKind::Nop
| StatementKind::Retag(..)
| StatementKind::StorageLive(..) => {}
}
}

fn statement_effect(
&self,
trans: &mut impl GenKill<Self::Idx>,
_: &mir::Statement<'tcx>,
loc: Location,
) {
// If we move from a place then only stops needing storage *after*
// that statement.
self.check_for_move(trans, loc);
}

fn before_terminator_effect(
&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);

match &terminator.kind {
TerminatorKind::Call { destination: Some((place, _)), .. } => {
trans.gen(place.local);
}

// Note that we do *not* gen the `resume_arg` of `Yield` terminators. The reason for
// that is that a `yield` will return from the function, and `resume_arg` is written
// only when the generator is later resumed. Unlike `Call`, this doesn't require the
// place to have storage *before* the yield, only after.
TerminatorKind::Yield { .. } => {}

TerminatorKind::InlineAsm { operands, .. } => {
for op in operands {
match op {
InlineAsmOperand::Out { place, .. }
| InlineAsmOperand::InOut { out_place: place, .. } => {
if let Some(place) = place {
trans.gen(place.local);
}
}
InlineAsmOperand::In { .. }
| InlineAsmOperand::Const { .. }
| InlineAsmOperand::SymFn { .. }
| InlineAsmOperand::SymStatic { .. } => {}
}
}
}

// Nothing to do for these. Match exhaustively so this fails to compile when new
// variants are added.
TerminatorKind::Call { destination: None, .. }
| TerminatorKind::Abort
| TerminatorKind::Assert { .. }
| TerminatorKind::Drop { .. }
| TerminatorKind::DropAndReplace { .. }
| TerminatorKind::FalseEdges { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::GeneratorDrop
| TerminatorKind::Goto { .. }
| TerminatorKind::Resume
| TerminatorKind::Return
| TerminatorKind::SwitchInt { .. }
| TerminatorKind::Unreachable => {}
}
}

fn terminator_effect(
&self,
trans: &mut impl GenKill<Self::Idx>,
terminator: &mir::Terminator<'tcx>,
loc: Location,
) {
match &terminator.kind {
// For call terminators the destination requires storage for the call
// and after the call returns successfully, but not after a panic.
// Since `propagate_call_unwind` doesn't exist, we have to kill the
// destination here, and then gen it again in `call_return_effect`.
TerminatorKind::Call { destination: Some((place, _)), .. } => {
trans.kill(place.local);
}

// Nothing to do for these. Match exhaustively so this fails to compile when new
// variants are added.
TerminatorKind::Call { destination: None, .. }
| TerminatorKind::Yield { .. }
| TerminatorKind::Abort
| TerminatorKind::Assert { .. }
| TerminatorKind::Drop { .. }
| TerminatorKind::DropAndReplace { .. }
| TerminatorKind::FalseEdges { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::GeneratorDrop
| TerminatorKind::Goto { .. }
| TerminatorKind::InlineAsm { .. }
| TerminatorKind::Resume
| TerminatorKind::Return
| TerminatorKind::SwitchInt { .. }
| TerminatorKind::Unreachable => {}
}

self.check_for_move(trans, loc);
}

fn call_return_effect(
&self,
trans: &mut impl GenKill<Self::Idx>,
_block: BasicBlock,
_func: &mir::Operand<'tcx>,
_args: &[mir::Operand<'tcx>],
return_place: mir::Place<'tcx>,
) {
trans.gen(return_place.local);
}

fn yield_resume_effect(
&self,
trans: &mut impl GenKill<Self::Idx>,
_resume_block: BasicBlock,
resume_place: mir::Place<'tcx>,
) {
trans.gen(resume_place.local);
}
}

impl<'mir, 'tcx> MaybeRequiresStorage<'mir, '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);
}
}

impl<'mir, 'tcx> BottomValue for MaybeRequiresStorage<'mir, 'tcx> {
/// bottom = dead
const BOTTOM_VALUE: bool = false;
}

struct MoveVisitor<'a, 'mir, 'tcx, T> {
borrowed_locals: &'a RefCell<BorrowedLocalsResults<'mir, 'tcx>>,
trans: &'a mut T,
}

impl<'a, 'mir, 'tcx, T> Visitor<'tcx> for MoveVisitor<'a, 'mir, '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.trans.kill(*local);
}
}
}
}
Loading

0 comments on commit 4f75a62

Please sign in to comment.