From 135657206fba40c3f6138362b62457f4452e578b Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sat, 16 Apr 2016 18:09:51 +0300 Subject: [PATCH 1/2] MIR: Do not require END_BLOCK to always exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once upon a time, along with START_BLOCK and END_BLOCK in the castle of important blocks also lived a RESUME_BLOCK (or was it UNWIND_BLOCK? Either works, I don’t remember anymore). This trinity of important blocks were required to always exist from the birth to death of the MIR-land they belonged to. Some time later, it was discovered that RESUME_BLOCK was just a lazy goon enjoying comfortable life in the light of fame of the other two. Needless to say, once found out, the RESUME_BLOCK was quickly slain and disposed of. Now, the all-seeing eye of ours discovers that END_BLOCK is actually the more evil and better disguised twin of the slain RESUME_BLOCK. Thus END_BLOCK gets slain and quickly disposed of. Glory to the START_BLOCK, one and only lord of the important blocks’ castle! --- Basically, all this does, is removing restriction for END_BLOCK to exist past the first invocation of RemoveDeadBlocks pass. This way for functions whose CFG does not reach the `END_BLOCK` end up not containing the block. As far as the implementation goes, I’m not entirely satisfied with the `BasicBlock::end_block`, I had hoped to make `new` a `const fn` and then just have a `const END_BLOCK` private to mir::build, but it turns out that constant functions don’t yet support conditionals nor a way to assert. --- src/librustc/mir/repr.rs | 13 ++++++++----- src/librustc_mir/build/expr/into.rs | 2 +- src/librustc_mir/build/mod.rs | 10 +++++----- src/librustc_mir/lib.rs | 1 + src/librustc_mir/transform/remove_dead_blocks.rs | 3 +-- src/librustc_trans/mir/mod.rs | 2 -- 6 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs index aacb3aae81c5e..67cd28c30e6b5 100644 --- a/src/librustc/mir/repr.rs +++ b/src/librustc/mir/repr.rs @@ -59,9 +59,6 @@ pub struct Mir<'tcx> { /// where execution begins pub const START_BLOCK: BasicBlock = BasicBlock(0); -/// where execution ends, on normal return -pub const END_BLOCK: BasicBlock = BasicBlock(1); - impl<'tcx> Mir<'tcx> { pub fn all_basic_blocks(&self) -> Vec { (0..self.basic_blocks.len()) @@ -216,6 +213,13 @@ impl BasicBlock { BasicBlock(index as u32) } + /// Returns a BasicBlock with index 1. This is actual end block (containing + /// the Return terminator) only during the building of MIR and should not be + /// used outside that. + pub const fn end_block() -> BasicBlock { + BasicBlock(1) + } + /// Extract the index. pub fn index(self) -> usize { self.0 as usize @@ -305,8 +309,7 @@ pub enum TerminatorKind<'tcx> { Resume, /// Indicates a normal return. The ReturnPointer lvalue should - /// have been filled in by now. This should only occur in the - /// `END_BLOCK`. + /// have been filled in by now. This should occur at most once. Return, /// Drop the Lvalue diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 231d7da10a02d..976d13fca29d6 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -262,7 +262,7 @@ impl<'a,'tcx> Builder<'a,'tcx> { } }; let extent = this.extent_of_return_scope(); - this.exit_scope(expr_span, extent, block, END_BLOCK); + this.exit_scope(expr_span, extent, block, BasicBlock::end_block()); this.cfg.start_new_block().unit() } ExprKind::Call { ty, fun, args } => { diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 2e5b6a952b72d..a92d686b85d12 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -183,7 +183,8 @@ pub fn construct<'a,'tcx>(hir: Cx<'a,'tcx>, }; assert_eq!(builder.cfg.start_new_block(), START_BLOCK); - assert_eq!(builder.cfg.start_new_block(), END_BLOCK); + let end_block = builder.cfg.start_new_block(); + assert_eq!(end_block, BasicBlock::end_block()); let mut arg_decls = None; // assigned to `Some` in closures below @@ -205,11 +206,10 @@ pub fn construct<'a,'tcx>(hir: Cx<'a,'tcx>, })); builder.cfg.terminate(block, call_site_scope_id, span, - TerminatorKind::Goto { target: END_BLOCK }); - builder.cfg.terminate(END_BLOCK, call_site_scope_id, span, + TerminatorKind::Goto { target: end_block }); + builder.cfg.terminate(end_block, call_site_scope_id, span, TerminatorKind::Return); - - END_BLOCK.unit() + end_block.unit() }); assert!( diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index ced73f34e0d92..78d3f4f6207d1 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -21,6 +21,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment! #![unstable(feature = "rustc_private", issue = "27812")] #![feature(box_patterns)] +#![feature(const_fn)] #![feature(rustc_private)] #![feature(staged_api)] #![feature(question_mark)] diff --git a/src/librustc_mir/transform/remove_dead_blocks.rs b/src/librustc_mir/transform/remove_dead_blocks.rs index dc1ddad124f87..2099e9a435a0f 100644 --- a/src/librustc_mir/transform/remove_dead_blocks.rs +++ b/src/librustc_mir/transform/remove_dead_blocks.rs @@ -43,9 +43,8 @@ pub struct RemoveDeadBlocks; impl<'tcx> MirPass<'tcx> for RemoveDeadBlocks { fn run_pass(&mut self, _: &TyCtxt<'tcx>, _: NodeId, mir: &mut Mir<'tcx>) { let mut seen = BitVector::new(mir.basic_blocks.len()); - // These blocks are always required. + // This block is always required. seen.insert(START_BLOCK.index()); - seen.insert(END_BLOCK.index()); let mut worklist = Vec::with_capacity(4); worklist.push(START_BLOCK); diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs index 1869845ccb189..eb907f496eb33 100644 --- a/src/librustc_trans/mir/mod.rs +++ b/src/librustc_trans/mir/mod.rs @@ -164,8 +164,6 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) { .map(|&bb|{ if bb == mir::START_BLOCK { fcx.new_block("start", None) - } else if bb == mir::END_BLOCK { - fcx.new_block("end", None) } else { fcx.new_block(&format!("{:?}", bb), None) } From d1180afbd99d533802f57f8c1fbf8c2723831ef2 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Wed, 20 Apr 2016 00:13:30 +0300 Subject: [PATCH 2/2] Generate block containing return lazily instead --- src/librustc/mir/repr.rs | 7 ---- src/librustc_mir/build/expr/into.rs | 3 +- src/librustc_mir/build/mod.rs | 50 ++++++++++++++++++----------- src/librustc_mir/lib.rs | 1 - 4 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs index 67cd28c30e6b5..d5c321ffa6a3e 100644 --- a/src/librustc/mir/repr.rs +++ b/src/librustc/mir/repr.rs @@ -213,13 +213,6 @@ impl BasicBlock { BasicBlock(index as u32) } - /// Returns a BasicBlock with index 1. This is actual end block (containing - /// the Return terminator) only during the building of MIR and should not be - /// used outside that. - pub const fn end_block() -> BasicBlock { - BasicBlock(1) - } - /// Extract the index. pub fn index(self) -> usize { self.0 as usize diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 976d13fca29d6..fe32f1de0c520 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -262,7 +262,8 @@ impl<'a,'tcx> Builder<'a,'tcx> { } }; let extent = this.extent_of_return_scope(); - this.exit_scope(expr_span, extent, block, BasicBlock::end_block()); + let return_block = this.return_block(); + this.exit_scope(expr_span, extent, block, return_block); this.cfg.start_new_block().unit() } ExprKind::Call { ty, fun, args } => { diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index a92d686b85d12..82916f08999b2 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -24,23 +24,23 @@ pub struct Builder<'a, 'tcx: 'a> { fn_span: Span, - // the current set of scopes, updated as we traverse; - // see the `scope` module for more details + /// the current set of scopes, updated as we traverse; + /// see the `scope` module for more details scopes: Vec>, - // for each scope, a span of blocks that defines it; - // we track these for use in region and borrow checking, - // but these are liable to get out of date once optimization - // begins. They are also hopefully temporary, and will be - // no longer needed when we adopt graph-based regions. + /// for each scope, a span of blocks that defines it; + /// we track these for use in region and borrow checking, + /// but these are liable to get out of date once optimization + /// begins. They are also hopefully temporary, and will be + /// no longer needed when we adopt graph-based regions. scope_auxiliary: ScopeAuxiliaryVec, - // the current set of loops; see the `scope` module for more - // details + /// the current set of loops; see the `scope` module for more + /// details loop_scopes: Vec, - // the vector of all scopes that we have created thus far; - // we track this for debuginfo later + /// the vector of all scopes that we have created thus far; + /// we track this for debuginfo later scope_datas: Vec, var_decls: Vec>, @@ -48,9 +48,11 @@ pub struct Builder<'a, 'tcx: 'a> { temp_decls: Vec>, unit_temp: Option>, - // cached block with a RESUME terminator; we create this at the - // first panic + /// cached block with the RESUME terminator; this is created + /// when first set of cleanups are built. cached_resume_block: Option, + /// cached block with the RETURN terminator + cached_return_block: Option, } struct CFG<'tcx> { @@ -180,12 +182,10 @@ pub fn construct<'a,'tcx>(hir: Cx<'a,'tcx>, var_indices: FnvHashMap(), unit_temp: None, cached_resume_block: None, + cached_return_block: None }; assert_eq!(builder.cfg.start_new_block(), START_BLOCK); - let end_block = builder.cfg.start_new_block(); - assert_eq!(end_block, BasicBlock::end_block()); - let mut arg_decls = None; // assigned to `Some` in closures below let call_site_extent = @@ -205,11 +205,12 @@ pub fn construct<'a,'tcx>(hir: Cx<'a,'tcx>, block.unit() })); + let return_block = builder.return_block(); builder.cfg.terminate(block, call_site_scope_id, span, - TerminatorKind::Goto { target: end_block }); - builder.cfg.terminate(end_block, call_site_scope_id, span, + TerminatorKind::Goto { target: return_block }); + builder.cfg.terminate(return_block, call_site_scope_id, span, TerminatorKind::Return); - end_block.unit() + return_block.unit() }); assert!( @@ -290,6 +291,17 @@ impl<'a,'tcx> Builder<'a,'tcx> { } } } + + fn return_block(&mut self) -> BasicBlock { + match self.cached_return_block { + Some(rb) => rb, + None => { + let rb = self.cfg.start_new_block(); + self.cached_return_block = Some(rb); + rb + } + } + } } /////////////////////////////////////////////////////////////////////////// diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index 78d3f4f6207d1..ced73f34e0d92 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -21,7 +21,6 @@ Rust MIR: a lowered representation of Rust. Also: an experiment! #![unstable(feature = "rustc_private", issue = "27812")] #![feature(box_patterns)] -#![feature(const_fn)] #![feature(rustc_private)] #![feature(staged_api)] #![feature(question_mark)]