trans: Don't try to place declarations during codegen unit partitioning.

Author: michaelwoerister

src/librustc_trans/base.rs

@@ -80,7 +80,7 @@ use machine::{llalign_of_min, llsize_of, llsize_of_real};
 use meth;
 use mir;
 use monomorphize::{self, Instance};
-use partitioning::{self, PartitioningStrategy, InstantiationMode, CodegenUnit};
+use partitioning::{self, PartitioningStrategy, CodegenUnit};
 use symbol_names_test;
 use trans_item::TransItem;
 use tvec;
@@ -2942,8 +2942,8 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a
         None => TransItemCollectionMode::Lazy
     };
 
-    let (items, reference_map) = time(time_passes, "translation item collection", || {
-        collector::collect_crate_translation_items(scx, collection_mode)
+    let (items, inlining_map) = time(time_passes, "translation item collection", || {
+        collector::collect_crate_translation_items(&scx, collection_mode)
     });
 
     let strategy = if scx.sess().opts.debugging_opts.incremental.is_some() {
@@ -2956,7 +2956,7 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a
         partitioning::partition(scx.tcx(),
                                 items.iter().cloned(),
                                 strategy,
-                                &reference_map)
+                                &inlining_map)
     });
 
     if scx.sess().opts.debugging_opts.print_trans_items.is_some() {
@@ -2984,18 +2984,17 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a
                     output.push_str(&cgu_name[..]);
 
                     let linkage_abbrev = match linkage {
-                        InstantiationMode::Def(llvm::ExternalLinkage) => "External",
-                        InstantiationMode::Def(llvm::AvailableExternallyLinkage) => "Available",
-                        InstantiationMode::Def(llvm::LinkOnceAnyLinkage) => "OnceAny",
-                        InstantiationMode::Def(llvm::LinkOnceODRLinkage) => "OnceODR",
-                        InstantiationMode::Def(llvm::WeakAnyLinkage) => "WeakAny",
-                        InstantiationMode::Def(llvm::WeakODRLinkage) => "WeakODR",
-                        InstantiationMode::Def(llvm::AppendingLinkage) => "Appending",
-                        InstantiationMode::Def(llvm::InternalLinkage) => "Internal",
-                        InstantiationMode::Def(llvm::PrivateLinkage) => "Private",
-                        InstantiationMode::Def(llvm::ExternalWeakLinkage) => "ExternalWeak",
-                        InstantiationMode::Def(llvm::CommonLinkage) => "Common",
-                        InstantiationMode::Decl => "Declaration",
+                        llvm::ExternalLinkage => "External",
+                        llvm::AvailableExternallyLinkage => "Available",
+                        llvm::LinkOnceAnyLinkage => "OnceAny",
+                        llvm::LinkOnceODRLinkage => "OnceODR",
+                        llvm::WeakAnyLinkage => "WeakAny",
+                        llvm::WeakODRLinkage => "WeakODR",
+                        llvm::AppendingLinkage => "Appending",
+                        llvm::InternalLinkage => "Internal",
+                        llvm::PrivateLinkage => "Private",
+                        llvm::ExternalWeakLinkage => "ExternalWeak",
+                        llvm::CommonLinkage => "Common",
                     };
 
                     output.push_str("[");

src/librustc_trans/collector.rs

@@ -188,8 +188,6 @@
 //! this is not implemented however: a translation item will be produced
 //! regardless of whether it is actually needed or not.
 
-use rustc_data_structures::bitvec::BitVector;
-
 use rustc::hir;
 use rustc::hir::intravisit as hir_visit;
 
@@ -226,42 +224,33 @@ pub enum TransItemCollectionMode {
 
 /// Maps every translation item to all translation items it references in its
 /// body.
-pub struct ReferenceMap<'tcx> {
-    // Maps a source translation item to a range of target translation items.
+pub struct InliningMap<'tcx> {
+    // Maps a source translation item to a range of target translation items
+    // that are potentially inlined by LLVM into the source.
     // The two numbers in the tuple are the start (inclusive) and
-    // end index (exclusive) within the `targets` and the `inlined` vecs.
+    // end index (exclusive) within the `targets` vecs.
     index: FnvHashMap<TransItem<'tcx>, (usize, usize)>,
     targets: Vec<TransItem<'tcx>>,
-    inlined: BitVector
 }
 
-impl<'tcx> ReferenceMap<'tcx> {
+impl<'tcx> InliningMap<'tcx> {
 
-    fn new() -> ReferenceMap<'tcx> {
-        ReferenceMap {
+    fn new() -> InliningMap<'tcx> {
+        InliningMap {
             index: FnvHashMap(),
             targets: Vec::new(),
-            inlined: BitVector::new(64 * 256),
         }
     }
 
-    fn record_references<I>(&mut self, source: TransItem<'tcx>, targets: I)
-        where I: Iterator<Item=(TransItem<'tcx>, bool)>
+    fn record_inlining_canditates<I>(&mut self,
+                                     source: TransItem<'tcx>,
+                                     targets: I)
+        where I: Iterator<Item=TransItem<'tcx>>
     {
         assert!(!self.index.contains_key(&source));
 
         let start_index = self.targets.len();
-
-        for (target, inlined) in targets {
-            let index = self.targets.len();
-            self.targets.push(target);
-            self.inlined.grow(index + 1);
-
-            if inlined {
-                self.inlined.insert(index);
-            }
-        }
-
+        self.targets.extend(targets);
         let end_index = self.targets.len();
         self.index.insert(source, (start_index, end_index));
     }
@@ -272,28 +261,17 @@ impl<'tcx> ReferenceMap<'tcx> {
         where F: FnMut(TransItem<'tcx>) {
         if let Some(&(start_index, end_index)) = self.index.get(&source)
         {
-            for index in start_index .. end_index {
-                if self.inlined.contains(index) {
-                    f(self.targets[index])
-                }
+            for candidate in &self.targets[start_index .. end_index] {
+                f(*candidate)
             }
         }
     }
-
-    pub fn get_direct_references_from(&self, source: TransItem<'tcx>) -> &[TransItem<'tcx>]
-    {
-        if let Some(&(start_index, end_index)) = self.index.get(&source) {
-            &self.targets[start_index .. end_index]
-        } else {
-            &self.targets[0 .. 0]
-        }
-    }
 }
 
 pub fn collect_crate_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,
                                                  mode: TransItemCollectionMode)
                                                  -> (FnvHashSet<TransItem<'tcx>>,
-                                                     ReferenceMap<'tcx>) {
+                                                     InliningMap<'tcx>) {
     // We are not tracking dependencies of this pass as it has to be re-executed
     // every time no matter what.
     scx.tcx().dep_graph.with_ignore(|| {
@@ -302,17 +280,17 @@ pub fn collect_crate_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 't
         debug!("Building translation item graph, beginning at roots");
         let mut visited = FnvHashSet();
         let mut recursion_depths = DefIdMap();
-        let mut reference_map = ReferenceMap::new();
+        let mut inlining_map = InliningMap::new();
 
         for root in roots {
             collect_items_rec(scx,
                               root,
                               &mut visited,
                               &mut recursion_depths,
-                              &mut reference_map);
+                              &mut inlining_map);
         }
 
-        (visited, reference_map)
+        (visited, inlining_map)
     })
 }
 
@@ -343,7 +321,7 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>,
                                    starting_point: TransItem<'tcx>,
                                    visited: &mut FnvHashSet<TransItem<'tcx>>,
                                    recursion_depths: &mut DefIdMap<usize>,
-                                   reference_map: &mut ReferenceMap<'tcx>) {
+                                   inlining_map: &mut InliningMap<'tcx>) {
     if !visited.insert(starting_point.clone()) {
         // We've been here already, no need to search again.
         return;
@@ -390,10 +368,10 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>,
         }
     }
 
-    record_references(scx.tcx(), starting_point, &neighbors[..], reference_map);
+    record_inlining_canditates(scx.tcx(), starting_point, &neighbors[..], inlining_map);
 
     for neighbour in neighbors {
-        collect_items_rec(scx, neighbour, visited, recursion_depths, reference_map);
+        collect_items_rec(scx, neighbour, visited, recursion_depths, inlining_map);
     }
 
     if let Some((def_id, depth)) = recursion_depth_reset {
@@ -403,17 +381,19 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>,
     debug!("END collect_items_rec({})", starting_point.to_string(scx.tcx()));
 }
 
-fn record_references<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                               caller: TransItem<'tcx>,
-                               callees: &[TransItem<'tcx>],
-                               reference_map: &mut ReferenceMap<'tcx>) {
-    let iter = callees.into_iter()
-                      .map(|callee| {
-                        let is_inlining_candidate = callee.is_from_extern_crate() ||
-                                                    callee.requests_inline(tcx);
-                        (*callee, is_inlining_candidate)
-                      });
-    reference_map.record_references(caller, iter);
+fn record_inlining_canditates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                                        caller: TransItem<'tcx>,
+                                        callees: &[TransItem<'tcx>],
+                                        inlining_map: &mut InliningMap<'tcx>) {
+    let is_inlining_candidate = |trans_item: &TransItem<'tcx>| {
+        trans_item.is_from_extern_crate() || trans_item.requests_inline(tcx)
+    };
+
+    let inlining_candidates = callees.into_iter()
+                                     .map(|x| *x)
+                                     .filter(is_inlining_candidate);
+
+    inlining_map.record_inlining_canditates(caller, inlining_candidates);
 }
 
 fn check_recursion_limit<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

src/librustc_trans/partitioning.rs

@@ -116,7 +116,7 @@
 //! source-level module, functions from the same module will be available for
 //! inlining, even when they are not marked #[inline].
 
-use collector::ReferenceMap;
+use collector::InliningMap;
 use llvm;
 use monomorphize;
 use rustc::hir::def_id::DefId;
@@ -127,20 +127,9 @@ use syntax::parse::token::{self, InternedString};
 use trans_item::TransItem;
 use util::nodemap::{FnvHashMap, FnvHashSet};
 
-#[derive(Clone, Copy, Eq, PartialEq, Debug)]
-pub enum InstantiationMode {
-    /// This variant indicates that a translation item should be placed in some
-    /// codegen unit as a definition and with the given linkage.
-    Def(llvm::Linkage),
-
-    /// This variant indicates that only a declaration of some translation item
-    /// should be placed in a given codegen unit.
-    Decl
-}
-
 pub struct CodegenUnit<'tcx> {
     pub name: InternedString,
-    pub items: FnvHashMap<TransItem<'tcx>, InstantiationMode>,
+    pub items: FnvHashMap<TransItem<'tcx>, llvm::Linkage>,
 }
 
 pub enum PartitioningStrategy {
@@ -157,7 +146,7 @@ const FALLBACK_CODEGEN_UNIT: &'static str = "__rustc_fallback_codegen_unit";
 pub fn partition<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                               trans_items: I,
                               strategy: PartitioningStrategy,
-                              reference_map: &ReferenceMap<'tcx>)
+                              inlining_map: &InliningMap<'tcx>)
                               -> Vec<CodegenUnit<'tcx>>
     where I: Iterator<Item = TransItem<'tcx>>
 {
@@ -177,13 +166,8 @@ pub fn partition<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     // translation items can be drop-glue, functions from external crates, and
     // local functions the definition of which is marked with #[inline].
     let post_inlining = place_inlined_translation_items(initial_partitioning,
-                                                        reference_map);
-
-    // Now we know all *definitions* within all codegen units, thus we can
-    // easily determine which declarations need to be placed within each one.
-    let post_declarations = place_declarations(post_inlining, reference_map);
-
-    post_declarations.0
+                                                        inlining_map);
+    post_inlining.0
 }
 
 struct PreInliningPartitioning<'tcx> {
@@ -192,7 +176,6 @@ struct PreInliningPartitioning<'tcx> {
 }
 
 struct PostInliningPartitioning<'tcx>(Vec<CodegenUnit<'tcx>>);
