user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Move predecessors cache invalidation back to basic_blocks_mut, add a couple more ensure_predecessors to prevent panics

Browse source
This commit is contained in:
Paul Daniel Faria 2019-10-04 00:55:28 -04:00
parent 8e8c97e5fd
commit ad734680af
18 changed files with 101 additions and 124 deletions
Download patch file Download diff file Expand all files Collapse all files

31
src/librustc/mir/mod.rs
View file

@ -203,25 +203,11 @@ impl<'tcx> Body<'tcx> {
#[inline]
pub fn basic_blocks_mut(&mut self) -> &mut IndexVec<BasicBlock, BasicBlockData<'tcx>> {
debug!("Clearing predecessors cache at: {:?}", self.span.data());
self.predecessors_cache = None;
&mut self.basic_blocks
}
pub fn basic_block_terminator_opt_mut(
&mut self, bb: BasicBlock
) -> &mut Option<Terminator<'tcx>> {
// FIXME we should look into improving the cache invalidation
debug!("Invalidating predecessors cache through opt terminator for block at : {:?}", self.span.data());
self.predecessors_cache = None;
&mut self.basic_blocks[bb].terminator
}
pub fn basic_block_terminator_mut(&mut self, bb: BasicBlock) -> &mut Terminator<'tcx> {
// FIXME we should look into improving the cache invalidation
debug!("Invalidating predecessors cache through terminator for block at : {:?}", self.span.data());
self.predecessors_cache = None;
self.basic_blocks[bb].terminator_mut()
}
#[inline]
pub fn basic_blocks_and_local_decls_mut(
&mut self,
@ -1370,10 +1356,6 @@ impl<'tcx> BasicBlockData<'tcx> {
BasicBlockData { statements: vec![], terminator, is_cleanup: false }
}
pub fn terminator_opt(&self) -> &Option<Terminator<'tcx>> {
&self.terminator
}
/// Accessor for terminator.
///
/// Terminator may not be None after construction of the basic block is complete. This accessor
@ -1382,17 +1364,10 @@ impl<'tcx> BasicBlockData<'tcx> {
self.terminator.as_ref().expect("invalid terminator state")
}
// This cannot be public since changing the terminator will break the predecessors cache in Body
// To do so outside of this module, use Body::basic_block_terminator_mut(BasicBlock)
fn terminator_mut(&mut self) -> &mut Terminator<'tcx> {
pub fn terminator_mut(&mut self) -> &mut Terminator<'tcx> {
self.terminator.as_mut().expect("invalid terminator state")
}
// This can be public since changing the kind will not break the predecessors cache in Body
pub fn terminator_kind_mut(&mut self) -> &mut TerminatorKind<'tcx> {
&mut self.terminator_mut().kind
}
pub fn retain_statements<F>(&mut self, mut f: F)
where
F: FnMut(&mut Statement<'_>) -> bool,

6
src/librustc/mir/traversal.rs
View file

@ -55,7 +55,7 @@ impl<'a, 'tcx> Iterator for Preorder<'a, 'tcx> {
let data = &self.body[idx];
if let Some(ref term) = data.terminator_opt() {
if let Some(ref term) = data.terminator {
self.worklist.extend(term.successors());
}
@ -117,7 +117,7 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> {
let data = &po.body[root];
if let Some(ref term) = data.terminator_opt() {
if let Some(ref term) = data.terminator {
po.visited.insert(root);
po.visit_stack.push((root, term.successors()));
po.traverse_successor();
@ -186,7 +186,7 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> {
};
if self.visited.insert(bb) {
if let Some(term) = &self.body[bb].terminator_opt() {
if let Some(term) = &self.body[bb].terminator {
self.visit_stack.push((bb, term.successors()));
}
}

35
src/librustc/mir/visit.rs
View file

@ -789,31 +789,18 @@ macro_rules! make_mir_visitor {
}
// Convenience methods
make_mir_visitor!{@make_visit_location, $($mutability)?}
}
};
(@make_visit_location, mut) => {
fn visit_location(&mut self, body: &mut Body<'tcx>, location: Location) {
if body[location.block].statements.len() == location.statement_index {
if let Some(ref mut terminator) = body.basic_block_terminator_opt_mut(location.block) {
self.visit_terminator(terminator, location)
}
} else {
let statement = &mut body[location.block].statements[location.statement_index];
self.visit_statement(statement, location)
}
}
};
(@make_visit_location, ) => {
fn visit_location(&mut self, body: &Body<'tcx>, location: Location) {
let basic_block = &body[location.block];
if basic_block.statements.len() == location.statement_index {
if let Some(ref terminator) = basic_block.terminator_opt() {
self.visit_terminator(terminator, location)
fn visit_location(&mut self, body: & $($mutability)? Body<'tcx>, location: Location) {
let basic_block = & $($mutability)? body[location.block];
if basic_block.statements.len() == location.statement_index {
if let Some(ref $($mutability)? terminator) = basic_block.terminator {
self.visit_terminator(terminator, location)
}
} else {
let statement = & $($mutability)?
basic_block.statements[location.statement_index];
self.visit_statement(statement, location)
}
} else {
let statement = &basic_block.statements[location.statement_index];
self.visit_statement(statement, location)
}
}
}

2
src/librustc_codegen_ssa/mir/mod.rs
View file

@ -265,7 +265,7 @@ fn create_funclets<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
let funclet;
let ret_llbb;
match mir[bb].terminator_opt().as_ref().map(|t| &t.kind) {
match mir[bb].terminator.as_ref().map(|t| &t.kind) {
// This is a basic block that we're aborting the program for,
// notably in an `extern` function. These basic blocks are inserted
// so that we assert that `extern` functions do indeed not panic,

2
src/librustc_mir/borrow_check/error_reporting.rs
View file

@ -497,7 +497,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
..
},
..
}) = bbd.terminator_opt() {
}) = bbd.terminator {
if let Some(source)
= BorrowedContentSource::from_call(func.ty(self.body, tcx), tcx)
{

5
src/librustc_mir/borrow_check/mod.rs
View file

@ -166,6 +166,11 @@ fn do_mir_borrowck<'a, 'tcx>(
let mut promoted: IndexVec<Promoted, Body<'tcx>> = input_promoted.clone();
let free_regions =
nll::replace_regions_in_mir(infcx, def_id, param_env, &mut body, &mut promoted);
// Region replacement above very likely invalidated the predecessors cache. It's used later on
// when retrieving the dominators from the body, so we need to ensure it exists before locking
// the body for changes.
body.ensure_predecessors();
let body = &body; // no further changes
let location_table = &LocationTable::new(body);

2
src/librustc_mir/build/cfg.rs
View file

@ -64,7 +64,7 @@ impl<'tcx> CFG<'tcx> {
source_info: SourceInfo,
kind: TerminatorKind<'tcx>) {
debug!("terminating block {:?} <- {:?}", block, kind);
debug_assert!(self.block_data(block).terminator_opt().is_none(),
debug_assert!(self.block_data(block).terminator.is_none(),
"terminate: block {:?}={:?} already has a terminator set",
block,
self.block_data(block));

2
src/librustc_mir/build/mod.rs
View file

@ -731,7 +731,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn finish(self) -> Body<'tcx> {
for (index, block) in self.cfg.basic_blocks.iter().enumerate() {
if block.terminator_opt().is_none() {
if block.terminator.is_none() {
span_bug!(self.fn_span, "no terminator on block {:?}", index);
}
}

2
src/librustc_mir/lints.rs
View file

@ -79,7 +79,7 @@ fn check_fn_for_unconditional_recursion(
let block = &basic_blocks[bb];
if let Some(ref terminator) = block.terminator_opt() {
if let Some(ref terminator) = block.terminator {
match terminator.kind {
TerminatorKind::Call { ref func, .. } => {
let func_ty = func.ty(body, tcx);

9
src/librustc_mir/shim.rs
View file

@ -123,6 +123,15 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx
&add_call_guards::CriticalCallEdges,
]);
// The `ensure_predecessors_cache::EnsurePredecessorsCache` MirPass wasn't used in the
// `run_passes` above because the above pass is not always guaranteed to run. There can be
// instances where, e.g. a `MirPhase::Validated` pass has already been run on a `Body` by the
// time it arrived at this line, and so the above `run_passes` call will NOT run any of the
// passes (They do not run if a same or later pass has already been executed on a `Body`).
// Adding the ensure pass during the `run_passes` for `MirPhase::Validated` would not
// help because the predecessors cache would be invalidated between that pass and this call.
// Having the single ensure outside of the `run_passes` list here guarantees that anyone
// using this `Body` could call `Body::unwrap_predecessors()` without worrying about a panic.
result.ensure_predecessors();
debug!("make_shim({:?}) = {:?}", instance, result);

9
src/librustc_mir/transform/add_call_guards.rs
View file

@ -46,9 +46,8 @@ impl AddCallGuards {
let cur_len = body.basic_blocks().len();
for bb in body.basic_blocks().indices() {
let is_cleanup = body.basic_blocks()[bb].is_cleanup;
match body.basic_block_terminator_opt_mut(bb) {
for block in body.basic_blocks_mut() {
match block.terminator {
Some(Terminator {
kind: TerminatorKind::Call {
destination: Some((_, ref mut destination)),
@ -61,9 +60,9 @@ impl AddCallGuards {
// It's a critical edge, break it
let call_guard = BasicBlockData {
statements: vec![],
is_cleanup: is_cleanup,
is_cleanup: block.is_cleanup,
terminator: Some(Terminator {
source_info: *source_info,
source_info,
kind: TerminatorKind::Goto { target: *destination }
})
};

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

@ -368,7 +368,7 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> {
VariantIdx::new(RETURNED) // state for returned
};
data.statements.push(self.set_discr(state, source_info));
*data.terminator_kind_mut() = TerminatorKind::Return;
data.terminator_mut().kind = TerminatorKind::Return;
}
self.super_basic_block_data(block, data);
@ -852,10 +852,10 @@ fn insert_switch<'tcx>(
is_cleanup: false,
});
for bb in body.basic_blocks_mut().indices() {
for target in body.basic_block_terminator_mut(bb).successors_mut() {
*target = BasicBlock::new(target.index() + 1);
}
let blocks = body.basic_blocks_mut().iter_mut();
for target in blocks.flat_map(|b| b.terminator_mut().successors_mut()) {
*target = BasicBlock::new(target.index() + 1);
}
}
@ -941,7 +941,7 @@ fn create_generator_drop_shim<'tcx>(
insert_switch(&mut body, cases, &transform, TerminatorKind::Return);
for block in body.basic_blocks_mut() {
let kind = block.terminator_kind_mut();
let kind = &mut block.terminator_mut().kind;
if let TerminatorKind::GeneratorDrop = *kind {
*kind = TerminatorKind::Return;
}
@ -1203,6 +1203,10 @@ impl<'tcx> MirPass<'tcx> for StateTransform {
// RETURN_PLACE then is a fresh unused local with type ret_ty.
let new_ret_local = replace_result_variable(ret_ty, body, tcx);
// Replacing result variables very likely clears the predecessors cache (needed inside of
// compute layout), so we need to ensure the cache exists.
body.ensure_predecessors();
// 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

6
src/librustc_mir/transform/inline.rs
View file

@ -379,7 +379,7 @@ impl Inliner<'tcx> {
callsite: CallSite<'tcx>,
caller_body: &mut Body<'tcx>,
mut callee_body: Body<'tcx>) -> bool {
let terminator = caller_body.basic_block_terminator_opt_mut(callsite.bb).take().unwrap();
let terminator = caller_body[callsite.bb].terminator.take().unwrap();
match terminator.kind {
// FIXME: Handle inlining of diverging calls
TerminatorKind::Call { args, destination: Some(destination), cleanup, .. } => {
@ -496,12 +496,12 @@ impl Inliner<'tcx> {
kind: TerminatorKind::Goto { target: BasicBlock::new(bb_len) }
};
*caller_body.basic_block_terminator_opt_mut(callsite.bb) = Some(terminator);
caller_body[callsite.bb].terminator= Some(terminator);
true
}
kind => {
*caller_body.basic_block_terminator_opt_mut(callsite.bb) = Some(Terminator {
caller_body[callsite.bb].terminator = Some(Terminator {
source_info: terminator.source_info,
kind,
});

17
src/librustc_mir/transform/promote_consts.rs
View file

@ -866,7 +866,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
let terminator = if self.keep_original {
self.source[loc.block].terminator().clone()
} else {
let terminator = self.source.basic_block_terminator_mut(loc.block);
let terminator = self.source[loc.block].terminator_mut();
let target = match terminator.kind {
TerminatorKind::Call { destination: Some((_, target)), .. } => target,
ref kind => {
@ -891,7 +891,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
let last = self.promoted.basic_blocks().last().unwrap();
let new_target = self.new_block();
*self.promoted.basic_block_terminator_mut(last) = Terminator {
*self.promoted[last].terminator_mut() = Terminator {
kind: TerminatorKind::Call {
func,
args,
@ -976,12 +976,11 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
}
},
Candidate::Argument { bb, index } => {
let data = &mut blocks[bb];
let terminator_span = data.terminator().source_info.span;
match data.terminator_kind_mut() {
let terminator = blocks[bb].terminator_mut();
match terminator.kind {
TerminatorKind::Call { ref mut args, .. } => {
let ty = args[index].ty(local_decls, self.tcx);
let span = terminator_span;
let span = terminator.source_info.span;
let operand = Operand::Copy(promoted_place(ty, span));
mem::replace(&mut args[index], operand)
}
@ -1104,8 +1103,8 @@ pub fn promote_candidates<'tcx>(
// Eliminate assignments to, and drops of promoted temps.
let promoted = |index: Local| temps[index] == TempState::PromotedOut;
for bb in body.basic_blocks().indices() {
body.basic_blocks_mut()[bb].statements.retain(|statement| {
for block in body.basic_blocks_mut() {
block.statements.retain(|statement| {
match &statement.kind {
StatementKind::Assign(box(place, _)) => {
if let Some(index) = place.as_local() {
@ -1121,7 +1120,7 @@ pub fn promote_candidates<'tcx>(
_ => true
}
});
let terminator = body.basic_block_terminator_mut(bb);
let terminator = block.terminator_mut();
match &terminator.kind {
TerminatorKind::Drop { location: place, target, .. } => {
if let Some(index) = place.as_local() {

4
src/librustc_mir/transform/remove_noop_landing_pads.rs
View file

@ -103,7 +103,7 @@ impl RemoveNoopLandingPads {
let postorder: Vec<_> = traversal::postorder(body).map(|(bb, _)| bb).collect();
for bb in postorder {
debug!(" processing {:?}", bb);
for target in body.basic_block_terminator_mut(bb).successors_mut() {
for target in body[bb].terminator_mut().successors_mut() {
if *target != resume_block && nop_landing_pads.contains(*target) {
debug!(" folding noop jump to {:?} to resume block", target);
*target = resume_block;
@ -111,7 +111,7 @@ impl RemoveNoopLandingPads {
}
}
match body.basic_block_terminator_mut(bb).unwind_mut() {
match body[bb].terminator_mut().unwind_mut() {
Some(unwind) => {
if *unwind == Some(resume_block) {
debug!(" removing noop landing pad");

71
src/librustc_mir/transform/simplify.rs
View file

@ -63,7 +63,7 @@ impl<'tcx> MirPass<'tcx> for SimplifyCfg {
}
pub struct CfgSimplifier<'a, 'tcx> {
body: &'a mut Body<'tcx>,
basic_blocks: &'a mut IndexVec<BasicBlock, BasicBlockData<'tcx>>,
pred_count: IndexVec<BasicBlock, u32>
}
@ -76,23 +76,21 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
pred_count[START_BLOCK] = 1;
for (_, data) in traversal::preorder(body) {
if let Some(ref term) = data.terminator_opt() {
if let Some(ref term) = data.terminator {
for &tgt in term.successors() {
pred_count[tgt] += 1;
}
}
}
let basic_blocks = body.basic_blocks_mut();
CfgSimplifier {
body,
basic_blocks,
pred_count,
}
}
fn basic_blocks(&mut self) -> &mut IndexVec<BasicBlock, BasicBlockData<'tcx>> {
self.body.basic_blocks_mut()
}
pub fn simplify(mut self) {
self.strip_nops();
@ -103,14 +101,14 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
self.collapse_goto_chain(&mut start, &mut changed);
for bb in self.body.basic_blocks().indices() {
for bb in self.basic_blocks.indices() {
if self.pred_count[bb] == 0 {
continue
}
debug!("simplifying {:?}", bb);
let mut terminator = self.body.basic_block_terminator_opt_mut(bb).take()
let mut terminator = self.basic_blocks[bb].terminator.take()
.expect("invalid terminator state");
for successor in terminator.successors_mut() {
@ -125,8 +123,12 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
inner_changed |= self.merge_successor(&mut new_stmts, &mut terminator);
changed |= inner_changed;
}
self.basic_blocks()[bb].statements.extend(new_stmts);
*self.body.basic_block_terminator_opt_mut(bb) = Some(terminator);
{
let data = &mut self.basic_blocks[bb];
data.statements.extend(new_stmts);
data.terminator = Some(terminator);
}
changed |= inner_changed;
}
@ -136,17 +138,17 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
if start != START_BLOCK {
debug_assert!(self.pred_count[START_BLOCK] == 0);
self.basic_blocks().swap(START_BLOCK, start);
self.basic_blocks.swap(START_BLOCK, start);
self.pred_count.swap(START_BLOCK, start);
// pred_count == 1 if the start block has no predecessor _blocks_.
if self.pred_count[START_BLOCK] > 1 {
for bb in self.basic_blocks().indices() {
for (bb, data) in self.basic_blocks.iter_enumerated_mut() {
if self.pred_count[bb] == 0 {
continue;
}
for target in self.body.basic_block_terminator_mut(bb).successors_mut() {
for target in data.terminator_mut().successors_mut() {
if *target == start {
*target = START_BLOCK;
}
@ -158,7 +160,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
// Collapse a goto chain starting from `start`
fn collapse_goto_chain(&mut self, start: &mut BasicBlock, changed: &mut bool) {
let mut terminator = match self.basic_blocks()[*start] {
let mut terminator = match self.basic_blocks[*start] {
BasicBlockData {
ref statements,
terminator: ref mut terminator @ Some(Terminator {
@ -177,7 +179,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
}
_ => unreachable!()
};
self.basic_blocks()[*start].terminator = terminator;
self.basic_blocks[*start].terminator = terminator;
debug!("collapsing goto chain from {:?} to {:?}", *start, target);
@ -209,7 +211,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
};
debug!("merging block {:?} into {:?}", target, terminator);
*terminator = match self.body.basic_block_terminator_opt_mut(target).take() {
*terminator = match self.basic_blocks[target].terminator.take() {
Some(terminator) => terminator,
None => {
// unreachable loop - this should not be possible, as we
@ -217,7 +219,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
return false
}
};
new_stmts.extend(self.basic_blocks()[target].statements.drain(..));
new_stmts.extend(self.basic_blocks[target].statements.drain(..));
self.pred_count[target] = 0;
true
@ -250,7 +252,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
}
fn strip_nops(&mut self) {
for blk in self.basic_blocks().iter_mut() {
for blk in self.basic_blocks.iter_mut() {
blk.statements.retain(|stmt| if let StatementKind::Nop = stmt.kind {
false
} else {
@ -266,27 +268,24 @@ pub fn remove_dead_blocks(body: &mut Body<'_>) {
seen.insert(bb.index());
}
let mut replacements: Vec<BasicBlock>;
{
let basic_blocks = body.basic_blocks_mut();
let basic_blocks = body.basic_blocks_mut();
let num_blocks = basic_blocks.len();
replacements = (0..num_blocks).map(BasicBlock::new).collect();
let mut used_blocks = 0;
for alive_index in seen.iter() {
replacements[alive_index] = BasicBlock::new(used_blocks);
if alive_index != used_blocks {
// Swap the next alive block data with the current available slot. Since
// alive_index is non-decreasing this is a valid operation.
basic_blocks.raw.swap(alive_index, used_blocks);
}
used_blocks += 1;
let num_blocks = basic_blocks.len();
let mut replacements : Vec<_> = (0..num_blocks).map(BasicBlock::new).collect();
let mut used_blocks = 0;
for alive_index in seen.iter() {
replacements[alive_index] = BasicBlock::new(used_blocks);
if alive_index != used_blocks {
// Swap the next alive block data with the current available slot. Since
// alive_index is non-decreasing this is a valid operation.
basic_blocks.raw.swap(alive_index, used_blocks);
}
basic_blocks.raw.truncate(used_blocks);
used_blocks += 1;
}
basic_blocks.raw.truncate(used_blocks);
for bb in body.basic_blocks().indices() {
for target in body.basic_block_terminator_mut(bb).successors_mut() {
for block in basic_blocks {
for target in block.terminator_mut().successors_mut() {
*target = replacements[target.index()];
}
}

4
src/librustc_mir/transform/simplify_branches.rs
View file

@ -21,8 +21,8 @@ impl<'tcx> MirPass<'tcx> for SimplifyBranches {
fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut Body<'tcx>) {
let param_env = tcx.param_env(src.def_id());
for bb in body.basic_blocks().indices() {
let terminator = body.basic_block_terminator_mut(bb);
for block in body.basic_blocks_mut() {
let terminator = block.terminator_mut();
terminator.kind = match terminator.kind {
TerminatorKind::SwitchInt {
discr: Operand::Constant(ref c), switch_ty, ref values, ref targets, ..

2
src/librustc_mir/util/patch.rs
View file

@ -141,7 +141,7 @@ impl<'tcx> MirPatch<'tcx> {
for (src, patch) in self.patch_map.into_iter_enumerated() {
if let Some(patch) = patch {
debug!("MirPatch: patching block {:?}", src);
body.basic_block_terminator_mut(src).kind = patch;
body[src].terminator_mut().kind = patch;
}
}