mir-opt: Eliminate statements need debuginfo when mir-opt-level >= 2

`-Copt-level=0` (`-Zmir-opt-level=1`) is preferred for debugging.

Author: dianqk

compiler/rustc_middle/src/mir/mod.rs

@@ -1433,6 +1433,13 @@ impl<'tcx> BasicBlockData<'tcx> {
     pub fn strip_nops(&mut self) {
         self.retain_statements(|stmt| !matches!(stmt.kind, StatementKind::Nop))
     }
+
+    pub fn drop_debuginfo(&mut self) {
+        self.after_last_stmt_debuginfos = Vec::new();
+        for stmt in self.statements.iter_mut() {
+            stmt.debuginfos = Vec::new();
+        }
+    }
 }
 
 ///////////////////////////////////////////////////////////////////////////

compiler/rustc_mir_transform/src/copy_prop.rs

@@ -52,7 +52,7 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
         Replacer { tcx, copy_classes: ssa.copy_classes(), fully_moved, storage_to_remove }
             .visit_body_preserves_cfg(body);
 
-        crate::simplify::remove_unused_definitions(body);
+        crate::simplify::remove_unused_definitions(body, tcx.sess.mir_opt_level());
     }
 
     fn is_required(&self) -> bool {

compiler/rustc_mir_transform/src/dead_store_elimination.rs

@@ -22,13 +22,14 @@ use rustc_mir_dataflow::impls::{
     LivenessTransferFunction, MaybeTransitiveLiveLocals, borrowed_locals,
 };
 
+use crate::simplify::remove_unused_definitions;
 use crate::util::is_within_packed;
 
 /// Performs the optimization on the body
 ///
 /// The `borrowed` set must be a `DenseBitSet` of all the locals that are ever borrowed in this
 /// body. It can be generated via the [`borrowed_locals`] function.
-fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
+fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
     let borrowed_locals = borrowed_locals(body);
 
     // If the user requests complete debuginfo, mark the locals that appear in it as live, so
@@ -89,8 +90,9 @@ fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
     }
 
     if patch.is_empty() && call_operands_to_move.is_empty() {
-        return;
+        return false;
     }
+    let eliminated = !patch.is_empty();
 
     let bbs = body.basic_blocks.as_mut_preserves_cfg();
     for (Location { block, statement_index }, drop_debuginfo) in patch {
@@ -104,6 +106,8 @@ fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
         let Operand::Copy(place) = *arg else { bug!() };
         *arg = Operand::Move(place);
     }
+
+    eliminated
 }
 
 pub(super) enum DeadStoreElimination {
@@ -124,7 +128,10 @@ impl<'tcx> crate::MirPass<'tcx> for DeadStoreElimination {
     }
 
     fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
-        eliminate(tcx, body);
+        if eliminate(tcx, body) {
+            // Remove unnecessary StorageLive and StorageDead annotations.
+            remove_unused_definitions(body, tcx.sess.mir_opt_level());
+        }
     }
 
     fn is_required(&self) -> bool {

compiler/rustc_mir_transform/src/inline.rs

@@ -995,6 +995,10 @@ fn inline_call<'tcx, I: Inliner<'tcx>>(
         // people working on rust can build with or without debuginfo while
         // still getting consistent results from the mir-opt tests.
         caller_body.var_debug_info.append(&mut callee_body.var_debug_info);
+    } else {
+        for bb in callee_body.basic_blocks_mut() {
+            bb.drop_debuginfo();
+        }
     }
     caller_body.basic_blocks_mut().append(callee_body.basic_blocks_mut());
 

compiler/rustc_mir_transform/src/ref_prop.rs

@@ -99,7 +99,7 @@ fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
     replacer.visit_body_preserves_cfg(body);
 
     if replacer.any_replacement {
-        crate::simplify::remove_unused_definitions(body);
+        crate::simplify::remove_unused_definitions(body, tcx.sess.mir_opt_level());
     }
 
     replacer.any_replacement

compiler/rustc_mir_transform/src/simplify.rs

@@ -423,7 +423,7 @@ impl<'tcx> crate::MirPass<'tcx> for SimplifyLocals {
         // count. For example, if we removed `_2 = discriminant(_1)`, then we'll subtract one from
         // `use_counts[_1]`. That in turn might make `_1` unused, so we loop until we hit a
         // fixedpoint where there are no more unused locals.
-        remove_unused_definitions_helper(&mut used_locals, body);
+        remove_unused_definitions_helper(&mut used_locals, body, tcx.sess.mir_opt_level());
 
         // Finally, we'll actually do the work of shrinking `body.local_decls` and remapping the
         // `Local`s.
@@ -444,7 +444,7 @@ impl<'tcx> crate::MirPass<'tcx> for SimplifyLocals {
     }
 }
 
-pub(super) fn remove_unused_definitions<'tcx>(body: &mut Body<'tcx>) {
+pub(super) fn remove_unused_definitions<'tcx>(body: &mut Body<'tcx>, mir_opt_level: usize) {
     // First, we're going to get a count of *actual* uses for every `Local`.
     let mut used_locals = UsedLocals::new(body);
 
@@ -453,7 +453,7 @@ pub(super) fn remove_unused_definitions<'tcx>(body: &mut Body<'tcx>) {
     // count. For example, if we removed `_2 = discriminant(_1)`, then we'll subtract one from
     // `use_counts[_1]`. That in turn might make `_1` unused, so we loop until we hit a
     // fixedpoint where there are no more unused locals.
-    remove_unused_definitions_helper(&mut used_locals, body);
+    remove_unused_definitions_helper(&mut used_locals, body, mir_opt_level);
 }
 
 /// Construct the mapping while swapping out unused stuff out from the `vec`.
