More GVN for PtrMetadata

scottmcm · scottmcm · commit aa1ee42cd7a7 · 2024-06-16T18:32:28.000-07:00
`PtrMetadata` doesn't care about `*const`/`*mut`/`&amp;`/`&amp;mut`, so GVN away those casts in its argument.

This includes updating MIR to allow calling PtrMetadata on references too, not just raw pointers.  That means that `[T]::len` can be just `_0 = PtrMetadata(_1)`, for example.
diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs
@@ -638,7 +638,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         (OperandValue::Immediate(llval), operand.layout)
                     }
                     mir::UnOp::PtrMetadata => {
-                        debug_assert!(operand.layout.ty.is_unsafe_ptr());
+                        debug_assert!(
+                            operand.layout.ty.is_unsafe_ptr() || operand.layout.ty.is_ref(),
+                        );
                         let (_, meta) = operand.val.pointer_parts();
                         assert_eq!(operand.layout.fields.count() > 1, meta.is_some());
                         if let Some(meta) = meta {
diff --git a/compiler/rustc_const_eval/src/interpret/operator.rs b/compiler/rustc_const_eval/src/interpret/operator.rs
@@ -460,7 +460,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 let res = ScalarInt::truncate_from_uint(res, layout.size).0;
                 Ok(ImmTy::from_scalar(res.into(), layout))
             }
-            ty::RawPtr(..) => {
+            ty::RawPtr(..) | ty::Ref(..) => {
                 assert_eq!(un_op, PtrMetadata);
                 let (_, meta) = val.to_scalar_and_meta();
                 Ok(match meta {
diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs
@@ -1434,10 +1434,12 @@ pub enum UnOp {
     Not,
     /// The `-` operator for negation
     Neg,
-    /// Get the metadata `M` from a `*const/mut impl Pointee<Metadata = M>`.
+    /// Gets the metadata `M` from a `*const`/`*mut`/`&`/`&mut` to
+    /// `impl Pointee<Metadata = M>`.
     ///
     /// For example, this will give a `()` from `*const i32`, a `usize` from
-    /// `*mut [u8]`, or a pointer to a vtable from a `*const dyn Foo`.
+    /// `&mut [u8]`, or a `ptr::DynMetadata<dyn Foo>` (internally a pointer)
+    /// from a `*mut dyn Foo`.
     ///
     /// Allowed only in [`MirPhase::Runtime`]; earlier it's an intrinsic.
     PtrMetadata,
diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs
@@ -840,8 +840,51 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                 }
                 Value::BinaryOp(op, lhs, rhs)
             }
-            Rvalue::UnaryOp(op, ref mut arg) => {
-                let arg = self.simplify_operand(arg, location)?;
+            Rvalue::UnaryOp(op, ref mut arg_op) => {
+                let mut arg = self.simplify_operand(arg_op, location)?;
+
+                // PtrMetadata doesn't care about *const vs *mut vs & vs &mut,
+                // so start by removing those distinctions so we can update the `Operand`
+                if op == UnOp::PtrMetadata {
+                    let mut was_updated = false;
+                    loop {
+                        let value = self.get(arg);
+
+                        // FIXME: remove MutToConst after #126308
+                        if let Value::Cast {
+                            kind:
+                                CastKind::PtrToPtr
+                                | CastKind::PointerCoercion(
+                                    ty::adjustment::PointerCoercion::MutToConstPointer,
+                                ),
+                            value: inner,
+                            from,
+                            to,
+                        } = value
+                            && from.builtin_deref(true) == to.builtin_deref(true)
+                        {
+                            arg = *inner;
+                            was_updated = true;
+                            continue;
+                        }
+
+                        if let Value::Address { place, kind: _, provenance: _ } = value
+                            && let PlaceRef { local, projection: [PlaceElem::Deref] } =
+                                place.as_ref()
+                            && let Some(local_index) = self.locals[local]
+                        {
+                            arg = local_index;
+                            was_updated = true;
+                            continue;
+                        }
+
+                        if was_updated && let Some(op) = self.try_as_operand(arg, location) {
+                            *arg_op = op;
+                        }
+                        break;
+                    }
+                }
+
                 if let Some(value) = self.simplify_unary(op, arg) {
                     return Some(value);
                 }
@@ -988,6 +1031,24 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             (UnOp::PtrMetadata, Value::Aggregate(AggregateTy::RawPtr { .. }, _, fields)) => {
                 return Some(fields[1]);
             }
+            // We have an unsizing cast, which assigns the length to fat pointer metadata.
+            (
+                UnOp::PtrMetadata,
+                Value::Cast {
+                    kind: CastKind::PointerCoercion(ty::adjustment::PointerCoercion::Unsize),
+                    from,
+                    to,
+                    ..
+                },
+            ) if let ty::Slice(..) = to.builtin_deref(true).unwrap().kind()
+                && let ty::Array(_, len) = from.builtin_deref(true).unwrap().kind() =>
+            {
+                return self.insert_constant(Const::from_ty_const(
+                    *len,
+                    self.tcx.types.usize,
+                    self.tcx,
+                ));
+            }
             _ => return None,
         };
 
diff --git a/compiler/rustc_mir_transform/src/validate.rs b/compiler/rustc_mir_transform/src/validate.rs
@@ -1116,12 +1116,17 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                     UnOp::PtrMetadata => {
                         if !matches!(self.mir_phase, MirPhase::Runtime(_)) {
                             // It would probably be fine to support this in earlier phases,
-                            // but at the time of writing it's only ever introduced from intrinsic lowering,
+                            // but at the time of writing it's only ever introduced from intrinsic lowering
+                            // or other runtime-phase optimization passes,
                             // so earlier things can just `bug!` on it.
                             self.fail(location, "PtrMetadata should be in runtime MIR only");
                         }
 
-                        check_kinds!(a, "Cannot PtrMetadata non-pointer type {:?}", ty::RawPtr(..));
+                        check_kinds!(
+                            a,
+                            "Cannot PtrMetadata non-pointer non-reference type {:?}",
+                            ty::RawPtr(..) | ty::Ref(..)
+                        );
                     }
                 }
             }
diff --git a/tests/mir-opt/gvn.array_len.GVN.panic-abort.diff b/tests/mir-opt/gvn.array_len.GVN.panic-abort.diff
@@ -0,0 +1,36 @@
+- // MIR for `array_len` before GVN
++ // MIR for `array_len` after GVN
+  
+  fn array_len(_1: &mut [i32; 42]) -> usize {
+      debug x => _1;
+      let mut _0: usize;
+      let _2: *mut [i32];
+      let mut _3: *mut [i32; 42];
+      let mut _4: *const [i32];
+      let mut _5: *mut [i32];
+      scope 1 {
+          debug x => _2;
+      }
+  
+      bb0: {
+-         StorageLive(_2);
++         nop;
+          StorageLive(_3);
+          _3 = &raw mut (*_1);
+          _2 = move _3 as *mut [i32] (PointerCoercion(Unsize));
+          StorageDead(_3);
+          StorageLive(_4);
+          StorageLive(_5);
+          _5 = _2;
+-         _4 = move _5 as *const [i32] (PointerCoercion(MutToConstPointer));
++         _4 = _2 as *const [i32] (PointerCoercion(MutToConstPointer));
+          StorageDead(_5);
+-         _0 = PtrMetadata(move _4);
++         _0 = const 42_usize;
+          StorageDead(_4);
+-         StorageDead(_2);
++         nop;
+          return;
+      }
+  }
+  
diff --git a/tests/mir-opt/gvn.array_len.GVN.panic-unwind.diff b/tests/mir-opt/gvn.array_len.GVN.panic-unwind.diff
@@ -0,0 +1,36 @@
+- // MIR for `array_len` before GVN
++ // MIR for `array_len` after GVN
+  
+  fn array_len(_1: &mut [i32; 42]) -> usize {
+      debug x => _1;
+      let mut _0: usize;
+      let _2: *mut [i32];
+      let mut _3: *mut [i32; 42];
+      let mut _4: *const [i32];
+      let mut _5: *mut [i32];
+      scope 1 {
+          debug x => _2;
+      }
+  
+      bb0: {
+-         StorageLive(_2);
++         nop;
+          StorageLive(_3);
+          _3 = &raw mut (*_1);
+          _2 = move _3 as *mut [i32] (PointerCoercion(Unsize));
+          StorageDead(_3);
+          StorageLive(_4);
+          StorageLive(_5);
+          _5 = _2;
+-         _4 = move _5 as *const [i32] (PointerCoercion(MutToConstPointer));
++         _4 = _2 as *const [i32] (PointerCoercion(MutToConstPointer));
+          StorageDead(_5);
+-         _0 = PtrMetadata(move _4);
++         _0 = const 42_usize;
+          StorageDead(_4);
+-         StorageDead(_2);
++         nop;
+          return;
+      }
+  }
+  
diff --git a/tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff b/tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff
@@ -8,18 +8,18 @@
       let mut _3: fn(u8) -> u8;
       let _5: ();
       let mut _6: fn(u8) -> u8;
-      let mut _9: {closure@$DIR/gvn.rs:612:19: 612:21};
+      let mut _9: {closure@$DIR/gvn.rs:614:19: 614:21};
       let _10: ();
       let mut _11: fn();
-      let mut _13: {closure@$DIR/gvn.rs:612:19: 612:21};
+      let mut _13: {closure@$DIR/gvn.rs:614:19: 614:21};
       let _14: ();
       let mut _15: fn();
       scope 1 {
           debug f => _1;
           let _4: fn(u8) -> u8;
           scope 2 {
               debug g => _4;
-              let _7: {closure@$DIR/gvn.rs:612:19: 612:21};
+              let _7: {closure@$DIR/gvn.rs:614:19: 614:21};
               scope 3 {
                   debug closure => _7;
                   let _8: fn();
@@ -62,16 +62,16 @@
           StorageDead(_6);
           StorageDead(_5);
 -         StorageLive(_7);
--         _7 = {closure@$DIR/gvn.rs:612:19: 612:21};
+-         _7 = {closure@$DIR/gvn.rs:614:19: 614:21};
 -         StorageLive(_8);
 +         nop;
-+         _7 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
++         _7 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
 +         nop;
           StorageLive(_9);
 -         _9 = _7;
 -         _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Safe)));
-+         _9 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
-+         _8 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
++         _9 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
++         _8 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
           StorageDead(_9);
           StorageLive(_10);
           StorageLive(_11);
@@ -88,8 +88,8 @@
           StorageLive(_13);
 -         _13 = _7;
 -         _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Safe)));
