Auto merge of #126541 - scottmcm:more-ptr-metadata-gvn, r=<try>

More ptr metadata gvn

There's basically 3 parts to this PR.

1. Allow references as arguments to `UnOp::PtrMetadata`

This is a MIR semantics addition, so
r? mir

Rather than just raw pointers, also allow references to be passed to `PtrMetadata`.  That means the length of a slice can be just `PtrMetadata(_1)` instead of also needing a ref-to-pointer statement (`_2 = &raw *_1` + `PtrMetadata(_2)`).

AFAIK there should be no provenance or tagging implications of looking at the *metadata* of a pointer, and the code in the backends actually already supported it (other than a debug assert, given that they don't care about ptr vs reference, really), so we might as well allow it.

2. Simplify the argument to `PtrMetadata` in GVN

Because the specific kind of pointer-like thing isn't that important, GVN can simplify all those details away.  Things like `*const`-to-`*mut` casts and `&mut`-to-`&` reborrows are irrelevant, and skipping them lets it see more interesting things.

cc `@cjgillot`

Notably, unsizing casts for arrays.  GVN supported that for `Len`, and now it sees it for `PtrMetadata` as well, allowing `PtrMetadata(pointer)` to become a constant if that pointer came from an array-to-slice unsizing, even through a bunch of other possible steps.

3. Replace `NormalizeArrayLen` with GVN

The `NormalizeArrayLen` pass hasn't been running even in optimized builds for well over a year, and it turns out that GVN -- which *is* on in optimized builds -- can do everything it was trying to do.

So the code for the pass is deleted, but the tests are kept, just changed to the different pass.

As part of this, `LowerSliceLen` was changed to emit `PtrMetadata(_1)` instead of `Len(*_1)`, a small step on the road to eventually eliminating `Rvalue::Len`.

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 {

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 {

compiler/rustc_mir_transform/src/gvn.rs

@@ -840,8 +840,56 @@ 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 {
+                            if let Some(const_) = self.try_as_constant(arg) {
+                                *arg_op = Operand::Constant(Box::new(const_));
+                            } else if let Some(local) = self.try_as_local(arg, location) {
+                                *arg_op = Operand::Copy(Place::from(local));
+                                self.reused_locals.insert(local);
+                            }
+                        }
+                        break;
+                    }
+                }
+
                 if let Some(value) = self.simplify_unary(op, arg) {
                     return Some(value);
                 }
@@ -996,6 +1044,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,
         };
 

compiler/rustc_mir_transform/src/lib.rs

@@ -88,7 +88,6 @@ mod lower_slice_len;
 mod match_branches;
 mod mentioned_items;
 mod multiple_return_terminators;
-mod normalize_array_len;
 mod nrvo;
 mod prettify;
 mod promote_consts;
@@ -581,9 +580,6 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
             &o1(simplify::SimplifyCfg::AfterUnreachableEnumBranching),
             // Inlining may have introduced a lot of redundant code and a large move pattern.
             // Now, we need to shrink the generated MIR.
-
-            // Has to run after `slice::len` lowering
-            &normalize_array_len::NormalizeArrayLen,
             &ref_prop::ReferencePropagation,
             &sroa::ScalarReplacementOfAggregates,
             &match_branches::MatchBranchSimplification,

compiler/rustc_mir_transform/src/lower_slice_len.rs

@@ -1,10 +1,9 @@
-//! This pass lowers calls to core::slice::len to just Len op.
+//! This pass lowers calls to core::slice::len to just PtrMetadata op.
 //! It should run before inlining!
 
 use rustc_hir::def_id::DefId;
-use rustc_index::IndexSlice;
 use rustc_middle::mir::*;
-use rustc_middle::ty::{self, TyCtxt};
+use rustc_middle::ty::TyCtxt;
 
 pub struct LowerSliceLenCalls;
 
@@ -29,16 +28,11 @@ pub fn lower_slice_len_calls<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
     let basic_blocks = body.basic_blocks.as_mut_preserves_cfg();
     for block in basic_blocks {
         // lower `<[_]>::len` calls
-        lower_slice_len_call(tcx, block, &body.local_decls, slice_len_fn_item_def_id);
+        lower_slice_len_call(block, slice_len_fn_item_def_id);
     }
 }
 