@@ -466,7 +466,7 @@ fn make_local_map<V>(
 
     for alive_index in local_decls.indices() {
         // `is_used` treats the `RETURN_PLACE` and arguments as used.
-        if !used_locals.is_used(alive_index) {
+        if !used_locals.is_used(alive_index, true) {
             continue;
         }
 
@@ -504,16 +504,14 @@ impl UsedLocals {
     /// Checks if local is used.
     ///
     /// Return place and arguments are always considered used.
-    fn is_used(&self, local: Local) -> bool {
+    fn is_used(&self, local: Local, debuginfo_is_used: bool) -> bool {
         trace!(
             "is_used({:?}): use_count: {:?}, debuginfo_use: {}",
             local, self.use_count[local], self.debuginfo_use[local]
         );
-        local.as_u32() <= self.arg_count || self.use_count[local] != 0 || self.debuginfo_use[local]
-    }
-
-    fn is_only_debuginfo_used(&self, local: Local) -> bool {
-        local.as_u32() > self.arg_count && self.use_count[local] == 0 && self.debuginfo_use[local]
+        local.as_u32() <= self.arg_count
+            || self.use_count[local] != 0
+            || (debuginfo_is_used && self.debuginfo_use[local])
     }
 
     fn is_debuginfo_used(&self, local: Local) -> bool {
@@ -605,36 +603,46 @@ impl<'tcx> Visitor<'tcx> for UsedLocals {
 }
 
 /// Removes unused definitions. Updates the used locals to reflect the changes made.
-fn remove_unused_definitions_helper(used_locals: &mut UsedLocals, body: &mut Body<'_>) {
+fn remove_unused_definitions_helper(
+    used_locals: &mut UsedLocals,
+    body: &mut Body<'_>,
+    mir_opt_level: usize,
+) {
     // The use counts are updated as we remove the statements. A local might become unused
     // during the retain operation, leading to a temporary inconsistency (storage statements or
     // definitions referencing the local might remain). For correctness it is crucial that this
     // computation reaches a fixed point.
 
     let mut modified = true;
+    // Transform statements to debug information only when user requests more aggressive optimization.
+    // This may degrade the debugging experience.
+    let debuginfo_is_used = mir_opt_level < 2;
     while modified {
         modified = false;
 
         for data in body.basic_blocks.as_mut_preserves_cfg() {
             // Remove unnecessary StorageLive and StorageDead annotations.
             for statement in data.statements.iter_mut() {
                 let (keep_statement, drop_debuginfo) = match &statement.kind {
-                    StatementKind::StorageLive(local) | StatementKind::StorageDead(local) => (
-                        !used_locals.is_used(*local) || used_locals.is_only_debuginfo_used(*local),
-                        true,
-                    ),
+                    StatementKind::StorageLive(local) | StatementKind::StorageDead(local) => {
+                        (used_locals.is_used(*local, debuginfo_is_used), true)
+                    }
                     StatementKind::Assign(box (place, _))
                     | StatementKind::SetDiscriminant { box place, .. }
                     | StatementKind::BackwardIncompatibleDropHint { box place, .. }
                     | StatementKind::Deinit(box place) => (
-                        !used_locals.is_used(place.local)
-                            || (used_locals.is_only_debuginfo_used(place.local)
-                                && statement.kind.as_debuginfo().is_some()),
+                        used_locals.is_used(
+                            place.local,
+                            debuginfo_is_used
+                            // FIXME: We are not sure how we should represent this debugging information for some statements,
+                            // keep it for now.
+                            || statement.kind.as_debuginfo().is_none(),
+                        ),
                         !used_locals.is_debuginfo_used(place.local),
                     ),
                     _ => continue,
                 };
-                if !keep_statement {
+                if keep_statement {
                     continue;
                 }
                 trace!("removing statement {:?}", statement);

tests/mir-opt/copy-prop/reborrow.demiraw.CopyProp.panic-abort.diff

@@ -24,15 +24,13 @@
       bb0: {
 -         StorageLive(_2);
           _2 = &raw mut _1;
--         StorageLive(_3);
--         StorageLive(_4);
--         _4 = &mut (*_2);
--         _3 = &mut (*_4);
--         StorageDead(_4);
+          StorageLive(_3);
+          StorageLive(_4);
+          _4 = &mut (*_2);
+          _3 = &mut (*_4);
+          StorageDead(_4);
 -         StorageLive(_5);
 -         _5 = copy _2;
-+         // DBG: _4 = &(*_2)
-+         // DBG: _3 = &(*_4)
           StorageLive(_6);
 -         StorageLive(_7);
 -         _7 = copy _5;
@@ -45,7 +43,7 @@
           StorageDead(_6);
           _0 = const ();
 -         StorageDead(_5);
--         StorageDead(_3);
+          StorageDead(_3);
 -         StorageDead(_2);
           return;
       }

tests/mir-opt/copy-prop/reborrow.demiraw.CopyProp.panic-unwind.diff

@@ -24,15 +24,13 @@
       bb0: {
 -         StorageLive(_2);
           _2 = &raw mut _1;
--         StorageLive(_3);
--         StorageLive(_4);
--         _4 = &mut (*_2);
--         _3 = &mut (*_4);
--         StorageDead(_4);
+          StorageLive(_3);
+          StorageLive(_4);
+          _4 = &mut (*_2);
+          _3 = &mut (*_4);
+          StorageDead(_4);
 -         StorageLive(_5);
 -         _5 = copy _2;
-+         // DBG: _4 = &(*_2)
-+         // DBG: _3 = &(*_4)
           StorageLive(_6);
 -         StorageLive(_7);
 -         _7 = copy _5;
@@ -45,7 +43,7 @@
           StorageDead(_6);
           _0 = const ();
 -         StorageDead(_5);
--         StorageDead(_3);
+          StorageDead(_3);
 -         StorageDead(_2);
           return;
       }

tests/mir-opt/copy-prop/reborrow.miraw.CopyProp.panic-abort.diff

@@ -23,7 +23,7 @@
       bb0: {
 -         StorageLive(_2);
           _2 = &raw mut _1;
--         StorageLive(_3);
+          StorageLive(_3);
           _3 = &raw mut (*_2);
 -         StorageLive(_4);
 -         _4 = copy _2;
@@ -39,7 +39,7 @@
           StorageDead(_5);
           _0 = const ();
 -         StorageDead(_4);
--         StorageDead(_3);
+          StorageDead(_3);
 -         StorageDead(_2);
           return;
       }

tests/mir-opt/copy-prop/reborrow.miraw.CopyProp.panic-unwind.diff

@@ -23,7 +23,7 @@
       bb0: {
 -         StorageLive(_2);
           _2 = &raw mut _1;
--         StorageLive(_3);
+          StorageLive(_3);
           _3 = &raw mut (*_2);
 -         StorageLive(_4);
 -         _4 = copy _2;
@@ -39,7 +39,7 @@
           StorageDead(_5);
           _0 = const ();
 -         StorageDead(_4);
--         StorageDead(_3);
+          StorageDead(_3);
 -         StorageDead(_2);
           return;
       }