Allow GVN to produce places and not just locals.

compiler/rustc_mir_transform/src/gvn.rs

@@ -980,27 +980,17 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             }
         }
 
-        let tcx = self.tcx;
-        let mut projection = SmallVec::<[PlaceElem<'tcx>; 1]>::new();
-        loop {
-            if let Some(local) = self.try_as_local(copy_from_local_value, location) {
-                projection.reverse();
-                let place = Place { local, projection: tcx.mk_place_elems(projection.as_slice()) };
-                if rvalue.ty(self.local_decls, tcx) == place.ty(self.local_decls, tcx).ty {
-                    self.reused_locals.insert(local);
-                    *rvalue = Rvalue::Use(Operand::Copy(place));
-                    return Some(copy_from_value);
-                }
-                return None;
-            } else if let Value::Projection(pointer, proj) = *self.get(copy_from_local_value)
-                && let Some(proj) = self.try_as_place_elem(proj, location)
-            {
-                projection.push(proj);
-                copy_from_local_value = pointer;
-            } else {
-                return None;
+        // Allow introducing places with non-constant offsets, as those are still better than
+        // reconstructing an aggregate.
+        if let Some(place) = self.try_as_place(copy_from_local_value, location, true) {
+            if rvalue.ty(self.local_decls, self.tcx) == place.ty(self.local_decls, self.tcx).ty {
+                self.reused_locals.insert(place.local);
+                *rvalue = Rvalue::Use(Operand::Copy(place));
+                return Some(copy_from_local_value);
             }
         }
+
+        None
     }
 
     fn simplify_aggregate(
@@ -1672,14 +1662,14 @@ fn op_to_prop_const<'tcx>(
 }
 
 impl<'tcx> VnState<'_, 'tcx> {
-    /// If either [`Self::try_as_constant`] as [`Self::try_as_local`] succeeds,
+    /// If either [`Self::try_as_constant`] as [`Self::try_as_place`] succeeds,
     /// returns that result as an [`Operand`].
     fn try_as_operand(&mut self, index: VnIndex, location: Location) -> Option<Operand<'tcx>> {
         if let Some(const_) = self.try_as_constant(index) {
             Some(Operand::Constant(Box::new(const_)))
-        } else if let Some(local) = self.try_as_local(index, location) {
-            self.reused_locals.insert(local);
-            Some(Operand::Copy(local.into()))
+        } else if let Some(place) = self.try_as_place(index, location, false) {
+            self.reused_locals.insert(place.local);
+            Some(Operand::Copy(place))
         } else {
             None
         }
@@ -1712,6 +1702,35 @@ impl<'tcx> VnState<'_, 'tcx> {
         Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_ })
     }
 
+    /// Construct a place which holds the same value as `index` and for which all locals strictly
+    /// dominate `loc`. If you used this place, add its base local to `reused_locals` to remove
+    /// storage statements.
+    #[instrument(level = "trace", skip(self), ret)]
+    fn try_as_place(
+        &mut self,
+        mut index: VnIndex,
+        loc: Location,
+        allow_complex_projection: bool,
+    ) -> Option<Place<'tcx>> {
+        let mut projection = SmallVec::<[PlaceElem<'tcx>; 1]>::new();
+        loop {
+            if let Some(local) = self.try_as_local(index, loc) {
+                projection.reverse();
+                let place =
+                    Place { local, projection: self.tcx.mk_place_elems(projection.as_slice()) };
+                return Some(place);
+            } else if let Value::Projection(pointer, proj) = *self.get(index)
+                && (allow_complex_projection || proj.is_stable_offset())
+                && let Some(proj) = self.try_as_place_elem(proj, loc)
+            {
+                projection.push(proj);
+                index = pointer;
+            } else {
+                return None;
+            }
+        }
+    }
+
     /// If there is a local which is assigned `index`, and its assignment strictly dominates `loc`,
     /// return it. If you used this local, add it to `reused_locals` to remove storage statements.
     fn try_as_local(&mut self, index: VnIndex, loc: Location) -> Option<Local> {
@@ -1762,11 +1781,12 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
             if let Some(value) = value {
                 if let Some(const_) = self.try_as_constant(value) {
                     *rvalue = Rvalue::Use(Operand::Constant(Box::new(const_)));
-                } else if let Some(local) = self.try_as_local(value, location)
-                    && *rvalue != Rvalue::Use(Operand::Move(local.into()))
+                } else if let Some(place) = self.try_as_place(value, location, false)
+                    && *rvalue != Rvalue::Use(Operand::Move(place))
+                    && *rvalue != Rvalue::Use(Operand::Copy(place))
                 {
-                    *rvalue = Rvalue::Use(Operand::Copy(local.into()));
-                    self.reused_locals.insert(local);
+                    *rvalue = Rvalue::Use(Operand::Copy(place));
+                    self.reused_locals.insert(place.local);
                 }
             }
         }

tests/mir-opt/const_prop/address_of_pair.fn0.GVN.diff