-fn lower_slice_len_call<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    block: &mut BasicBlockData<'tcx>,
-    local_decls: &IndexSlice<Local, LocalDecl<'tcx>>,
-    slice_len_fn_item_def_id: DefId,
-) {
+fn lower_slice_len_call<'tcx>(block: &mut BasicBlockData<'tcx>, slice_len_fn_item_def_id: DefId) {
     let terminator = block.terminator();
     if let TerminatorKind::Call {
         func,
@@ -50,19 +44,17 @@ fn lower_slice_len_call<'tcx>(
     } = &terminator.kind
         // some heuristics for fast rejection
         && let [arg] = &args[..]
-        && let Some(arg) = arg.node.place()
-        && let ty::FnDef(fn_def_id, _) = func.ty(local_decls, tcx).kind()
-        && *fn_def_id == slice_len_fn_item_def_id
+        && let Some((fn_def_id, _)) = func.const_fn_def()
+        && fn_def_id == slice_len_fn_item_def_id
     {
         // perform modifications from something like:
         //     _5 = core::slice::<impl [u8]>::len(move _6) -> bb1
         // into:
-        //     _5 = Len(*_6)
+        //     _5 = PtrMetadata(move _6)
         //     goto bb1
 
         // make new RValue for Len
-        let deref_arg = tcx.mk_place_deref(arg);
-        let r_value = Rvalue::Len(deref_arg);
+        let r_value = Rvalue::UnaryOp(UnOp::PtrMetadata, arg.node.clone());
         let len_statement_kind = StatementKind::Assign(Box::new((*destination, r_value)));
         let add_statement =
             Statement { kind: len_statement_kind, source_info: terminator.source_info };

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(..)
+                        );
                     }
                 }
             }

tests/coverage/closure_macro.cov-map

@@ -22,19 +22,19 @@ Number of file 0 mappings: 5
 - Code(Counter(0)) at (prev + 3, 1) to (start + 0, 2)
 
 Function name: closure_macro::main::{closure#0}
-Raw bytes (35): 0x[01, 01, 03, 01, 05, 05, 0b, 09, 00, 05, 01, 10, 1c, 03, 21, 05, 04, 11, 01, 27, 02, 03, 11, 00, 16, 00, 00, 17, 00, 1e, 07, 02, 09, 00, 0a]
+Raw bytes (35): 0x[01, 01, 03, 01, 05, 05, 0b, 09, 0d, 05, 01, 10, 1c, 03, 21, 05, 04, 11, 01, 27, 02, 03, 11, 00, 16, 0d, 00, 17, 00, 1e, 07, 02, 09, 00, 0a]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 3
 - expression 0 operands: lhs = Counter(0), rhs = Counter(1)
 - expression 1 operands: lhs = Counter(1), rhs = Expression(2, Add)
-- expression 2 operands: lhs = Counter(2), rhs = Zero
+- expression 2 operands: lhs = Counter(2), rhs = Counter(3)
 Number of file 0 mappings: 5
 - Code(Counter(0)) at (prev + 16, 28) to (start + 3, 33)
 - Code(Counter(1)) at (prev + 4, 17) to (start + 1, 39)
 - Code(Expression(0, Sub)) at (prev + 3, 17) to (start + 0, 22)
     = (c0 - c1)
-- Code(Zero) at (prev + 0, 23) to (start + 0, 30)
+- Code(Counter(3)) at (prev + 0, 23) to (start + 0, 30)
 - Code(Expression(1, Add)) at (prev + 2, 9) to (start + 0, 10)
-    = (c1 + (c2 + Zero))
+    = (c1 + (c2 + c3))
 

tests/coverage/closure_macro_async.cov-map

@@ -30,19 +30,19 @@ Number of file 0 mappings: 5
 - Code(Counter(0)) at (prev + 3, 1) to (start + 0, 2)
 
 Function name: closure_macro_async::test::{closure#0}::{closure#0}
-Raw bytes (35): 0x[01, 01, 03, 01, 05, 05, 0b, 09, 00, 05, 01, 12, 1c, 03, 21, 05, 04, 11, 01, 27, 02, 03, 11, 00, 16, 00, 00, 17, 00, 1e, 07, 02, 09, 00, 0a]
+Raw bytes (35): 0x[01, 01, 03, 01, 05, 05, 0b, 09, 0d, 05, 01, 12, 1c, 03, 21, 05, 04, 11, 01, 27, 02, 03, 11, 00, 16, 0d, 00, 17, 00, 1e, 07, 02, 09, 00, 0a]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 3
 - expression 0 operands: lhs = Counter(0), rhs = Counter(1)
 - expression 1 operands: lhs = Counter(1), rhs = Expression(2, Add)
-- expression 2 operands: lhs = Counter(2), rhs = Zero
+- expression 2 operands: lhs = Counter(2), rhs = Counter(3)
 Number of file 0 mappings: 5
 - Code(Counter(0)) at (prev + 18, 28) to (start + 3, 33)
 - Code(Counter(1)) at (prev + 4, 17) to (start + 1, 39)
 - Code(Expression(0, Sub)) at (prev + 3, 17) to (start + 0, 22)
     = (c0 - c1)
-- Code(Zero) at (prev + 0, 23) to (start + 0, 30)
+- Code(Counter(3)) at (prev + 0, 23) to (start + 0, 30)
 - Code(Expression(1, Add)) at (prev + 2, 9) to (start + 0, 10)
-    = (c1 + (c2 + Zero))
+    = (c1 + (c2 + c3))
 

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;
+      }
+  }
+