-+         _13 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
-+         _12 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
++         _13 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
++         _12 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
           StorageDead(_13);
           StorageLive(_14);
           StorageLive(_15);
diff --git a/tests/mir-opt/gvn.fn_pointers.GVN.panic-unwind.diff b/tests/mir-opt/gvn.fn_pointers.GVN.panic-unwind.diff
@@ -8,18 +8,18 @@
       let mut _3: fn(u8) -> u8;
       let _5: ();
       let mut _6: fn(u8) -> u8;
-      let mut _9: {closure@$DIR/gvn.rs:612:19: 612:21};
+      let mut _9: {closure@$DIR/gvn.rs:614:19: 614:21};
       let _10: ();
       let mut _11: fn();
-      let mut _13: {closure@$DIR/gvn.rs:612:19: 612:21};
+      let mut _13: {closure@$DIR/gvn.rs:614:19: 614:21};
       let _14: ();
       let mut _15: fn();
       scope 1 {
           debug f => _1;
           let _4: fn(u8) -> u8;
           scope 2 {
               debug g => _4;
-              let _7: {closure@$DIR/gvn.rs:612:19: 612:21};
+              let _7: {closure@$DIR/gvn.rs:614:19: 614:21};
               scope 3 {
                   debug closure => _7;
                   let _8: fn();
@@ -62,16 +62,16 @@
           StorageDead(_6);
           StorageDead(_5);
 -         StorageLive(_7);
--         _7 = {closure@$DIR/gvn.rs:612:19: 612:21};
+-         _7 = {closure@$DIR/gvn.rs:614:19: 614:21};
 -         StorageLive(_8);
 +         nop;
-+         _7 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
++         _7 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
 +         nop;
           StorageLive(_9);
 -         _9 = _7;
 -         _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Safe)));