@@ -31,15 +31,13 @@
           (*_3) = const true;
           _4 = const ();
           StorageDead(_4);
--         StorageLive(_5);
-+         nop;
+          StorageLive(_5);
           StorageLive(_6);
           _6 = copy (_2.1: bool);
           _5 = Not(move _6);
           StorageDead(_6);
           _0 = copy _5;
--         StorageDead(_5);
-+         nop;
+          StorageDead(_5);
           StorageDead(_3);
           StorageDead(_2);
           return;

tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.32bit.panic-abort.diff

@@ -69,8 +69,7 @@
 
       bb0: {
           StorageLive(_1);
--         StorageLive(_2);
-+         nop;
+          StorageLive(_2);
           StorageLive(_3);
           StorageLive(_11);
           StorageLive(_12);
@@ -119,8 +118,7 @@
           StorageDead(_11);
           _2 = &_3;
           _1 = copy _2;
--         StorageDead(_2);
-+         nop;
+          StorageDead(_2);
           StorageLive(_4);
 -         _9 = deref_copy _3;
 +         _9 = copy _3;
@@ -141,7 +139,7 @@
           StorageLive(_8);
           _8 = copy _5;
 -         _7 = copy _8 as *mut () (PtrToPtr);
-+         _7 = copy _5 as *mut () (PtrToPtr);
++         _7 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *mut () (Transmute);
           StorageDead(_8);
           StorageDead(_7);
 -         StorageDead(_5);

tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.32bit.panic-unwind.diff

@@ -35,17 +35,15 @@
 
       bb0: {
           StorageLive(_1);
--         StorageLive(_2);
-+         nop;
+          StorageLive(_2);
           StorageLive(_3);
           _3 = Box::<()>::new(const ()) -> [return: bb1, unwind continue];
       }
 
       bb1: {
           _2 = &_3;
           _1 = copy _2;
--         StorageDead(_2);
-+         nop;
+          StorageDead(_2);
           StorageLive(_4);
 -         _9 = deref_copy _3;
 +         _9 = copy _3;
@@ -66,7 +64,7 @@
           StorageLive(_8);
           _8 = copy _5;
 -         _7 = copy _8 as *mut () (PtrToPtr);
-+         _7 = copy _5 as *mut () (PtrToPtr);
++         _7 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *mut () (Transmute);
           StorageDead(_8);
           StorageDead(_7);
 -         StorageDead(_5);

tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.64bit.panic-abort.diff

@@ -69,8 +69,7 @@
 
       bb0: {
           StorageLive(_1);
--         StorageLive(_2);
-+         nop;
+          StorageLive(_2);
           StorageLive(_3);
           StorageLive(_11);
           StorageLive(_12);
@@ -119,8 +118,7 @@
           StorageDead(_11);
           _2 = &_3;
           _1 = copy _2;
--         StorageDead(_2);
-+         nop;
+          StorageDead(_2);
           StorageLive(_4);
 -         _9 = deref_copy _3;
 +         _9 = copy _3;
@@ -141,7 +139,7 @@
           StorageLive(_8);
           _8 = copy _5;
 -         _7 = copy _8 as *mut () (PtrToPtr);
-+         _7 = copy _5 as *mut () (PtrToPtr);
++         _7 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *mut () (Transmute);
           StorageDead(_8);
           StorageDead(_7);
 -         StorageDead(_5);

tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.64bit.panic-unwind.diff

@@ -35,17 +35,15 @@
 
       bb0: {
           StorageLive(_1);
--         StorageLive(_2);
-+         nop;
+          StorageLive(_2);
           StorageLive(_3);
           _3 = Box::<()>::new(const ()) -> [return: bb1, unwind continue];
       }
 
       bb1: {
           _2 = &_3;
           _1 = copy _2;
--         StorageDead(_2);
-+         nop;
+          StorageDead(_2);
           StorageLive(_4);
 -         _9 = deref_copy _3;
 +         _9 = copy _3;
@@ -66,7 +64,7 @@
           StorageLive(_8);
           _8 = copy _5;
 -         _7 = copy _8 as *mut () (PtrToPtr);
-+         _7 = copy _5 as *mut () (PtrToPtr);
++         _7 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *mut () (Transmute);
           StorageDead(_8);
           StorageDead(_7);
 -         StorageDead(_5);

tests/mir-opt/gvn_copy_constant_projection.compare_constant_index.GVN.panic-abort.diff

@@ -0,0 +1,58 @@
+- // MIR for `compare_constant_index` before GVN
++ // MIR for `compare_constant_index` after GVN
+
+  fn compare_constant_index(_1: [i32; 1], _2: [i32; 1]) -> std::cmp::Ordering {
+      debug x => _1;
+      debug y => _2;
+      let mut _0: std::cmp::Ordering;
+      let _3: &i32;
+      let _4: usize;
+      let mut _5: bool;
+      let _6: &i32;
+      let _7: usize;
+      let mut _8: bool;
+      scope 1 (inlined std::cmp::impls::<impl Ord for i32>::cmp) {
+          let mut _9: i32;
+          let mut _10: i32;
+      }
+
+      bb0: {
+-         StorageLive(_4);
++         nop;
+          _4 = const 0_usize;
+-         _5 = Lt(copy _4, const 1_usize);
+-         assert(move _5, "index out of bounds: the length is {} but the index is {}", const 1_usize, copy _4) -> [success: bb1, unwind unreachable];
++         _5 = const true;
++         assert(const true, "index out of bounds: the length is {} but the index is {}", const 1_usize, const 0_usize) -> [success: bb1, unwind unreachable];
+      }
+
+      bb1: {
+-         _3 = &_1[_4];
++         _3 = &_1[0 of 1];
+          StorageLive(_7);
+          _7 = const 0_usize;
+-         _8 = Lt(copy _7, const 1_usize);
+-         assert(move _8, "index out of bounds: the length is {} but the index is {}", const 1_usize, copy _7) -> [success: bb2, unwind unreachable];
++         _8 = const true;
++         assert(const true, "index out of bounds: the length is {} but the index is {}", const 1_usize, const 0_usize) -> [success: bb2, unwind unreachable];
+      }
+
+      bb2: {
+-         _6 = &_2[_7];
++         _6 = &_2[0 of 1];
+          StorageLive(_9);
+-         _9 = copy (*_3);
++         _9 = copy _1[0 of 1];
+          StorageLive(_10);
+-         _10 = copy (*_6);
++         _10 = copy _2[0 of 1];
+          _0 = Cmp(move _9, move _10);
+          StorageDead(_10);
+          StorageDead(_9);
+          StorageDead(_7);
+-         StorageDead(_4);
++         nop;
+          return;
+      }
+  }
+

tests/mir-opt/gvn_copy_constant_projection.compare_constant_index.GVN.panic-unwind.diff

@@ -0,0 +1,58 @@
+- // MIR for `compare_constant_index` before GVN
++ // MIR for `compare_constant_index` after GVN
+
+  fn compare_constant_index(_1: [i32; 1], _2: [i32; 1]) -> std::cmp::Ordering {
+      debug x => _1;
+      debug y => _2;
+      let mut _0: std::cmp::Ordering;
+      let _3: &i32;
+      let _4: usize;
+      let mut _5: bool;
+      let _6: &i32;
+      let _7: usize;
+      let mut _8: bool;
+      scope 1 (inlined std::cmp::impls::<impl Ord for i32>::cmp) {
+          let mut _9: i32;
+          let mut _10: i32;
+      }
+
+      bb0: {
+-         StorageLive(_4);
++         nop;
+          _4 = const 0_usize;
+-         _5 = Lt(copy _4, const 1_usize);
+-         assert(move _5, "index out of bounds: the length is {} but the index is {}", const 1_usize, copy _4) -> [success: bb1, unwind continue];
++         _5 = const true;
++         assert(const true, "index out of bounds: the length is {} but the index is {}", const 1_usize, const 0_usize) -> [success: bb1, unwind continue];
+      }
+
+      bb1: {
+-         _3 = &_1[_4];
++         _3 = &_1[0 of 1];
+          StorageLive(_7);
+          _7 = const 0_usize;
+-         _8 = Lt(copy _7, const 1_usize);
+-         assert(move _8, "index out of bounds: the length is {} but the index is {}", const 1_usize, copy _7) -> [success: bb2, unwind continue];
++         _8 = const true;
++         assert(const true, "index out of bounds: the length is {} but the index is {}", const 1_usize, const 0_usize) -> [success: bb2, unwind continue];
+      }
+
+      bb2: {
+-         _6 = &_2[_7];
++         _6 = &_2[0 of 1];
+          StorageLive(_9);
+-         _9 = copy (*_3);
++         _9 = copy _1[0 of 1];
+          StorageLive(_10);
+-         _10 = copy (*_6);
++         _10 = copy _2[0 of 1];
+          _0 = Cmp(move _9, move _10);
+          StorageDead(_10);
+          StorageDead(_9);
+          StorageDead(_7);
+-         StorageDead(_4);
++         nop;
+          return;
+      }
+  }
+

tests/mir-opt/gvn_copy_constant_projection.rs

@@ -0,0 +1,18 @@
+// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
+
+use std::cmp::Ordering;
+fn compare_constant_index(x: [i32; 1], y: [i32; 1]) -> Ordering {
+    // CHECK-LABEL: fn compare_constant_index(
+    // CHECK-NOT: (*{{_.*}});
+    // CHECK: [[lhs:_.*]] = copy _1[0 of 1];
+    // CHECK-NOT: (*{{_.*}});
+    // CHECK: [[rhs:_.*]] = copy _2[0 of 1];
+    // CHECK: _0 = Cmp(move [[lhs]], move [[rhs]]);
+    Ord::cmp(&x[0], &y[0])
+}
+
+fn main() {
+    compare_constant_index([1], [2]);
+}
+
+// EMIT_MIR gvn_copy_constant_projection.compare_constant_index.GVN.diff