Auto merge of #30044 - nikomatsakis:issue-29466, r=arielb1

The graph extent mechanism is not good. I have some ideas for a better replacement, but this PR simply removes it. It also stops recursing on statement scopes and processes them using an "on the heap" stack, which fixes #29466.

r? @dotdash
This commit is contained in:
bors 2015-11-25 18:49:56 +00:00
commit 1bb91be05f
9 changed files with 3717 additions and 145 deletions

View File

@ -26,13 +26,6 @@ impl<'tcx> CFG<'tcx> {
&mut self.basic_blocks[blk.index()]
}
pub fn end_point(&self, block: BasicBlock) -> ExecutionPoint {
ExecutionPoint {
block: block,
statement: self.block_data(block).statements.len() as u32,
}
}
pub fn start_new_block(&mut self) -> BasicBlock {
let node_index = self.basic_blocks.len();
self.basic_blocks.push(BasicBlockData::new(Terminator::Diverge));

View File

@ -70,7 +70,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
this.cfg.push_assign(block, expr_span, &result, rvalue);
// schedule a shallow free of that memory, lest we unwind:
let extent = this.extent_of_innermost_scope().unwrap();
let extent = this.extent_of_innermost_scope();
this.schedule_drop(expr_span, extent, DropKind::Free, &result, value_ty);
// initialize the box contents:

View File

@ -206,7 +206,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
}
ExprKind::Return { value } => {
unpack!(block = this.into(&Lvalue::ReturnPointer, block, value));
let extent = this.extent_of_outermost_scope().unwrap();
let extent = this.extent_of_outermost_scope();
this.exit_scope(expr_span, extent, block, END_BLOCK);
this.cfg.start_new_block().unit()
}

View File

@ -42,7 +42,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
// suitable extent for all of the bindings in this match. It's
// easiest to do this up front because some of these arms may
// be unreachable or reachable multiple times.
let var_extent = self.extent_of_innermost_scope().unwrap();
let var_extent = self.extent_of_innermost_scope();
for arm in &arms {
self.declare_bindings(var_extent, &arm.patterns[0]);
}

View File