-+         _9 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
-+         _8 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
++         _9 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
++         _8 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
           StorageDead(_9);
           StorageLive(_10);
           StorageLive(_11);
@@ -88,8 +88,8 @@
           StorageLive(_13);
 -         _13 = _7;
 -         _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Safe)));
-+         _13 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
-+         _12 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
++         _13 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
++         _12 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
           StorageDead(_13);
           StorageLive(_14);
           StorageLive(_15);
diff --git a/tests/mir-opt/gvn.manual_slice_mut_len.GVN.panic-abort.diff b/tests/mir-opt/gvn.manual_slice_mut_len.GVN.panic-abort.diff
@@ -0,0 +1,41 @@
+- // MIR for `manual_slice_mut_len` before GVN
++ // MIR for `manual_slice_mut_len` after GVN
+  
+  fn manual_slice_mut_len(_1: &mut [i32]) -> usize {
+      debug x => _1;
+      let mut _0: usize;
+      let _2: *mut [i32];
+      let mut _4: *mut [i32];
+      let mut _5: *const [i32];
+      scope 1 {
+          debug x => _2;
+          let _3: *const [i32];
+          scope 2 {
+              debug x => _3;
+          }
+      }
+  
+      bb0: {
+-         StorageLive(_2);
++         nop;
+          _2 = &raw mut (*_1);
+-         StorageLive(_3);
++         nop;
+          StorageLive(_4);
+          _4 = _2;
+-         _3 = move _4 as *const [i32] (PointerCoercion(MutToConstPointer));
++         _3 = _2 as *const [i32] (PointerCoercion(MutToConstPointer));
+          StorageDead(_4);
+          StorageLive(_5);
+          _5 = _3;
+-         _0 = PtrMetadata(move _5);
++         _0 = PtrMetadata(_1);
+          StorageDead(_5);
+-         StorageDead(_3);
+-         StorageDead(_2);
++         nop;
++         nop;
+          return;
+      }
+  }
+  
diff --git a/tests/mir-opt/gvn.manual_slice_mut_len.GVN.panic-unwind.diff b/tests/mir-opt/gvn.manual_slice_mut_len.GVN.panic-unwind.diff
@@ -0,0 +1,41 @@
+- // MIR for `manual_slice_mut_len` before GVN
++ // MIR for `manual_slice_mut_len` after GVN
+  
+  fn manual_slice_mut_len(_1: &mut [i32]) -> usize {
+      debug x => _1;
+      let mut _0: usize;
+      let _2: *mut [i32];
+      let mut _4: *mut [i32];
+      let mut _5: *const [i32];
+      scope 1 {
+          debug x => _2;
+          let _3: *const [i32];
+          scope 2 {
+              debug x => _3;
+          }
+      }
+  
+      bb0: {
+-         StorageLive(_2);
++         nop;
+          _2 = &raw mut (*_1);
+-         StorageLive(_3);
++         nop;
+          StorageLive(_4);
+          _4 = _2;
+-         _3 = move _4 as *const [i32] (PointerCoercion(MutToConstPointer));
++         _3 = _2 as *const [i32] (PointerCoercion(MutToConstPointer));
+          StorageDead(_4);
+          StorageLive(_5);
+          _5 = _3;
+-         _0 = PtrMetadata(move _5);
++         _0 = PtrMetadata(_1);
+          StorageDead(_5);
+-         StorageDead(_3);
+-         StorageDead(_2);
++         nop;
++         nop;
+          return;
+      }
+  }
+  
diff --git a/tests/mir-opt/gvn.rs b/tests/mir-opt/gvn.rs
diff --git a/tests/mir-opt/pre-codegen/slice_index.slice_get_unchecked_mut_range.PreCodegen.after.panic-abort.mir b/tests/mir-opt/pre-codegen/slice_index.slice_get_unchecked_mut_range.PreCodegen.after.panic-abort.mir
diff --git a/tests/mir-opt/pre-codegen/slice_index.slice_get_unchecked_mut_range.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/pre-codegen/slice_index.slice_get_unchecked_mut_range.PreCodegen.after.panic-unwind.mir

