diff --git a/src/librustc_data_structures/bitvec.rs b/src/librustc_data_structures/bitvec.rs index 3677c8c5e593d..092b406ae9e3e 100644 --- a/src/librustc_data_structures/bitvec.rs +++ b/src/librustc_data_structures/bitvec.rs @@ -8,7 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::iter::FromIterator; + /// A very simple BitVector type. +#[derive(Clone)] pub struct BitVector { data: Vec, } @@ -50,7 +53,9 @@ impl BitVector { pub fn grow(&mut self, num_bits: usize) { let num_words = u64s(num_bits); let extra_words = self.data.len() - num_words; - self.data.extend((0..extra_words).map(|_| 0)); + if extra_words > 0 { + self.data.extend((0..extra_words).map(|_| 0)); + } } /// Iterates over indexes of set bits in a sorted order @@ -93,6 +98,27 @@ impl<'a> Iterator for BitVectorIter<'a> { } } +impl FromIterator for BitVector { + fn from_iter(iter: I) -> BitVector where I: IntoIterator { + let iter = iter.into_iter(); + let (len, _) = iter.size_hint(); + // Make the minimum length for the bitvector 64 bits since that's + // the smallest non-zero size anyway. + let len = if len < 64 { 64 } else { len }; + let mut bv = BitVector::new(len); + for (idx, val) in iter.enumerate() { + if idx > len { + bv.grow(idx); + } + if val { + bv.insert(idx); + } + } + + bv + } +} + /// A "bit matrix" is basically a square matrix of booleans /// represented as one gigantic bitvector. In other words, it is as if /// you have N bitvectors, each of length N. Note that `elements` here is `N`/ diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 468dc7b12c162..fbb185303e06d 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -876,10 +876,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, passes.push_pass(box mir::transform::remove_dead_blocks::RemoveDeadBlocks); passes.push_pass(box mir::transform::type_check::TypeckMir); passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg); - // Late passes - passes.push_pass(box mir::transform::no_landing_pads::NoLandingPads); passes.push_pass(box mir::transform::remove_dead_blocks::RemoveDeadBlocks); - passes.push_pass(box mir::transform::erase_regions::EraseRegions); // And run everything. passes.run_passes(tcx, &mut mir_map); }); @@ -932,16 +929,25 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, /// Run the translation phase to LLVM, after which the AST and analysis can pub fn phase_4_translate_to_llvm<'tcx>(tcx: &TyCtxt<'tcx>, - mir_map: MirMap<'tcx>, - analysis: ty::CrateAnalysis) - -> trans::CrateTranslation { + mut mir_map: MirMap<'tcx>, + analysis: ty::CrateAnalysis) -> trans::CrateTranslation { let time_passes = tcx.sess.time_passes(); time(time_passes, "resolving dependency formats", || dependency_format::calculate(&tcx.sess)); - // Option dance to work around the lack of stack once closures. + // Run the passes that transform the MIR into a more suitable for translation + // to LLVM code. + time(time_passes, "Prepare MIR codegen passes", || { + let mut passes = ::rustc::mir::transform::Passes::new(); + passes.push_pass(box mir::transform::no_landing_pads::NoLandingPads); + passes.push_pass(box mir::transform::remove_dead_blocks::RemoveDeadBlocks); + passes.push_pass(box mir::transform::erase_regions::EraseRegions); + passes.push_pass(box mir::transform::break_critical_edges::BreakCriticalEdges); + passes.run_passes(tcx, &mut mir_map); + }); + time(time_passes, "translation", move || trans::trans_crate(tcx, &mir_map, analysis)) diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index 67bac196f4806..cbbad057cf9c9 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -40,3 +40,4 @@ mod hair; pub mod mir_map; pub mod pretty; pub mod transform; +pub mod traversal; diff --git a/src/librustc_mir/transform/break_critical_edges.rs b/src/librustc_mir/transform/break_critical_edges.rs new file mode 100644 index 0000000000000..e1fb5dfd4374c --- /dev/null +++ b/src/librustc_mir/transform/break_critical_edges.rs @@ -0,0 +1,117 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use rustc::ty::TyCtxt; +use rustc::mir::repr::*; +use rustc::mir::transform::{MirPass, Pass}; +use syntax::ast::NodeId; + +use rustc_data_structures::bitvec::BitVector; + +use traversal; + +pub struct BreakCriticalEdges; + +/** + * Breaks critical edges in the MIR. + * + * Critical edges are edges that are neither the only edge leaving a + * block, nor the only edge entering one. + * + * When you want something to happen "along" an edge, you can either + * do at the end of the predecessor block, or at the start of the + * successor block. Critical edges have to be broken in order to prevent + * "edge actions" from affecting other edges. + * + * This function will break those edges by inserting new blocks along them. + * + * A special case is Drop and Call terminators with unwind/cleanup successors, + * They use `invoke` in LLVM, which terminates a block, meaning that code cannot + * be inserted after them, so even if an edge is the only edge leaving a block + * like that, we still insert blocks if the edge is one of many entering the + * target. + * + * NOTE: Simplify CFG will happily undo most of the work this pass does. + * + */ + +impl<'tcx> MirPass<'tcx> for BreakCriticalEdges { + fn run_pass(&mut self, _: &TyCtxt<'tcx>, _: NodeId, mir: &mut Mir<'tcx>) { + break_critical_edges(mir); + } +} + +impl Pass for BreakCriticalEdges {} + +fn break_critical_edges(mir: &mut Mir) { + let mut pred_count = vec![0u32; mir.basic_blocks.len()]; + + // Build the precedecessor map for the MIR + for (_, data) in traversal::preorder(mir) { + if let Some(ref term) = data.terminator { + for &tgt in term.successors().iter() { + pred_count[tgt.index()] += 1; + } + } + } + + let cleanup_map : BitVector = mir.basic_blocks + .iter().map(|bb| bb.is_cleanup).collect(); + + // We need a place to store the new blocks generated + let mut new_blocks = Vec::new(); + + let bbs = mir.all_basic_blocks(); + let cur_len = mir.basic_blocks.len(); + + for &bb in &bbs { + let data = mir.basic_block_data_mut(bb); + + if let Some(ref mut term) = data.terminator { + let is_invoke = term_is_invoke(term); + let term_span = term.span; + let term_scope = term.scope; + let succs = term.successors_mut(); + if succs.len() > 1 || (succs.len() > 0 && is_invoke) { + for tgt in succs { + let num_preds = pred_count[tgt.index()]; + if num_preds > 1 { + // It's a critical edge, break it + let goto = Terminator { + span: term_span, + scope: term_scope, + kind: TerminatorKind::Goto { target: *tgt } + }; + let mut data = BasicBlockData::new(Some(goto)); + data.is_cleanup = cleanup_map.contains(tgt.index()); + + // Get the index it will be when inserted into the MIR + let idx = cur_len + new_blocks.len(); + new_blocks.push(data); + *tgt = BasicBlock::new(idx); + } + } + } + } + } + + debug!("Broke {} N edges", new_blocks.len()); + + mir.basic_blocks.extend_from_slice(&new_blocks); +} + +// Returns true if the terminator would use an invoke in LLVM. +fn term_is_invoke(term: &Terminator) -> bool { + match term.kind { + TerminatorKind::Call { cleanup: Some(_), .. } | + TerminatorKind::Drop { unwind: Some(_), .. } => true, + _ => false + } +} diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 57690caeccb37..a52a8edc21173 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -13,3 +13,4 @@ pub mod simplify_cfg; pub mod erase_regions; pub mod no_landing_pads; pub mod type_check; +pub mod break_critical_edges; diff --git a/src/librustc_mir/traversal.rs b/src/librustc_mir/traversal.rs new file mode 100644 index 0000000000000..8b6821136f511 --- /dev/null +++ b/src/librustc_mir/traversal.rs @@ -0,0 +1,276 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::vec; + +use rustc_data_structures::bitvec::BitVector; + +use rustc::mir::repr::*; + +/// Preorder traversal of a graph. +/// +/// Preorder traversal is when each node is visited before an of it's +/// successors +/// +/// A +/// / \ +/// / \ +/// B C +/// \ / +/// \ / +/// D +/// +/// A preorder traversal of this graph is either `A B D C` or `A C D B` +#[derive(Clone)] +pub struct Preorder<'a, 'tcx: 'a> { + mir: &'a Mir<'tcx>, + visited: BitVector, + worklist: Vec, +} + +impl<'a, 'tcx> Preorder<'a, 'tcx> { + pub fn new(mir: &'a Mir<'tcx>, root: BasicBlock) -> Preorder<'a, 'tcx> { + let worklist = vec![root]; + + Preorder { + mir: mir, + visited: BitVector::new(mir.basic_blocks.len()), + worklist: worklist + } + } +} + +pub fn preorder<'a, 'tcx>(mir: &'a Mir<'tcx>) -> Preorder<'a, 'tcx> { + Preorder::new(mir, START_BLOCK) +} + +impl<'a, 'tcx> Iterator for Preorder<'a, 'tcx> { + type Item = (BasicBlock, &'a BasicBlockData<'tcx>); + + fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { + while let Some(idx) = self.worklist.pop() { + if !self.visited.insert(idx.index()) { + continue; + } + + let data = self.mir.basic_block_data(idx); + + if let Some(ref term) = data.terminator { + for &succ in term.successors().iter() { + self.worklist.push(succ); + } + } + + return Some((idx, data)); + } + + None + } +} + +/// Postorder traversal of a graph. +/// +/// Postorder traversal is when each node is visited after all of it's +/// successors, except when the successor is only reachable by a back-edge +/// +/// A +/// / \ +/// / \ +/// B C +/// \ / +/// \ / +/// D +/// +/// A Postorder traversal of this graph is `D B C A` or `D C B A` +pub struct Postorder<'a, 'tcx: 'a> { + mir: &'a Mir<'tcx>, + visited: BitVector, + visit_stack: Vec<(BasicBlock, vec::IntoIter)> +} + +impl<'a, 'tcx> Postorder<'a, 'tcx> { + pub fn new(mir: &'a Mir<'tcx>, root: BasicBlock) -> Postorder<'a, 'tcx> { + let mut po = Postorder { + mir: mir, + visited: BitVector::new(mir.basic_blocks.len()), + visit_stack: Vec::new() + }; + + + let data = po.mir.basic_block_data(root); + + if let Some(ref term) = data.terminator { + po.visited.insert(root.index()); + + let succs = term.successors().into_owned().into_iter(); + + po.visit_stack.push((root, succs)); + po.traverse_successor(); + } + + po + } + + fn traverse_successor(&mut self) { + // This is quite a complex loop due to 1. the borrow checker not liking it much + // and 2. what exactly is going on is not clear + // + // It does the actual traversal of the graph, while the `next` method on the iterator + // just pops off of the stack. `visit_stack` is a stack containing pairs of nodes and + // iterators over the sucessors of those nodes. Each iteration attempts to get the next + // node from the top of the stack, then pushes that node and an iterator over the + // successors to the top of the stack. This loop only grows `visit_stack`, stopping when + // we reach a child that has no children that we haven't already visited. + // + // For a graph that looks like this: + // + // A + // / \ + // / \ + // B C + // | | + // | | + // D | + // \ / + // \ / + // E + // + // The state of the stack starts out with just the root node (`A` in this case); + // [(A, [B, C])] + // + // When the first call to `traverse_sucessor` happens, the following happens: + // + // [(B, [D]), // `B` taken from the successors of `A`, pushed to the + // // top of the stack along with the successors of `B` + // (A, [C])] + // + // [(D, [E]), // `D` taken from successors of `B`, pushed to stack + // (B, []), + // (A, [C])] + // + // [(E, []), // `E` taken from successors of `D`, pushed to stack + // (D, []), + // (B, []), + // (A, [C])] + // + // Now that the top of the stack has no successors we can traverse, each item will + // be popped off during iteration until we get back to `A`. This yeilds [E, D, B]. + // + // When we yield `B` and call `traverse_successor`, we push `C` to the stack, but + // since we've already visited `E`, that child isn't added to the stack. The last + // two iterations yield `C` and finally `A` for a final traversal of [E, D, B, C, A] + loop { + let bb = if let Some(&mut (_, ref mut iter)) = self.visit_stack.last_mut() { + if let Some(bb) = iter.next() { + bb + } else { + break; + } + } else { + break; + }; + + if self.visited.insert(bb.index()) { + let data = self.mir.basic_block_data(bb); + + if let Some(ref term) = data.terminator { + let succs = term.successors().into_owned().into_iter(); + self.visit_stack.push((bb, succs)); + } + } + } + } +} + +pub fn postorder<'a, 'tcx>(mir: &'a Mir<'tcx>) -> Postorder<'a, 'tcx> { + Postorder::new(mir, START_BLOCK) +} + +impl<'a, 'tcx> Iterator for Postorder<'a, 'tcx> { + type Item = (BasicBlock, &'a BasicBlockData<'tcx>); + + fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { + let next = self.visit_stack.pop(); + if next.is_some() { + self.traverse_successor(); + } + + next.map(|(bb, _)| { + let data = self.mir.basic_block_data(bb); + (bb, data) + }) + } +} + +/// Reverse postorder traversal of a graph +/// +/// Reverse postorder is the reverse order of a postorder traversal. +/// This is different to a preorder traversal and represents a natural +/// linearisation of control-flow. +/// +/// A +/// / \ +/// / \ +/// B C +/// \ / +/// \ / +/// D +/// +/// A reverse postorder traversal of this graph is either `A B C D` or `A C B D` +/// Note that for a graph containing no loops (i.e. A DAG), this is equivalent to +/// a topological sort. +/// +/// Construction of a `ReversePostorder` traversal requires doing a full +/// postorder traversal of the graph, therefore this traversal should be +/// constructed as few times as possible. Use the `reset` method to be able +/// to re-use the traversal +#[derive(Clone)] +pub struct ReversePostorder<'a, 'tcx: 'a> { + mir: &'a Mir<'tcx>, + blocks: Vec, + idx: usize +} + +impl<'a, 'tcx> ReversePostorder<'a, 'tcx> { + pub fn new(mir: &'a Mir<'tcx>, root: BasicBlock) -> ReversePostorder<'a, 'tcx> { + let blocks : Vec<_> = Postorder::new(mir, root).map(|(bb, _)| bb).collect(); + + let len = blocks.len(); + + ReversePostorder { + mir: mir, + blocks: blocks, + idx: len + } + } + + pub fn reset(&mut self) { + self.idx = self.blocks.len(); + } +} + + +pub fn reverse_postorder<'a, 'tcx>(mir: &'a Mir<'tcx>) -> ReversePostorder<'a, 'tcx> { + ReversePostorder::new(mir, START_BLOCK) +} + +impl<'a, 'tcx> Iterator for ReversePostorder<'a, 'tcx> { + type Item = (BasicBlock, &'a BasicBlockData<'tcx>); + + fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { + if self.idx == 0 { return None; } + self.idx -= 1; + + self.blocks.get(self.idx).map(|&bb| { + let data = self.mir.basic_block_data(bb); + (bb, data) + }) + } +} diff --git a/src/librustc_trans/basic_block.rs b/src/librustc_trans/basic_block.rs index 919ea49b392c6..60bd3fb8ef1b8 100644 --- a/src/librustc_trans/basic_block.rs +++ b/src/librustc_trans/basic_block.rs @@ -49,4 +49,10 @@ impl BasicBlock { _ => None } } + + pub fn delete(self) { + unsafe { + llvm::LLVMDeleteBasicBlock(self.0); + } + } } diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index a60b03dd72413..1efbe5df1625f 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -300,33 +300,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { // Many different ways to call a function handled here if let Some(cleanup) = cleanup.map(|bb| self.bcx(bb)) { - // We translate the copy into a temporary block. The temporary block is - // necessary because the current block has already been terminated (by - // `invoke`) and we cannot really translate into the target block - // because: - // * The target block may have more than a single precedesor; - // * Some LLVM insns cannot have a preceeding store insn (phi, - // cleanuppad), and adding/prepending the store now may render - // those other instructions invalid. - // - // NB: This approach still may break some LLVM code. For example if the - // target block starts with a `phi` (which may only match on immediate - // precedesors), it cannot know about this temporary block thus - // resulting in an invalid code: - // - // this: - // … - // %0 = … - // %1 = invoke to label %temp … - // temp: - // store ty %1, ty* %dest - // br label %actualtargetblock - // actualtargetblock: ; preds: %temp, … - // phi … [%this, …], [%0, …] ; ERROR: phi requires to match only on - // ; immediate precedesors - - let ret_bcx = if destination.is_some() { - self.fcx.new_block("", None) + let ret_bcx = if let Some((_, target)) = *destination { + self.blocks[target.index()] } else { self.unreachable_block() }; @@ -343,15 +318,16 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { self.set_operand_dropped(bcx, op); }); - if let Some((_, target)) = *destination { + if destination.is_some() { let ret_bcx = ret_bcx.build(); - if let Some(ret_dest) = ret_dest { - fn_ty.ret.store(&ret_bcx, invokeret, ret_dest.llval); - } - for op in args { - self.set_operand_dropped(&ret_bcx, op); - } - ret_bcx.br(self.llblock(target)); + ret_bcx.at_start(|ret_bcx| { + if let Some(ret_dest) = ret_dest { + fn_ty.ret.store(&ret_bcx, invokeret, ret_dest.llval); + } + for op in args { + self.set_operand_dropped(&ret_bcx, op); + } + }); } } else { let llret = bcx.call(fn_ptr, &llargs, cleanup_bundle.as_ref()); diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs index 61536f3ecae87..91577fc654a46 100644 --- a/src/librustc_trans/mir/mod.rs +++ b/src/librustc_trans/mir/mod.rs @@ -19,7 +19,13 @@ use common::{self, Block, BlockAndBuilder, FunctionContext}; use std::ops::Deref; use std::rc::Rc; +use basic_block::BasicBlock; + +use rustc_data_structures::bitvec::BitVector; + use self::lvalue::{LvalueRef, get_dataptr, get_meta}; +use rustc_mir::traversal; + use self::operand::OperandRef; #[derive(Clone)] @@ -95,7 +101,7 @@ enum TempRef<'tcx> { /////////////////////////////////////////////////////////////////////////// -pub fn trans_mir<'blk, 'tcx>(fcx: &'blk FunctionContext<'blk, 'tcx>) { +pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) { let bcx = fcx.init(false, None).build(); let mir = bcx.mir(); @@ -132,8 +138,13 @@ pub fn trans_mir<'blk, 'tcx>(fcx: &'blk FunctionContext<'blk, 'tcx>) { let block_bcxs: Vec> = mir_blocks.iter() .map(|&bb|{ - // FIXME(#30941) this doesn't handle msvc-style exceptions - fcx.new_block(&format!("{:?}", bb), None) + 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) + } }) .collect(); @@ -142,7 +153,7 @@ pub fn trans_mir<'blk, 'tcx>(fcx: &'blk FunctionContext<'blk, 'tcx>) { bcx.br(start_bcx.llbb); let mut mircx = MirContext { - mir: mir, + mir: mir.clone(), fcx: fcx, llpersonalityslot: None, blocks: block_bcxs, @@ -152,11 +163,28 @@ pub fn trans_mir<'blk, 'tcx>(fcx: &'blk FunctionContext<'blk, 'tcx>) { args: args, }; - // Translate the body of each block - for &bb in &mir_blocks { + let mut visited = BitVector::new(mir_blocks.len()); + + let rpo = traversal::reverse_postorder(&mir); + // Translate the body of each block using reverse postorder + for (bb, _) in rpo { + visited.insert(bb.index()); mircx.trans_block(bb); } + // Remove blocks that haven't been visited, or have no + // predecessors. + for &bb in &mir_blocks { + let block = mircx.blocks[bb.index()]; + let block = BasicBlock(block.llbb); + // Unreachable block + if !visited.contains(bb.index()) { + block.delete(); + } else if block.pred_iter().count() == 0 { + block.delete(); + } + } + fcx.cleanup(); } diff --git a/src/test/run-pass/mir_trans_critical_edge.rs b/src/test/run-pass/mir_trans_critical_edge.rs new file mode 100644 index 0000000000000..320f40175926d --- /dev/null +++ b/src/test/run-pass/mir_trans_critical_edge.rs @@ -0,0 +1,53 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// This code produces a CFG with critical edges that, if we don't +// handle properly, will cause invalid codegen. + +#![feature(rustc_attrs)] + +enum State { + Both, + Front, + Back +} + +pub struct Foo { + state: State, + a: A, + b: B +} + +impl Foo +where A: Iterator, B: Iterator +{ + // This is the function we care about + #[rustc_mir] + fn next(&mut self) -> Option { + match self.state { + State::Both => match self.a.next() { + elt @ Some(..) => elt, + None => { + self.state = State::Back; + self.b.next() + } + }, + State::Front => self.a.next(), + State::Back => self.b.next(), + } + } +} + +// Make sure we actually translate a version of the function +pub fn do_stuff(mut f: Foo>, Box>>) { + let _x = f.next(); +} + +fn main() {}