From 6c10d74a326d82026bcff3ec4b2da22ea9155584 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 19 Nov 2015 05:54:27 -0500 Subject: [PATCH] Remove the GraphExtents, the design of which seems bogus. They carried the right information, but it's hard to maintain in the face of optimizations, and in the form that the analyses probably actually want. --- src/librustc_mir/build/cfg.rs | 7 --- src/librustc_mir/build/expr/as_rvalue.rs | 2 +- src/librustc_mir/build/expr/into.rs | 2 +- src/librustc_mir/build/matches/mod.rs | 2 +- src/librustc_mir/build/mod.rs | 3 - src/librustc_mir/build/scope.rs | 75 ++++++++++-------------- src/librustc_mir/repr.rs | 44 -------------- 7 files changed, 34 insertions(+), 101 deletions(-) diff --git a/src/librustc_mir/build/cfg.rs b/src/librustc_mir/build/cfg.rs index 318ae704089..e44e3936885 100644 --- a/src/librustc_mir/build/cfg.rs +++ b/src/librustc_mir/build/cfg.rs @@ -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)); diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index e351215276a..3f3bceef1eb 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -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: diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 57c6db79c52..7d79e90b3f1 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -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() } diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index cc8549de26a..cc6155844bc 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -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]); } diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index eb03727d9b2..f67c2920ba3 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -19,7 +19,6 @@ use syntax::codemap::Span; struct Builder<'a, 'tcx: 'a> { hir: Cx<'a, 'tcx>, - extents: FnvHashMap>, cfg: CFG<'tcx>, scopes: Vec>, loop_scopes: Vec, @@ -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, diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index cecd610ff72..987424f4ac7 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -94,7 +94,6 @@ use syntax::codemap::Span; pub struct Scope<'tcx> { extent: CodeExtent, - exits: Vec, drops: Vec<(DropKind, Span, Lvalue<'tcx>)>, cached_block: Option, } @@ -116,7 +115,7 @@ impl<'a,'tcx> Builder<'a,'tcx> { -> BlockAnd where F: FnOnce(&mut Builder<'a, 'tcx>) -> BlockAnd { - 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,51 @@ 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(&mut self, extent: CodeExtent, block: BasicBlock, f: F) -> BlockAnd + /// Convenience wrapper that pushes a scope and then executes `f` + /// to build its contents, popping the scope afterwards. + pub fn in_scope(&mut self, extent: CodeExtent, mut block: BasicBlock, f: F) -> BlockAnd where F: FnOnce(&mut Builder<'a, 'tcx>) -> BlockAnd { debug!("in_scope(extent={:?}, block={:?})", extent, block); + self.push_scope(extent, block); + let rv = unpack!(block = f(self)); + assert_eq!(self.extent_of_innermost_scope(), extent); + self.pop_scope(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 the innermost scope, 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, block: BasicBlock) { + debug!("pop_scope({:?})", block); + let scope = self.scopes.pop().unwrap(); // 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) -> 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 +221,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 +259,12 @@ impl<'a,'tcx> Builder<'a,'tcx> { } } - pub fn extent_of_innermost_scope(&self) -> Option { - 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 { - self.scopes.first().map(|scope| scope.extent) + pub fn extent_of_outermost_scope(&self) -> CodeExtent { + self.scopes.first().map(|scope| scope.extent).unwrap() } } diff --git a/src/librustc_mir/repr.rs b/src/librustc_mir/repr.rs index 19b9e87701c..dad8961a788 100644 --- a/src/librustc_mir/repr.rs +++ b/src/librustc_mir/repr.rs @@ -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; @@ -156,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), -} - /////////////////////////////////////////////////////////////////////////// // BasicBlock