Original file line number	Diff line number	Diff line change
`@@ -460,7 +460,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {`
`460`	`460`	`let res = ScalarInt::truncate_from_uint(res, layout.size).0;`
`461`	`461`	`Ok(ImmTy::from_scalar(res.into(), layout))`
`462`	`462`	`}`
`463`		`- ty::RawPtr(..) => {`
	`463`	`+ ty::RawPtr(..) \| ty::Ref(..) => {`
`464`	`464`	`assert_eq!(un_op, PtrMetadata);`
`465`	`465`	`let (_, meta) = val.to_scalar_and_meta();`
`466`	`466`	`Ok(match meta {`
Original file line number	Diff line number	Diff line change
`@@ -1116,12 +1116,17 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {`
`1116`	`1116`	`UnOp::PtrMetadata => {`
`1117`	`1117`	`if !matches!(self.mir_phase, MirPhase::Runtime(_)) {`
`1118`	`1118`	`// It would probably be fine to support this in earlier phases,`
`1119`		`- // but at the time of writing it's only ever introduced from intrinsic lowering,`
	`1119`	`+ // but at the time of writing it's only ever introduced from intrinsic lowering`
	`1120`	`+ // or other runtime-phase optimization passes,`
`1120`	`1121`	// so earlier things can just `bug!` on it.
`1121`	`1122`	`self.fail(location, "PtrMetadata should be in runtime MIR only");`
`1122`	`1123`	`}`
`1123`	`1124`
`1124`		`- check_kinds!(a, "Cannot PtrMetadata non-pointer type {:?}", ty::RawPtr(..));`
	`1125`	`+ check_kinds!(`
	`1126`	`+ a,`
	`1127`	`+ "Cannot PtrMetadata non-pointer non-reference type {:?}",`
	`1128`	`+ ty::RawPtr(..) \| ty::Ref(..)`
	`1129`	`+ );`
`1125`	`1130`	`}`
`1126`	`1131`	`}`
`1127`	`1132`	`}`