-struct PostDeclarationsPartitioning<'tcx>(Vec<CodegenUnit<'tcx>>);
 
 fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                                              trans_items: I)
@@ -240,8 +223,7 @@ fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                 }
             };
 
-            codegen_unit.items.insert(trans_item,
-                                      InstantiationMode::Def(linkage));
+            codegen_unit.items.insert(trans_item, linkage);
             roots.insert(trans_item);
         }
     }
@@ -295,15 +277,15 @@ fn merge_codegen_units<'tcx>(initial_partitioning: &mut PreInliningPartitioning<
 }
 
 fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartitioning<'tcx>,
-                                         reference_map: &ReferenceMap<'tcx>)
+                                         inlining_map: &InliningMap<'tcx>)
                                          -> PostInliningPartitioning<'tcx> {
     let mut new_partitioning = Vec::new();
 
     for codegen_unit in &initial_partitioning.codegen_units[..] {
         // Collect all items that need to be available in this codegen unit
         let mut reachable = FnvHashSet();
         for root in codegen_unit.items.keys() {
-            follow_inlining(*root, reference_map, &mut reachable);
+            follow_inlining(*root, inlining_map, &mut reachable);
         }
 
         let mut new_codegen_unit = CodegenUnit {
@@ -313,22 +295,22 @@ fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartit
 
         // Add all translation items that are not already there
         for trans_item in reachable {
-            if let Some(instantiation_mode) = codegen_unit.items.get(&trans_item) {
+            if let Some(linkage) = codegen_unit.items.get(&trans_item) {
                 // This is a root, just copy it over
-                new_codegen_unit.items.insert(trans_item, *instantiation_mode);
+                new_codegen_unit.items.insert(trans_item, *linkage);
             } else {
                 if initial_partitioning.roots.contains(&trans_item) {
                     // This item will be instantiated in some other codegen unit,
                     // so we just add it here with AvailableExternallyLinkage
                     new_codegen_unit.items.insert(trans_item,
-                        InstantiationMode::Def(llvm::AvailableExternallyLinkage));
+                                                  llvm::AvailableExternallyLinkage);
                 } else {
                     // We can't be sure if this will also be instantiated
                     // somewhere else, so we add an instance here with
                     // LinkOnceODRLinkage. That way the item can be discarded if
                     // it's not needed (inlined) after all.
                     new_codegen_unit.items.insert(trans_item,
-                        InstantiationMode::Def(llvm::LinkOnceODRLinkage));
+                                                  llvm::LinkOnceODRLinkage);
                 }
             }
         }
@@ -339,43 +321,18 @@ fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartit
     return PostInliningPartitioning(new_partitioning);
 
     fn follow_inlining<'tcx>(trans_item: TransItem<'tcx>,
-                             reference_map: &ReferenceMap<'tcx>,
+                             inlining_map: &InliningMap<'tcx>,
                              visited: &mut FnvHashSet<TransItem<'tcx>>) {
         if !visited.insert(trans_item) {
             return;
         }
 
-        reference_map.with_inlining_candidates(trans_item, |target| {
-            follow_inlining(target, reference_map, visited);
+        inlining_map.with_inlining_candidates(trans_item, |target| {
+            follow_inlining(target, inlining_map, visited);
         });
     }
 }
 
-fn place_declarations<'tcx>(codegen_units: PostInliningPartitioning<'tcx>,
-                            reference_map: &ReferenceMap<'tcx>)
-                            -> PostDeclarationsPartitioning<'tcx> {
-    let PostInliningPartitioning(mut codegen_units) = codegen_units;
-
-    for codegen_unit in codegen_units.iter_mut() {
-        let mut declarations = FnvHashSet();
-
-        for (trans_item, _) in &codegen_unit.items {
-            for referenced_item in reference_map.get_direct_references_from(*trans_item) {
-                if !codegen_unit.items.contains_key(referenced_item) {
-                    declarations.insert(*referenced_item);
-                }
-            }
-        }
-
-        codegen_unit.items
-                    .extend(declarations.iter()
-                                        .map(|trans_item| (*trans_item,
-                                                           InstantiationMode::Decl)));
-    }
-
-    PostDeclarationsPartitioning(codegen_units)
-}
-
 fn characteristic_def_id_of_trans_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                                                  trans_item: TransItem<'tcx>)
                                                  -> Option<DefId> {