Preserve DebugInfo in DeadStoreElimination.

rust-lang · Sep 26, 2023 · f561c35 · f561c35
1 parent 551c718
commit f561c35
Show file tree

Hide file tree

Showing 29 changed files with 747 additions and 699 deletions.
diff --git a/compiler/rustc_index/src/bit_set.rs b/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();

diff --git a/compiler/rustc_mir_dataflow/src/debuginfo.rs b/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);
+    }
+}
diff --git a/compiler/rustc_mir_dataflow/src/lib.rs b/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;

diff --git a/compiler/rustc_mir_transform/src/dead_store_elimination.rs b/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);
     }
 }
diff --git a/tests/mir-opt/dead-store-elimination/cycle.cycle.DeadStoreElimination.panic-abort.diff b/tests/mir-opt/dead-store-elimination/cycle.cycle.DeadStoreElimination.panic-abort.diff
@@ -9,14 +9,15 @@
 -     let mut _4: ();
 -     let mut _5: bool;
 -     let _6: i32;
--     let mut _7: i32;
--     let mut _8: i32;
++     let mut _4: bool;
++     let _5: i32;
++     let mut _6: i32;
+      let mut _7: i32;
+      let mut _8: i32;
 -     let mut _9: i32;
 -     let mut _10: !;
 -     let _11: ();
 -     let mut _12: !;
-+     let mut _4: bool;
-+     let _5: i32;
       scope 1 {
 -         debug temp => _6;
 +         debug temp => _5;
@@ -39,23 +40,31 @@
       }
 
       bb3: {
--         StorageLive(_6);
++         StorageLive(_5);
++         _5 = _3;
+          StorageLive(_6);
 -         _6 = _3;
--         StorageLive(_7);
++         _6 = _2;
++         _3 = move _6;
++         StorageDead(_6);
+          StorageLive(_7);
 -         _7 = _2;
 -         _3 = move _7;
--         StorageDead(_7);
--         StorageLive(_8);
++         _7 = _1;
++         _2 = move _7;
+          StorageDead(_7);
+          StorageLive(_8);
 -         _8 = _1;
 -         _2 = move _8;
--         StorageDead(_8);
++         _8 = _5;
++         _1 = move _8;
+          StorageDead(_8);
 -         StorageLive(_9);
 -         _9 = _6;
 -         _1 = move _9;
 -         StorageDead(_9);
 -         _4 = const ();
 -         StorageDead(_6);
-+         StorageLive(_5);
           StorageDead(_5);
 +         StorageDead(_4);
           goto -> bb1;

diff --git a/tests/mir-opt/dead-store-elimination/cycle.cycle.DeadStoreElimination.panic-unwind.diff b/tests/mir-opt/dead-store-elimination/cycle.cycle.DeadStoreElimination.panic-unwind.diff
@@ -2,71 +2,27 @@
 + // MIR for `cycle` after DeadStoreElimination
 
   fn cycle(_1: i32, _2: i32, _3: i32) -> () {
-      debug x => _1;
-      debug y => _2;
-      debug z => _3;
       let mut _0: ();
--     let mut _4: ();
--     let mut _5: bool;
--     let _6: i32;
--     let mut _7: i32;
--     let mut _8: i32;
--     let mut _9: i32;
--     let mut _10: !;
--     let _11: ();
--     let mut _12: !;
-+     let mut _4: bool;
-+     let _5: i32;
-      scope 1 {
--         debug temp => _6;
-+         debug temp => _5;
-      }
+      let mut _4: bool;
+-     let mut _5: i32;
 
       bb0: {
-          goto -> bb1;
+          _4 = cond() -> [return: bb1, unwind continue];
       }
 
       bb1: {
--         StorageLive(_5);
--         _5 = cond() -> [return: bb2, unwind continue];
-+         StorageLive(_4);
-+         _4 = cond() -> [return: bb2, unwind continue];
+          switchInt(_4) -> [1: bb2, otherwise: bb3];
       }
 
       bb2: {
--         switchInt(move _5) -> [0: bb4, otherwise: bb3];
-+         switchInt(move _4) -> [0: bb4, otherwise: bb3];
+-         _5 = _3;
+-         _3 = _2;
+-         _2 = _1;
+-         _1 = _5;
+          _4 = cond() -> [return: bb1, unwind continue];
       }
 
       bb3: {
--         StorageLive(_6);
--         _6 = _3;
--         StorageLive(_7);
--         _7 = _2;
--         _3 = move _7;
--         StorageDead(_7);
--         StorageLive(_8);
--         _8 = _1;
--         _2 = move _8;
--         StorageDead(_8);
--         StorageLive(_9);
--         _9 = _6;
--         _1 = move _9;
--         StorageDead(_9);
--         _4 = const ();
--         StorageDead(_6);
-+         StorageLive(_5);
-          StorageDead(_5);
-+         StorageDead(_4);
-          goto -> bb1;
-      }
-
-      bb4: {
--         StorageLive(_11);
-          _0 = const ();
--         StorageDead(_11);
--         StorageDead(_5);
-+         StorageDead(_4);
           return;
       }
   }

diff --git a/tests/mir-opt/dead-store-elimination/cycle.rs b/tests/mir-opt/dead-store-elimination/cycle.rs
@@ -1,21 +1,40 @@
+// This example is interesting because the non-transitive version of `MaybeLiveLocals` would
+// report that *all* of these stores are live.
+//
 // EMIT_MIR_FOR_EACH_PANIC_STRATEGY
 // 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() {

diff --git a/tests/mir-opt/dest-prop/dead_stores_better.f.DestinationPropagation.after.panic-abort.mir b/tests/mir-opt/dest-prop/dead_stores_better.f.DestinationPropagation.after.panic-abort.mir
@@ -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;

diff --git a/tests/mir-opt/dest-prop/dead_stores_better.f.DestinationPropagation.after.panic-unwind.mir b/tests/mir-opt/dest-prop/dead_stores_better.f.DestinationPropagation.after.panic-unwind.mir
@@ -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;

diff --git a/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-abort.diff b/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-abort.diff
@@ -22,8 +22,10 @@
       }
 
       bb1: {
+          _1 = Un { us: move _2 };
           StorageDead(_2);
           StorageLive(_3);
+          _3 = (_1.0: u32);
           StorageDead(_3);
           StorageDead(_1);
           return;

diff --git a/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-unwind.diff b/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-unwind.diff
@@ -22,8 +22,10 @@
       }
 
       bb1: {
+          _1 = Un { us: move _2 };
           StorageDead(_2);
           StorageLive(_3);
+          _3 = (_1.0: u32);
           StorageDead(_3);
           StorageDead(_1);
           return;

diff --git a/tests/mir-opt/issue_101973.inner.ConstProp.panic-abort.diff b/tests/mir-opt/issue_101973.inner.ConstProp.panic-abort.diff
@@ -33,6 +33,7 @@
           StorageLive(_2);
           StorageLive(_3);
           StorageLive(_4);
+          _4 = const 0_u32;
           StorageLive(_15);
           StorageLive(_14);
           _14 = Shr(_1, const 0_i32);

diff --git a/tests/mir-opt/issue_101973.inner.ConstProp.panic-unwind.diff b/tests/mir-opt/issue_101973.inner.ConstProp.panic-unwind.diff
@@ -33,6 +33,7 @@
           StorageLive(_2);
           StorageLive(_3);
           StorageLive(_4);
+          _4 = const 0_u32;
           StorageLive(_15);
           StorageLive(_14);
           _14 = Shr(_1, const 0_i32);