bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #70447 - ecstatic-morse:storage-live-always, r=tmandry

Browse source 
Add utility to find locals that don't use `StorageLive` annotations and use it for `MaybeStorageLive`

Addresses https://github.com/rust-lang/rust/pull/70004#issuecomment-599271717 (cc @RalfJung).

The only dataflow analysis that is incorrect in this case is `MaybeStorageLive`. `transform/generator.rs` implemented custom handling for this class of locals, but other consumers of this analysis (there's one in [clippy](513b46793e/clippy_lints/src/redundant_clone.rs (L402))) would be incorrect.

r? @tmandry
This commit is contained in:
bors 2020-04-10 06:14:47 +00:00
commit 96d77f0e5f
5 changed files with 117 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

22
src/librustc_mir/dataflow/impls/storage_liveness.rs
View file

@ -2,12 +2,21 @@ pub use super::*;
use crate::dataflow::BottomValue;
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(Copy, Clone)]
pub struct MaybeStorageLive;
#[derive(Clone)]
pub struct MaybeStorageLive {
always_live_locals: AlwaysLiveLocals,
}
impl MaybeStorageLive {
pub fn new(always_live_locals: AlwaysLiveLocals) -> Self {
MaybeStorageLive { always_live_locals }
}
}
impl dataflow::AnalysisDomain<'tcx> for MaybeStorageLive {
type Idx = Local;
@ -19,9 +28,12 @@ impl dataflow::AnalysisDomain<'tcx> for MaybeStorageLive {
}
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) {
assert_eq!(body.local_decls.len(), self.always_live_locals.domain_size());
for local in self.always_live_locals.iter() {
on_entry.insert(local);
}
for arg in body.args_iter() {
on_entry.insert(arg);
}
}

19
src/librustc_mir/interpret/eval_context.rs
View file

