Preserve DebugInfo in DeadStoreElimination.

Author: cjgillot

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/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;
+      }
+  }
+  

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

@@ -1,21 +1,40 @@
-// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
+// This example is interesting because the non-transitive version of `MaybeLiveLocals` would
+// report that *all* of these stores are live.
+//
+// needs-unwind
 // unit-test: DeadStoreElimination
 
+#![feature(core_intrinsics, custom_mir)]
+use std::intrinsics::mir::*;
+
 #[inline(never)]
 fn cond() -> bool {
     false
 }
 
 // EMIT_MIR cycle.cycle.DeadStoreElimination.diff
+#[custom_mir(dialect = "runtime", phase = "post-cleanup")]
 fn cycle(mut x: i32, mut y: i32, mut z: i32) {
-    // This example is interesting because the non-transitive version of `MaybeLiveLocals` would
-    // report that *all* of these stores are live.
-    while cond() {
-        let temp = z;
-        z = y;
-        y = x;
-        x = temp;
-    }
+    // We use custom MIR to avoid generating debuginfo, that would force to preserve writes.
+    mir!(
+        let condition: bool;
+        {
+            Call(condition = cond(), bb1)
+        }
+        bb1 = {
+            match condition { true => bb2, _ => ret }
+        }
+        bb2 = {
+            let temp = z;
+            z = y;
+            y = x;
+            x = temp;
+            Call(condition = cond(), bb1)
+        }
+        ret = {
+            Return()
+        }
+    )
 }
 
 fn main() {

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

@@ -7,15 +7,16 @@ fn f(_1: usize) -> usize {
     let mut _3: usize;
     let mut _4: usize;
     scope 1 {
-        debug b => _1;
+        debug b => _3;
     }
 
     bb0: {
         nop;
+        _3 = _1;
+        _1 = const 5_usize;
         nop;
         nop;
-        nop;
-        nop;
+        _1 = move _3;
         nop;
         nop;
         nop;

tests/mir-opt/dead-store-elimination/cycle.rs

@@ -7,15 +7,16 @@ fn f(_1: usize) -> usize {
     let mut _3: usize;
     let mut _4: usize;
     scope 1 {
-        debug b => _1;
+        debug b => _3;
     }
 
     bb0: {
         nop;
+        _3 = _1;
+        _1 = const 5_usize;
         nop;
         nop;
-        nop;
-        nop;
+        _1 = move _3;
         nop;
         nop;
         nop;

tests/mir-opt/dest-prop/dead_stores_better.f.DestinationPropagation.after.panic-abort.mir

@@ -22,8 +22,10 @@
       }
 
       bb1: {
+          _1 = Un { us: move _2 };
           StorageDead(_2);
           StorageLive(_3);
+          _3 = (_1.0: u32);
           StorageDead(_3);
           StorageDead(_1);
           return;

tests/mir-opt/dest-prop/dead_stores_better.f.DestinationPropagation.after.panic-unwind.mir

@@ -22,8 +22,10 @@
       }
 
       bb1: {
+          _1 = Un { us: move _2 };
           StorageDead(_2);
           StorageLive(_3);
+          _3 = (_1.0: u32);
           StorageDead(_3);
           StorageDead(_1);
           return;