Auto merge of rust-lang#155491 - ohadravid:faster-storage-in-copyprop-and-gvn, r=saethlin

Fix performance regression introduced in rust-lang#142531 by excluding `Storage{Live,Dead}` from CGU size estimation



Fix performance regression introduced in rust-lang#142531 ([rust-timer comment](rust-lang#142531 (comment))) by excluding `Storage{Live,Dead}` from CGU size estimation.

Also, avoid unneeded work for storage removal in non-opt builds in CopyProp and GVN
by allocating local sets for the storage accounting only when `tcx.sess.emit_lifetime_markers()`. 

r? saethlin

Author: bors

compiler/rustc_mir_transform/src/copy_prop.rs

@@ -38,12 +38,10 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
         let mut any_replacement = false;
         // Locals that participate in copy propagation either as a source or a destination.
         let mut unified = DenseBitSet::new_empty(body.local_decls.len());
-        let mut storage_to_remove = DenseBitSet::new_empty(body.local_decls.len());
 
         for (local, &head) in ssa.copy_classes().iter_enumerated() {
             if local != head {
                 any_replacement = true;
-                storage_to_remove.insert(head);
                 unified.insert(head);
                 unified.insert(local);
             }
@@ -58,7 +56,7 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
         // only if the head might be uninitialized at that point, or if the local is borrowed
         // (since we cannot easily determine when it's used).
         let storage_to_remove = if tcx.sess.emit_lifetime_markers() {
-            storage_to_remove.clear();
+            let mut storage_to_remove = DenseBitSet::new_empty(body.local_decls.len());
 
             // If the local is borrowed, we cannot easily determine if it is used, so we have to remove the storage statements.
             let borrowed_locals = ssa.borrowed_locals();
@@ -83,15 +81,16 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
                 storage_checker.visit_basic_block_data(bb, data);
             }
 
-            storage_checker.storage_to_remove
+            Some(storage_checker.storage_to_remove)
         } else {
-            // Remove the storage statements of all the head locals.
-            storage_to_remove
+            None
         };
 
+        // If None, remove the storage statements of all the unified locals.
+        let storage_to_remove = storage_to_remove.as_ref().unwrap_or(&unified);
         debug!(?storage_to_remove);
 
-        Replacer { tcx, copy_classes: ssa.copy_classes(), unified, storage_to_remove }
+        Replacer { tcx, copy_classes: ssa.copy_classes(), unified: &unified, storage_to_remove }
             .visit_body_preserves_cfg(body);
 
         crate::simplify::remove_unused_definitions(body);
@@ -106,8 +105,8 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
 /// all occurrences of the key get replaced by the value.
 struct Replacer<'a, 'tcx> {
     tcx: TyCtxt<'tcx>,
-    unified: DenseBitSet<Local>,
-    storage_to_remove: DenseBitSet<Local>,
+    unified: &'a DenseBitSet<Local>,
+    storage_to_remove: &'a DenseBitSet<Local>,
     copy_classes: &'a IndexSlice<Local, Local>,
 }
 

compiler/rustc_mir_transform/src/gvn.rs

@@ -173,15 +173,16 @@ impl<'tcx> crate::MirPass<'tcx> for GVN {
                 storage_checker.visit_basic_block_data(bb, data);
             }
 
-            storage_checker.storage_to_remove
+            Some(storage_checker.storage_to_remove)
         } else {
-            // Remove the storage statements of all the reused locals.
-            state.reused_locals.clone()
+            None
         };
 
+        // If None, remove the storage statements of all the reused locals.
+        let storage_to_remove = storage_to_remove.as_ref().unwrap_or(&state.reused_locals);
         debug!(?storage_to_remove);
 
-        StorageRemover { tcx, reused_locals: state.reused_locals, storage_to_remove }
+        StorageRemover { tcx, reused_locals: &state.reused_locals, storage_to_remove }
             .visit_body_preserves_cfg(body);
     }
 
@@ -2058,13 +2059,13 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, '_, 'tcx> {
     }
 }
 
-struct StorageRemover<'tcx> {
+struct StorageRemover<'a, 'tcx> {
     tcx: TyCtxt<'tcx>,
-    reused_locals: DenseBitSet<Local>,
-    storage_to_remove: DenseBitSet<Local>,
+    reused_locals: &'a DenseBitSet<Local>,
+    storage_to_remove: &'a DenseBitSet<Local>,
 }
 
-impl<'tcx> MutVisitor<'tcx> for StorageRemover<'tcx> {
+impl<'a, 'tcx> MutVisitor<'tcx> for StorageRemover<'a, 'tcx> {
     fn tcx(&self) -> TyCtxt<'tcx> {
         self.tcx
     }

compiler/rustc_monomorphize/src/partitioning.rs

@@ -109,6 +109,7 @@ use rustc_hir::definitions::DefPathDataName;
 use rustc_middle::bug;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::middle::exported_symbols::{SymbolExportInfo, SymbolExportLevel};
+use rustc_middle::mir::StatementKind;
 use rustc_middle::mono::{
     CodegenUnit, CodegenUnitNameBuilder, InstantiationMode, MonoItem, MonoItemData,
     MonoItemPartitions, Visibility,
@@ -1334,7 +1335,21 @@ pub(crate) fn provide(providers: &mut Providers) {
             | InstanceKind::DropGlue(..)
             | InstanceKind::AsyncDropGlueCtorShim(..) => {
                 let mir = tcx.instance_mir(instance.def);
-                mir.basic_blocks.iter().map(|bb| bb.statements.len() + 1).sum()
+                mir.basic_blocks
+                    .iter()
+                    .map(|bb| {
+                        bb.statements
+                            .iter()
+                            .filter_map(|stmt| match stmt.kind {
+                                StatementKind::StorageLive(_) | StatementKind::StorageDead(_) => {
+                                    None
+                                }
+                                _ => Some(stmt),
+                            })
+                            .count()
+                            + 1
+                    })
+                    .sum()
             }
             // Other compiler-generated shims size estimate: 1
             _ => 1,