@ -24,6 +24,7 @@ use super::{
Immediate, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Memory, OpTy, Operand, Place, PlaceTy,
ScalarMaybeUndef, StackPopJump,
};
use crate::util::storage::AlwaysLiveLocals;
pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
/// Stores the `Machine` instance.
@ -610,17 +611,17 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Now mark those locals as dead that we do not want to initialize
match self.tcx.def_kind(instance.def_id()) {
// statics and constants don't have `Storage*` statements, no need to look for them
//
// FIXME: The above is likely untrue. See
// <https://github.com/rust-lang/rust/pull/70004#issuecomment-602022110>. Is it
// okay to ignore `StorageDead`/`StorageLive` annotations during CTFE?
Some(DefKind::Static) | Some(DefKind::Const) | Some(DefKind::AssocConst) => {}
_ => {
for block in body.basic_blocks() {
for stmt in block.statements.iter() {
use rustc_middle::mir::StatementKind::{StorageDead, StorageLive};
match stmt.kind {
StorageLive(local) | StorageDead(local) => {
locals[local].value = LocalValue::Dead;
}
_ => {}
}
// Mark locals that use `Storage*` annotations as dead on function entry.
let always_live = AlwaysLiveLocals::new(self.body());
for local in locals.indices() {
if !always_live.contains(local) {
locals[local].value = LocalValue::Dead;
}
}
}

77
src/librustc_mir/transform/generator.rs
View file

@ -56,12 +56,13 @@ use crate::transform::simplify;
use crate::transform::{MirPass, MirSource};
use crate::util::dump_mir;
use crate::util::liveness;
use crate::util::storage;
use rustc_data_structures::fx::FxHashMap;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_index::bit_set::{BitMatrix, BitSet};
use rustc_index::vec::{Idx, IndexVec};
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
use rustc_middle::mir::visit::{MutVisitor, PlaceContext};
use rustc_middle::mir::*;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::GeneratorSubsts;
@ -222,6 +223,9 @@ struct TransformVisitor<'tcx> {
// A list of suspension points, generated during the transform
suspension_points: Vec<SuspensionPoint<'tcx>>,
// The set of locals that have no `StorageLive`/`StorageDead` annotations.
always_live_locals: storage::AlwaysLiveLocals,
// The original RETURN_PLACE local
new_ret_local: Local,
}
@ -416,19 +420,6 @@ fn replace_local<'tcx>(
new_local
}
struct StorageIgnored(liveness::LiveVarSet);
impl<'tcx> Visitor<'tcx> for StorageIgnored {
fn visit_statement(&mut self, statement: &Statement<'tcx>, _location: Location) {
match statement.kind {
StatementKind::StorageLive(l) | StatementKind::StorageDead(l) => {
self.0.remove(l);
}
_ => (),
}
}
}
struct LivenessInfo {
/// Which locals are live across any suspension point.
///
@ -454,6 +445,7 @@ fn locals_live_across_suspend_points(
tcx: TyCtxt<'tcx>,
body: ReadOnlyBodyAndCache<'_, 'tcx>,
source: MirSource<'tcx>,
always_live_locals: &storage::AlwaysLiveLocals,
movable: bool,
) -> LivenessInfo {
let def_id = source.def_id();
@ -461,16 +453,11 @@ fn locals_live_across_suspend_points(
// Calculate when MIR locals have live storage. This gives us an upper bound of their
// lifetimes.
let mut storage_live = MaybeStorageLive
let mut storage_live = MaybeStorageLive::new(always_live_locals.clone())
.into_engine(tcx, body_ref, def_id)
.iterate_to_fixpoint()
.into_results_cursor(body_ref);
// Find the MIR locals which do not use StorageLive/StorageDead statements.
// The storage of these locals are always live.
let mut ignored = StorageIgnored(BitSet::new_filled(body.local_decls.len()));
ignored.visit_body(&body);
// Calculate the MIR locals which have been previously
// borrowed (even if they are still active).
let borrowed_locals_results =
@ -515,11 +502,14 @@ fn locals_live_across_suspend_points(
}
storage_live.seek_before(loc);
let storage_liveness = storage_live.get();
let mut storage_liveness = storage_live.get().clone();
// Later passes handle the generator's `self` argument separately.
storage_liveness.remove(SELF_ARG);
// Store the storage liveness for later use so we can restore the state
// after a suspension point
storage_liveness_map.insert(block, storage_liveness.clone());
storage_liveness_map.insert(block, storage_liveness);
requires_storage_cursor.seek_before(loc);
let storage_required = requires_storage_cursor.get().clone();
@ -551,8 +541,12 @@ fn locals_live_across_suspend_points(
.map(|live_here| renumber_bitset(&live_here, &live_locals))
.collect();
let storage_conflicts =
compute_storage_conflicts(body_ref, &live_locals, &ignored, requires_storage_results);
let storage_conflicts = compute_storage_conflicts(
body_ref,
&live_locals,
always_live_locals.clone(),
requires_storage_results,
);
LivenessInfo {
live_locals,
@ -590,18 +584,18 @@ fn renumber_bitset(
fn compute_storage_conflicts(
body: &'mir Body<'tcx>,
stored_locals: &liveness::LiveVarSet,
ignored: &StorageIgnored,
always_live_locals: storage::AlwaysLiveLocals,
requires_storage: dataflow::Results<'tcx, MaybeRequiresStorage<'mir, 'tcx>>,
) -> BitMatrix<GeneratorSavedLocal, GeneratorSavedLocal> {
assert_eq!(body.local_decls.len(), ignored.0.domain_size());
assert_eq!(body.local_decls.len(), stored_locals.domain_size());
debug!("compute_storage_conflicts({:?})", body.span);
debug!("ignored = {:?}", ignored.0);
// Storage ignored locals are not eligible for overlap, since their storage
// is always live.
let mut ineligible_locals = ignored.0.clone();
ineligible_locals.intersect(&stored_locals);
debug!("compute_storage_conflicts({:?})", body.span);
debug!("always_live = {:?}", always_live_locals);
// Locals that are always live or ones that need to be stored across
// suspension points are not eligible for overlap.
let mut ineligible_locals = always_live_locals.into_inner();
ineligible_locals.intersect(stored_locals);
// Compute the storage conflicts for all eligible locals.
let mut visitor = StorageConflictVisitor {
@ -697,6 +691,7 @@ fn compute_layout<'tcx>(
source: MirSource<'tcx>,
upvars: &Vec<Ty<'tcx>>,
interior: Ty<'tcx>,
always_live_locals: &storage::AlwaysLiveLocals,
movable: bool,
body: &mut BodyAndCache<'tcx>,
) -> (
@ -710,7 +705,13 @@ fn compute_layout<'tcx>(
live_locals_at_suspension_points,
storage_conflicts,
storage_liveness,
} = locals_live_across_suspend_points(tcx, read_only!(body), source, movable);
} = locals_live_across_suspend_points(
tcx,
read_only!(body),
source,
always_live_locals,
movable,
);
// Erase regions from the types passed in from typeck so we can compare them with
// MIR types
@ -1180,7 +1181,10 @@ fn create_cases<'tcx>(
}
let l = Local::new(i);
if point.storage_liveness.contains(l) && !transform.remap.contains_key(&l) {
let needs_storage_live = point.storage_liveness.contains(l)
&& !transform.remap.contains_key(&l)
&& !transform.always_live_locals.contains(l);
if needs_storage_live {
statements
.push(Statement { source_info, kind: StatementKind::StorageLive(l) });
}
@ -1276,11 +1280,13 @@ impl<'tcx> MirPass<'tcx> for StateTransform {
},
);
let always_live_locals = storage::AlwaysLiveLocals::new(&body);
// Extract locals which are live across suspension point into `layout`
// `remap` gives a mapping from local indices onto generator struct indices
// `storage_liveness` tells us which locals have live storage at suspension points
let (remap, layout, storage_liveness) =
compute_layout(tcx, source, &upvars, interior, movable, body);
compute_layout(tcx, source, &upvars, interior, &always_live_locals, movable, body);
let can_return = can_return(tcx, body);
@ -1294,6 +1300,7 @@ impl<'tcx> MirPass<'tcx> for StateTransform {
state_substs,
remap,
storage_liveness,
always_live_locals,
suspension_points: Vec::new(),
new_ret_local,
discr_ty,

1
src/librustc_mir/util/mod.rs
View file

@ -3,6 +3,7 @@ pub mod borrowck_errors;
pub mod def_use;
pub mod elaborate_drops;
pub mod patch;
pub mod storage;
mod alignment;
pub mod collect_writes;

47
src/librustc_mir/util/storage.rs Normal file
View file

@ -0,0 +1,47 @@
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{self, Local, Location};
/// The set of locals in a MIR body that do not have `StorageLive`/`StorageDead` annotations.
///
/// These locals have fixed storage for the duration of the body.
//
// FIXME: Currently, we need to traverse the entire MIR to compute this. We should instead store it
// as a field in the `LocalDecl` for each `Local`.
#[derive(Debug, Clone)]
pub struct AlwaysLiveLocals(BitSet<Local>);
impl AlwaysLiveLocals {
pub fn new(body: &mir::Body<'tcx>) -> Self {
let mut ret = AlwaysLiveLocals(BitSet::new_filled(body.local_decls.len()));
let mut vis = StorageAnnotationVisitor(&mut ret);
vis.visit_body(body);
ret
}
pub fn into_inner(self) -> BitSet<Local> {
self.0
}
}
impl std::ops::Deref for AlwaysLiveLocals {
type Target = BitSet<Local>;
fn deref(&self) -> &Self::Target {
&self.0
}
}
/// Removes locals that have `Storage*` annotations from `AlwaysLiveLocals`.
struct StorageAnnotationVisitor<'a>(&'a mut AlwaysLiveLocals);
impl Visitor<'tcx> for StorageAnnotationVisitor<'_> {
fn visit_statement(&mut self, statement: &mir::Statement<'tcx>, _location: Location) {
use mir::StatementKind::{StorageDead, StorageLive};
if let StorageLive(l) | StorageDead(l) = statement.kind {
(self.0).0.remove(l);
}
}
}