@ -19,7 +19,6 @@ use syntax::codemap::Span;
struct Builder<'a, 'tcx: 'a> {
hir: Cx<'a, 'tcx>,
extents: FnvHashMap<CodeExtent, Vec<GraphExtent>>,
cfg: CFG<'tcx>,
scopes: Vec<scope::Scope<'tcx>>,
loop_scopes: Vec<scope::LoopScope>,
@ -92,7 +91,6 @@ pub fn construct<'a,'tcx>(mut hir: Cx<'a,'tcx>,
let mut builder = Builder {
hir: hir,
cfg: cfg,
extents: FnvHashMap(),
scopes: vec![],
loop_scopes: vec![],
temp_decls: temp_decls,
@ -117,7 +115,6 @@ pub fn construct<'a,'tcx>(mut hir: Cx<'a,'tcx>,
Mir {
basic_blocks: builder.cfg.basic_blocks,
extents: builder.extents,
var_decls: builder.var_decls,
arg_decls: arg_decls,
temp_decls: builder.temp_decls,

View File

@ -94,7 +94,6 @@ use syntax::codemap::Span;
pub struct Scope<'tcx> {
extent: CodeExtent,
exits: Vec<ExecutionPoint>,
drops: Vec<(DropKind, Span, Lvalue<'tcx>)>,
cached_block: Option<BasicBlock>,
}
@ -116,7 +115,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
-> BlockAnd<R>
where F: FnOnce(&mut Builder<'a, 'tcx>) -> BlockAnd<R>
{
let extent = self.extent_of_innermost_scope().unwrap();
let extent = self.extent_of_innermost_scope();
let loop_scope = LoopScope {
extent: extent.clone(),
continue_block: loop_block,
@ -128,61 +127,52 @@ impl<'a,'tcx> Builder<'a,'tcx> {
r
}
/// Start a scope. The closure `f` should translate the contents
/// of the scope. See module comment for more details.
pub fn in_scope<F, R>(&mut self, extent: CodeExtent, block: BasicBlock, f: F) -> BlockAnd<R>
/// Convenience wrapper that pushes a scope and then executes `f`
/// to build its contents, popping the scope afterwards.
pub fn in_scope<F, R>(&mut self, extent: CodeExtent, mut block: BasicBlock, f: F) -> BlockAnd<R>
where F: FnOnce(&mut Builder<'a, 'tcx>) -> BlockAnd<R>
{
debug!("in_scope(extent={:?}, block={:?})", extent, block);
self.push_scope(extent, block);
let rv = unpack!(block = f(self));
self.pop_scope(extent, block);
debug!("in_scope: exiting extent={:?} block={:?}", extent, block);
block.and(rv)
}
let start_point = self.cfg.end_point(block);
/// Push a scope onto the stack. You can then build code in this
/// scope and call `pop_scope` afterwards. Note that these two
/// calls must be paired; using `in_scope` as a convenience
/// wrapper maybe preferable.
pub fn push_scope(&mut self, extent: CodeExtent, block: BasicBlock) {
debug!("push_scope({:?}, {:?})", extent, block);
// push scope, execute `f`, then pop scope again
self.scopes.push(Scope {
extent: extent.clone(),
drops: vec![],
exits: vec![],
cached_block: None,
});
let BlockAnd(fallthrough_block, rv) = f(self);
let mut scope = self.scopes.pop().unwrap();
}
/// Pops a scope, which should have extent `extent`, adding any
/// drops onto the end of `block` that are needed. This must
/// match 1-to-1 with `push_scope`.
pub fn pop_scope(&mut self, extent: CodeExtent, block: BasicBlock) {
debug!("pop_scope({:?}, {:?})", extent, block);
let scope = self.scopes.pop().unwrap();
assert_eq!(scope.extent, extent);
// add in any drops needed on the fallthrough path (any other
// exiting paths, such as those that arise from `break`, will
// have drops already)
for (kind, span, lvalue) in scope.drops {
self.cfg.push_drop(fallthrough_block, span, kind, &lvalue);
}
// add the implicit fallthrough edge
scope.exits.push(self.cfg.end_point(fallthrough_block));
// compute the extent from start to finish and store it in the graph
let graph_extent = self.graph_extent(start_point, scope.exits);
self.extents.entry(extent)
.or_insert(vec![])
.push(graph_extent);
debug!("in_scope: exiting extent={:?} fallthrough_block={:?}", extent, fallthrough_block);
fallthrough_block.and(rv)
}
/// Creates a graph extent (SEME region) from an entry point and
/// exit points.
fn graph_extent(&self, entry: ExecutionPoint, exits: Vec<ExecutionPoint>) -> GraphExtent {
if exits.len() == 1 && entry.block == exits[0].block {
GraphExtent {
entry: entry,
exit: GraphExtentExit::Statement(exits[0].statement),
}
} else {
GraphExtent {
entry: entry,
exit: GraphExtentExit::Points(exits),
}
self.cfg.push_drop(block, span, kind, &lvalue);
}
}
/// Finds the loop scope for a given label. This is used for
/// resolving `break` and `continue`.
pub fn find_loop_scope(&mut self,
@ -232,8 +222,6 @@ impl<'a,'tcx> Builder<'a,'tcx> {
for &(kind, drop_span, ref lvalue) in &scope.drops {
self.cfg.push_drop(block, drop_span, kind, lvalue);
}
scope.exits.push(self.cfg.end_point(block));
}
self.cfg.terminate(block, Terminator::Goto { target: target });
@ -272,12 +260,12 @@ impl<'a,'tcx> Builder<'a,'tcx> {
}
}
pub fn extent_of_innermost_scope(&self) -> Option<CodeExtent> {
self.scopes.last().map(|scope| scope.extent)
pub fn extent_of_innermost_scope(&self) -> CodeExtent {
self.scopes.last().map(|scope| scope.extent).unwrap()
}
pub fn extent_of_outermost_scope(&self) -> Option<CodeExtent> {
self.scopes.first().map(|scope| scope.extent)
pub fn extent_of_outermost_scope(&self) -> CodeExtent {
self.scopes.first().map(|scope| scope.extent).unwrap()
}
}

View File

@ -14,48 +14,70 @@ use repr::*;
impl<'a,'tcx> Builder<'a,'tcx> {
pub fn stmts(&mut self, mut block: BasicBlock, stmts: Vec<StmtRef<'tcx>>) -> BlockAnd<()> {
for stmt in stmts {
unpack!(block = self.stmt(block, stmt));
// This convoluted structure is to avoid using recursion as we walk down a list
// of statements. Basically, the structure we get back is something like:
//
// let x = <init> in {
// let y = <init> in {
// expr1;
// expr2;
// }
// }
//
// To process this, we keep a stack of (Option<CodeExtent>,
// vec::IntoIter<Stmt>) pairs. At each point we pull off the
// top most pair and extract one statement from the
// iterator. Once it's complete, we pop the scope from the
// first half the pair.
let this = self;
let mut stmt_lists = vec![(None, stmts.into_iter())];
while !stmt_lists.is_empty() {
let stmt = {
let &mut (_, ref mut stmts) = stmt_lists.last_mut().unwrap();
stmts.next()
};
let stmt = match stmt {
Some(stmt) => stmt,
None => {
let (extent, _) = stmt_lists.pop().unwrap();
if let Some(extent) = extent {
this.pop_scope(extent, block);
}
continue
}
};
let Stmt { span, kind } = this.hir.mirror(stmt);
match kind {
StmtKind::Let { remainder_scope, init_scope, pattern, initializer, stmts } => {
this.push_scope(remainder_scope, block);
stmt_lists.push((Some(remainder_scope), stmts.into_iter()));
unpack!(block = this.in_scope(init_scope, block, move |this| {
// FIXME #30046 ^~~~
match initializer {
Some(initializer) => {
this.expr_into_pattern(block, remainder_scope, pattern, initializer)
}
None => {
this.declare_bindings(remainder_scope, &pattern);
block.unit()
}
}
}));
}
StmtKind::Expr { scope, expr } => {
unpack!(block = this.in_scope(scope, block, |this| {
let expr = this.hir.mirror(expr);
let temp = this.temp(expr.ty.clone());
unpack!(block = this.into(&temp, block, expr));
this.cfg.push_drop(block, span, DropKind::Deep, &temp);
block.unit()
}));
}
}
}
block.unit()
}
pub fn stmt(&mut self, mut block: BasicBlock, stmt: StmtRef<'tcx>) -> BlockAnd<()> {
let this = self;
let Stmt { span, kind } = this.hir.mirror(stmt);
match kind {
StmtKind::Let { remainder_scope,
init_scope,
pattern,
initializer: Some(initializer),
stmts } => {
this.in_scope(remainder_scope, block, |this| {
unpack!(block = this.in_scope(init_scope, block, |this| {
this.expr_into_pattern(block, remainder_scope, pattern, initializer)
}));
this.stmts(block, stmts)
})
}
StmtKind::Let { remainder_scope, init_scope, pattern, initializer: None, stmts } => {
this.in_scope(remainder_scope, block, |this| {
unpack!(block = this.in_scope(init_scope, block, |this| {
this.declare_bindings(remainder_scope, &pattern);
block.unit()
}));
this.stmts(block, stmts)
})
}
StmtKind::Expr { scope, expr } => {
this.in_scope(scope, block, |this| {
let expr = this.hir.mirror(expr);
let temp = this.temp(expr.ty.clone());
unpack!(block = this.into(&temp, block, expr));
this.cfg.push_drop(block, span, DropKind::Deep, &temp);
block.unit()
})
}
}
}
}

View File

@ -10,11 +10,9 @@
use rustc::middle::const_eval::ConstVal;
use rustc::middle::def_id::DefId;
use rustc::middle::region::CodeExtent;
use rustc::middle::subst::Substs;
use rustc::middle::ty::{AdtDef, ClosureSubsts, FnOutput, Region, Ty};
use rustc_back::slice;
use rustc_data_structures::fnv::FnvHashMap;
use rustc_front::hir::InlineAsm;
use syntax::ast::Name;
use syntax::codemap::Span;
@ -23,15 +21,24 @@ use std::u32;
/// Lowered representation of a single function.
pub struct Mir<'tcx> {
/// List of basic blocks. References to basic block use a newtyped index type `BasicBlock`
/// that indexes into this vector.
pub basic_blocks: Vec<BasicBlockData<'tcx>>,
/// Return type of the function.
pub return_ty: FnOutput<'tcx>,
// for every node id
pub extents: FnvHashMap<CodeExtent, Vec<GraphExtent>>,
/// Variables: these are stack slots corresponding to user variables. They may be
/// assigned many times.
pub var_decls: Vec<VarDecl<'tcx>>,
/// Args: these are stack slots corresponding to the input arguments.
pub arg_decls: Vec<ArgDecl<'tcx>>,
/// Temp declarations: stack slots that for temporaries created by
/// the compiler. These are assigned once, but they are not SSA
/// values in that it is possible to borrow them and mutate them
/// through the resulting reference.
pub temp_decls: Vec<TempDecl<'tcx>>,
}
@ -147,48 +154,6 @@ pub struct ArgDecl<'tcx> {
pub ty: Ty<'tcx>,
}
///////////////////////////////////////////////////////////////////////////
// Graph extents
/// A moment in the flow of execution. It corresponds to a point in
/// between two statements:
///
/// BB[block]:
/// <--- if statement == 0
/// STMT[0]
/// <--- if statement == 1
/// STMT[1]
/// ...
/// <--- if statement == n-1
/// STMT[n-1]
/// <--- if statement == n
///
/// where the block has `n` statements.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub struct ExecutionPoint {
pub block: BasicBlock,
pub statement: u32,
}
/// A single-entry-multiple-exit region in the graph. We build one of
/// these for every node-id during MIR construction. By construction
/// we are assured that the entry dominates all points within, and
/// that, for every interior point X, it is postdominated by some exit.
pub struct GraphExtent {
pub entry: ExecutionPoint,
pub exit: GraphExtentExit,
}
pub enum GraphExtentExit {
/// `Statement(X)`: a very common special case covering a span
/// that is local to a single block. It starts at the entry point
/// and extends until the start of statement `X` (non-inclusive).
Statement(u32),
/// The more general case where the exits are a set of points.
Points(Vec<ExecutionPoint>),
}
///////////////////////////////////////////////////////////////////////////
// BasicBlock

File diff suppressed because it is too large Load Diff