Mir predecessors cache invalidate on terminators

src/librustc/mir/mod.rs

@@ -26,7 +26,6 @@ use rustc_data_structures::graph::dominators::{dominators, Dominators};
 use rustc_data_structures::graph::{self, GraphPredecessors, GraphSuccessors};
 use rustc_data_structures::indexed_vec::{Idx, IndexVec};
 use rustc_data_structures::sync::Lrc;
-use rustc_data_structures::sync::MappedReadGuard;
 use rustc_macros::HashStable;
 use rustc_serialize::{Encodable, Decodable};
 use smallvec::SmallVec;
@@ -42,7 +41,6 @@ use syntax_pos::{Span, DUMMY_SP};
 
 pub use crate::mir::interpret::AssertMessage;
 
-mod cache;
 pub mod interpret;
 pub mod mono;
 pub mod tcx;
@@ -162,7 +160,7 @@ pub struct Body<'tcx> {
     pub span: Span,
 
     /// A cache for various calculations.
-    cache: cache::Cache,
+    predecessors_cache: Option<IndexVec<BasicBlock, Vec<BasicBlock>>>,
 }
 
 impl<'tcx> Body<'tcx> {
@@ -200,7 +198,7 @@ impl<'tcx> Body<'tcx> {
             __upvar_debuginfo_codegen_only_do_not_use,
             spread_arg: None,
             span,
-            cache: cache::Cache::new(),
+            predecessors_cache: None,
             control_flow_destroyed,
         }
     }
@@ -212,26 +210,73 @@ impl<'tcx> Body<'tcx> {
 
     #[inline]
     pub fn basic_blocks_mut(&mut self) -> &mut IndexVec<BasicBlock, BasicBlockData<'tcx>> {
-        self.cache.invalidate();
         &mut self.basic_blocks
     }
 
+    pub fn basic_block_terminator_opt_mut(&mut self, bb: BasicBlock) -> &mut Option<Terminator<'tcx>> {
+        self.predecessors_cache = None;
+        &mut self.basic_blocks[bb].terminator
+    }
+
+    pub fn basic_block_terminator_mut(&mut self, bb: BasicBlock) -> &mut Terminator<'tcx> {
+        self.predecessors_cache = None;
+/*
+        let data = &mut self.basic_blocks[bb];
+        if let Some(cache) = self.predecessors_cache.as_mut() {
+            for successor in data.terminator().successors() {
+                let successor_vec = &mut cache[*successor];
+                for i in (0..successor_vec.len()).rev() {
+                    if successor_vec[i] == bb {
+                        successor_vec.swap_remove(i);
+                        break;
+                    }
+                }
+            }
+        }
+*/
+
+        self.basic_blocks[bb].terminator_mut()
+    }
+
     #[inline]
     pub fn basic_blocks_and_local_decls_mut(
         &mut self,
     ) -> (&mut IndexVec<BasicBlock, BasicBlockData<'tcx>>, &mut LocalDecls<'tcx>) {
-        self.cache.invalidate();
         (&mut self.basic_blocks, &mut self.local_decls)
     }
 
     #[inline]
-    pub fn predecessors(&self) -> MappedReadGuard<'_, IndexVec<BasicBlock, Vec<BasicBlock>>> {
-        self.cache.predecessors(self)
+    pub fn unwrap_predecessors(&self) -> &IndexVec<BasicBlock, Vec<BasicBlock>> {
+        assert!(self.predecessors_cache.is_some());
+        self.predecessors_cache.as_ref().unwrap()
     }
 
     #[inline]
-    pub fn predecessors_for(&self, bb: BasicBlock) -> MappedReadGuard<'_, Vec<BasicBlock>> {
-        MappedReadGuard::map(self.predecessors(), |p| &p[bb])
+    pub fn predecessors(&mut self) -> &IndexVec<BasicBlock, Vec<BasicBlock>> {
+        if self.predecessors_cache.is_none() {
+            self.predecessors_cache = Some(self.calculate_predecessors())
+        }
+
+        self.predecessors_cache.as_ref().unwrap()
+    }
+
+    fn calculate_predecessors(&self) -> IndexVec<BasicBlock, Vec<BasicBlock>> {
+        let mut result = IndexVec::from_elem(vec![], self.basic_blocks());
+        for (bb, data) in self.basic_blocks().iter_enumerated() {
+            if let Some(ref term) = data.terminator {
+                for &tgt in term.successors() {
+                    result[tgt].push(bb);
+                }
+            }
+        }
+
+        result
+    }
+
+    #[inline]
+    pub fn predecessors_for(&self, bb: BasicBlock) -> &[BasicBlock] {
+        // TODO(nashenas88) could this be predecessors sometimes too?
+        &self.unwrap_predecessors()[bb]
     }
 
     #[inline]
