Auto merge of #116183 - cjgillot:debug-dse-always, r=oli-obk

Always preserve DebugInfo in DeadStoreElimination.

This is a version of #106852 that does not check the current crate's debuginfo flag, and always attempts to preserve debuginfo.

I haven't figured out how to handle mixing debuginfo levels for std, the one for the test, and the one for the CI target just right to merge #106852, so this can at least fix the debuginfo issue.

Fixes #103655

Author: bors

compiler/rustc_index/src/bit_set.rs

@@ -365,7 +365,7 @@ impl<T: Idx> From<GrowableBitSet<T>> for BitSet<T> {
 /// All operations that involve an element will panic if the element is equal
 /// to or greater than the domain size. All operations that involve two bitsets
 /// will panic if the bitsets have differing domain sizes.
-#[derive(Debug, PartialEq, Eq)]
+#[derive(PartialEq, Eq)]
 pub struct ChunkedBitSet<T> {
     domain_size: usize,
 
@@ -1074,6 +1074,12 @@ impl<T: Idx> fmt::Debug for BitSet<T> {
     }
 }
 
+impl<T: Idx> fmt::Debug for ChunkedBitSet<T> {
+    fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result {
+        w.debug_list().entries(self.iter()).finish()
+    }
+}
+
 impl<T: Idx> ToString for BitSet<T> {
     fn to_string(&self) -> String {
         let mut result = String::new();

compiler/rustc_mir_dataflow/src/debuginfo.rs

@@ -0,0 +1,20 @@
+use rustc_index::bit_set::BitSet;
+use rustc_middle::mir::visit::*;
+use rustc_middle::mir::*;
+
+/// Return the set of locals that appear in debuginfo.
+pub fn debuginfo_locals(body: &Body<'_>) -> BitSet<Local> {
+    let mut visitor = DebuginfoLocals(BitSet::new_empty(body.local_decls.len()));
+    for debuginfo in body.var_debug_info.iter() {
+        visitor.visit_var_debug_info(debuginfo);
+    }
+    visitor.0
+}
+
+struct DebuginfoLocals(BitSet<Local>);
+
+impl Visitor<'_> for DebuginfoLocals {
+    fn visit_local(&mut self, local: Local, _: PlaceContext, _: Location) {
+        self.0.insert(local);
+    }
+}

compiler/rustc_mir_dataflow/src/lib.rs

@@ -35,6 +35,7 @@ pub use self::framework::{
 
 use self::move_paths::MoveData;
 
+pub mod debuginfo;
 pub mod drop_flag_effects;
 pub mod elaborate_drops;
 mod errors;

compiler/rustc_mir_transform/src/dead_store_elimination.rs

@@ -13,10 +13,10 @@
 //!
 
 use crate::util::is_within_packed;
-use rustc_index::bit_set::BitSet;
 use rustc_middle::mir::visit::Visitor;
 use rustc_middle::mir::*;
 use rustc_middle::ty::TyCtxt;
+use rustc_mir_dataflow::debuginfo::debuginfo_locals;
 use rustc_mir_dataflow::impls::{
     borrowed_locals, LivenessTransferFunction, MaybeTransitiveLiveLocals,
 };
@@ -26,8 +26,15 @@ use rustc_mir_dataflow::Analysis;
 ///
 /// The `borrowed` set must be a `BitSet` of all the locals that are ever borrowed in this body. It
 /// can be generated via the [`borrowed_locals`] function.
-pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitSet<Local>) {
-    let mut live = MaybeTransitiveLiveLocals::new(borrowed)
+pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
+    let borrowed_locals = borrowed_locals(body);
+
+    // If the user requests complete debuginfo, mark the locals that appear in it as live, so
+    // we don't remove assignements to them.
+    let mut always_live = debuginfo_locals(body);
+    always_live.union(&borrowed_locals);
+
+    let mut live = MaybeTransitiveLiveLocals::new(&always_live)
         .into_engine(tcx, body)
         .iterate_to_fixpoint()
         .into_results_cursor(body);
@@ -48,7 +55,9 @@ pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitS
             for (index, arg) in args.iter().enumerate().rev() {
                 if let Operand::Copy(place) = *arg
                     && !place.is_indirect()
-                    && !borrowed.contains(place.local)
+                    // Do not skip the transformation if the local is in debuginfo, as we do
+                    // not really lose any information for this purpose.
+                    && !borrowed_locals.contains(place.local)
                     && !state.contains(place.local)
                     // If `place` is a projection of a disaligned field in a packed ADT,
                     // the move may be codegened as a pointer to that field.
@@ -75,7 +84,7 @@ pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitS
                 StatementKind::Assign(box (place, _))
                 | StatementKind::SetDiscriminant { place: box place, .. }
                 | StatementKind::Deinit(box place) => {
-                    if !place.is_indirect() && !borrowed.contains(place.local) {
+                    if !place.is_indirect() && !always_live.contains(place.local) {
                         live.seek_before_primary_effect(loc);
                         if !live.get().contains(place.local) {
                             patch.push(loc);
@@ -126,7 +135,6 @@ impl<'tcx> MirPass<'tcx> for DeadStoreElimination {
     }
 
     fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
-        let borrowed = borrowed_locals(body);
-        eliminate(tcx, body, &borrowed);
+        eliminate(tcx, body);
     }
 }

tests/incremental/hashes/for_loops.rs

@@ -28,9 +28,9 @@ pub fn change_loop_body() {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(cfg="cfail2", except="hir_owner_nodes")]
+#[rustc_clean(cfg="cfail2", except="hir_owner_nodes, optimized_mir")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, optimized_mir")]
 #[rustc_clean(cfg="cfail6")]
 pub fn change_loop_body() {
     let mut _x = 0;
@@ -180,7 +180,7 @@ pub fn add_loop_label_to_break() {
 #[cfg(not(any(cfail1,cfail4)))]
 #[rustc_clean(cfg="cfail2", except="hir_owner_nodes")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, optimized_mir")]
 #[rustc_clean(cfg="cfail6")]
 pub fn add_loop_label_to_break() {
     let mut _x = 0;

tests/incremental/hashes/loop_expressions.rs

@@ -28,9 +28,9 @@ pub fn change_loop_body() {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(cfg="cfail2", except="hir_owner_nodes")]
+#[rustc_clean(cfg="cfail2", except="hir_owner_nodes, optimized_mir")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, optimized_mir")]
 #[rustc_clean(cfg="cfail6")]
 pub fn change_loop_body() {
     let mut _x = 0;

tests/incremental/hashes/while_loops.rs

@@ -28,9 +28,9 @@ pub fn change_loop_body() {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(cfg="cfail2", except="hir_owner_nodes")]
+#[rustc_clean(cfg="cfail2", except="hir_owner_nodes, optimized_mir")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, optimized_mir")]
 #[rustc_clean(cfg="cfail6")]
 pub fn change_loop_body() {
     let mut _x = 0;
@@ -53,9 +53,9 @@ pub fn change_loop_condition() {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(cfg="cfail2", except="hir_owner_nodes")]
+#[rustc_clean(cfg="cfail2", except="hir_owner_nodes, optimized_mir")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, optimized_mir")]
 #[rustc_clean(cfg="cfail6")]
 pub fn change_loop_condition() {
     let mut _x = 0;
@@ -211,7 +211,7 @@ pub fn change_continue_label() {
 #[cfg(not(any(cfail1,cfail4)))]
 #[rustc_clean(cfg="cfail2", except="hir_owner_nodes")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes,optimized_mir")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes")]
 #[rustc_clean(cfg="cfail6")]
 pub fn change_continue_label() {
     let mut _x = 0;

tests/mir-opt/dead-store-elimination/cycle.cycle.DeadStoreElimination.diff

@@ -0,0 +1,29 @@
+- // MIR for `cycle` before DeadStoreElimination
++ // MIR for `cycle` after DeadStoreElimination
+  
+  fn cycle(_1: i32, _2: i32, _3: i32) -> () {
+      let mut _0: ();
+      let mut _4: bool;
+-     let mut _5: i32;
+  
+      bb0: {
+          _4 = cond() -> [return: bb1, unwind continue];
+      }
+  
+      bb1: {
+          switchInt(_4) -> [1: bb2, otherwise: bb3];
+      }
+  
+      bb2: {
+-         _5 = _3;
+-         _3 = _2;
+-         _2 = _1;
+-         _1 = _5;
+          _4 = cond() -> [return: bb1, unwind continue];
+      }
+  
+      bb3: {
+          return;
+      }
+  }
+