Do not remove unused definitions inside GVN.

Author: cjgillot

compiler/rustc_mir_transform/src/gvn.rs

@@ -111,16 +111,11 @@ fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
         let data = &mut body.basic_blocks.as_mut_preserves_cfg()[bb];
         state.visit_basic_block_data(bb, data);
     }
-    let any_replacement = state.any_replacement;
 
     // For each local that is reused (`y` above), we remove its storage statements do avoid any
     // difficulty. Those locals are SSA, so should be easy to optimize by LLVM without storage
     // statements.
     StorageRemover { tcx, reused_locals: state.reused_locals }.visit_body_preserves_cfg(body);
-
-    if any_replacement {
-        crate::simplify::remove_unused_definitions(body);
-    }
 }
 
 newtype_index! {
@@ -183,7 +178,6 @@ struct VnState<'body, 'tcx> {
     ssa: &'body SsaLocals,
     dominators: &'body Dominators<BasicBlock>,
     reused_locals: BitSet<Local>,
-    any_replacement: bool,
 }
 
 impl<'body, 'tcx> VnState<'body, 'tcx> {
@@ -205,7 +199,6 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             ssa,
             dominators,
             reused_locals: BitSet::new_empty(local_decls.len()),
-            any_replacement: false,
         }
     }
 
@@ -305,13 +298,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
         {
             *place = local.into();
             self.reused_locals.insert(local);
-            self.any_replacement = true;
         } else if place_ref.local != place.local
             || place_ref.projection.len() < place.projection.len()
         {
             *place = place_ref.project_deeper(&[], self.tcx);
             self.reused_locals.insert(place_ref.local);
-            self.any_replacement = true;
         }
 
         Some(value)
@@ -329,7 +320,6 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                 let value = self.simplify_place(place, location)?;
                 if let Some(const_) = self.try_as_constant(value) {
                     *operand = Operand::Constant(Box::new(const_));
-                    self.any_replacement = true;
                 }
                 Some(value)
             }
@@ -451,13 +441,11 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
         {
             if let Some(const_) = self.try_as_constant(value) {
                 *rvalue = Rvalue::Use(Operand::Constant(Box::new(const_)));
-                self.any_replacement = true;
             } else if let Some(local) = self.try_as_local(value, location)
                 && *rvalue != Rvalue::Use(Operand::Move(local.into()))
             {
                 *rvalue = Rvalue::Use(Operand::Copy(local.into()));
                 self.reused_locals.insert(local);
-                self.any_replacement = true;
             }
         }
     }

compiler/rustc_mir_transform/src/lib.rs

@@ -553,6 +553,7 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
             &separate_const_switch::SeparateConstSwitch,
             &const_prop::ConstProp,
             &gvn::GVN,
+            &simplify::SimplifyLocals::AfterGVN,
             &dataflow_const_prop::DataflowConstProp,
             //
             // Const-prop runs unconditionally, but doesn't mutate the MIR at mir-opt-level=0.

compiler/rustc_mir_transform/src/simplify.rs

@@ -458,13 +458,15 @@ fn save_unreachable_coverage(
 
 pub enum SimplifyLocals {
     BeforeConstProp,
+    AfterGVN,
     Final,
 }
 
 impl<'tcx> MirPass<'tcx> for SimplifyLocals {
     fn name(&self) -> &'static str {
         match &self {
             SimplifyLocals::BeforeConstProp => "SimplifyLocals-before-const-prop",
+            SimplifyLocals::AfterGVN => "SimplifyLocals-after-value-numbering",
             SimplifyLocals::Final => "SimplifyLocals-final",
         }
     }

tests/mir-opt/const_debuginfo.main.ConstDebugInfo.diff

@@ -4,12 +4,6 @@
   fn main() -> () {
       let mut _0: ();
       let _1: u8;
-      let mut _5: u8;
-      let mut _6: u8;
-      let mut _7: u8;
-      let mut _8: u8;
-      let mut _12: u32;
-      let mut _13: u32;
       scope 1 {
 -         debug x => _1;
 +         debug x => const 1_u8;
@@ -25,33 +19,33 @@
                   scope 4 {
 -                     debug sum => _4;
 +                     debug sum => const 6_u8;
-                      let _9: &str;
+                      let _5: &str;
                       scope 5 {
--                         debug s => _9;
+-                         debug s => _5;
 +                         debug s => const "hello, world!";
-                          let _14: bool;
-                          let _15: bool;
-                          let _16: u32;
+                          let _8: bool;
+                          let _9: bool;
+                          let _10: u32;
                           scope 6 {
--                             debug ((f: (bool, bool, u32)).0: bool) => _14;
--                             debug ((f: (bool, bool, u32)).1: bool) => _15;
--                             debug ((f: (bool, bool, u32)).2: u32) => _16;
+-                             debug ((f: (bool, bool, u32)).0: bool) => _8;
+-                             debug ((f: (bool, bool, u32)).1: bool) => _9;
+-                             debug ((f: (bool, bool, u32)).2: u32) => _10;
 +                             debug ((f: (bool, bool, u32)).0: bool) => const true;
 +                             debug ((f: (bool, bool, u32)).1: bool) => const false;
 +                             debug ((f: (bool, bool, u32)).2: u32) => const 123_u32;
-                              let _10: std::option::Option<u16>;
+                              let _6: std::option::Option<u16>;
                               scope 7 {
-                                  debug o => _10;
-                                  let _17: u32;
-                                  let _18: u32;
+                                  debug o => _6;
+                                  let _11: u32;
+                                  let _12: u32;
                                   scope 8 {
--                                     debug ((p: Point).0: u32) => _17;
--                                     debug ((p: Point).1: u32) => _18;
+-                                     debug ((p: Point).0: u32) => _11;
+-                                     debug ((p: Point).1: u32) => _12;
 +                                     debug ((p: Point).0: u32) => const 32_u32;
 +                                     debug ((p: Point).1: u32) => const 32_u32;
-                                      let _11: u32;
+                                      let _7: u32;
                                       scope 9 {
--                                         debug a => _11;
+-                                         debug a => _7;
 +                                         debug a => const 64_u32;
                                       }
                                   }
@@ -69,26 +63,26 @@
           _3 = const 3_u8;
           StorageLive(_4);
           _4 = const 6_u8;
+          StorageLive(_5);
+          _5 = const "hello, world!";
+          StorageLive(_8);
           StorageLive(_9);
-          _9 = const "hello, world!";
-          StorageLive(_14);
-          StorageLive(_15);
-          StorageLive(_16);
-          _14 = const true;
-          _15 = const false;
-          _16 = const 123_u32;
           StorageLive(_10);
-          _10 = Option::<u16>::Some(const 99_u16);
-          _17 = const 32_u32;
-          _18 = const 32_u32;
-          StorageLive(_11);
-          _11 = const 64_u32;
-          StorageDead(_11);
-          StorageDead(_10);
-          StorageDead(_14);
-          StorageDead(_15);
-          StorageDead(_16);
+          _8 = const true;
+          _9 = const false;
+          _10 = const 123_u32;
+          StorageLive(_6);
+          _6 = Option::<u16>::Some(const 99_u16);
+          _11 = const 32_u32;
+          _12 = const 32_u32;
+          StorageLive(_7);
+          _7 = const 64_u32;
+          StorageDead(_7);
+          StorageDead(_6);
+          StorageDead(_8);
           StorageDead(_9);
+          StorageDead(_10);
+          StorageDead(_5);
           StorageDead(_4);
           return;
       }