@@ -422,7 +467,7 @@ impl_stable_hash_for!(struct Body<'tcx> {
     spread_arg,
     control_flow_destroyed,
     span,
-    cache
+    predecessors_cache
 });
 
 impl<'tcx> Index<BasicBlock> for Body<'tcx> {
@@ -1006,6 +1051,8 @@ impl BasicBlock {
     }
 }
 
+CloneTypeFoldableAndLiftImpls!{ BasicBlock, }
+
 ///////////////////////////////////////////////////////////////////////////
 // BasicBlockData and Terminator
 
@@ -1336,6 +1383,10 @@ 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
@@ -1344,10 +1395,17 @@ impl<'tcx> BasicBlockData<'tcx> {
         self.terminator.as_ref().expect("invalid terminator state")
     }
 
-    pub fn terminator_mut(&mut self) -> &mut Terminator<'tcx> {
+    // 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> {
         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,
@@ -2625,7 +2683,7 @@ impl<'tcx> graph::WithPredecessors for Body<'tcx> {
         &self,
         node: Self::Node,
     ) -> <Self as GraphPredecessors<'_>>::Iter {
-        self.predecessors_for(node).clone().into_iter()
+        self.predecessors_for(node).to_vec().into_iter()
     }
 }
 
@@ -2685,13 +2743,13 @@ impl Location {
         }
 
         // If we're in another block, then we want to check that block is a predecessor of `other`.
-        let mut queue: Vec<BasicBlock> = body.predecessors_for(other.block).clone();
+        let mut queue: Vec<BasicBlock> = body.predecessors_for(other.block).to_vec();
         let mut visited = FxHashSet::default();
 
         while let Some(block) = queue.pop() {
             // If we haven't visited this block before, then make sure we visit it's predecessors.
             if visited.insert(block) {
-                queue.append(&mut body.predecessors_for(block).clone());
+                queue.extend(body.predecessors_for(block).iter().cloned());
             } else {
                 continue;
             }
@@ -2964,7 +3022,7 @@ BraceStructTypeFoldableImpl! {
         spread_arg,
         control_flow_destroyed,
         span,
-        cache,
+        predecessors_cache,
     }
 }
 

src/librustc/mir/traversal.rs

@@ -55,7 +55,7 @@ impl<'a, 'tcx> Iterator for Preorder<'a, 'tcx> {
 
             let data = &self.body[idx];
 
-            if let Some(ref term) = data.terminator {
+            if let Some(ref term) = data.terminator_opt() {
                 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 {
+        if let Some(ref term) = data.terminator_opt() {
             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 {
+                if let Some(term) = &self.body[bb].terminator_opt() {
                     self.visit_stack.push((bb, term.successors()));
                 }
             }

src/librustc/mir/visit.rs

@@ -847,6 +847,7 @@ macro_rules! make_mir_visitor {
             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 {
+                    // TODO(nashenas88) how to ensure we clear the cache only in the mutable case...
                     if let Some(ref $($mutability)? terminator) = basic_block.terminator {
                         self.visit_terminator(terminator, location)
                     }

src/librustc_codegen_ssa/mir/mod.rs

@@ -369,7 +369,7 @@ fn create_funclets<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
 
         let funclet;
         let ret_llbb;
-        match mir[bb].terminator.as_ref().map(|t| &t.kind) {
+        match mir[bb].terminator_opt().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,

src/librustc_mir/borrow_check/error_reporting.rs

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

src/librustc_mir/build/cfg.rs

@@ -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.is_none(),
+        debug_assert!(self.block_data(block).terminator_opt().is_none(),
                       "terminate: block {:?}={:?} already has a terminator set",
                       block,
                       self.block_data(block));

src/librustc_mir/build/mod.rs

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

src/librustc_mir/dataflow/impls/borrows.rs

@@ -98,7 +98,7 @@ fn precompute_borrows_out_of_scope<'tcx>(
             // Add successor BBs to the work list, if necessary.
             let bb_data = &body[bb];
             assert!(hi == bb_data.statements.len());
-            for &succ_bb in bb_data.terminator.as_ref().unwrap().successors() {
+            for &succ_bb in bb_data.terminator().successors() {
                 visited.entry(succ_bb)
                     .and_modify(|lo| {
                         // `succ_bb` has been seen before. If it wasn't

src/librustc_mir/lints.rs

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

src/librustc_mir/transform/add_call_guards.rs

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