Auto merge of rust-lang#115870 - RalfJung:const-value-slice, r=<try>

adjust constValue::Slice to work for arbitrary slice types

valtrees have already been assuming that this works; this PR makes it a reality. Also further restrict `ConstValue::Slice` to what it is actually used for; this even shrinks `ConstValue` from 32 to 24 bytes which is a nice win. :)

The alternative to this approach is to make `ConstValue::Slice` work really only for `&str`/`&[u8]` literals, and never return it in `op_to_const`. That would make `op_to_const` very clean. We could then even remove the `meta` field; the length would always be `data.inner().len()`. We could *almost* just use a `Symbol` instead of a `ConstAllocation`, but we have to support byte strings and there doesn't seem to be an interned representation of them (or rather, `ConstAllocation` *is* their interned representation). In this world, valtrees of slice reference types would then become noticeably more expensive to turn into a `ConstValue` -- but does that matter? Specifically for `&str`/`&[u8]` we could still use the optimized representation if we wanted.

If byte strings were already interned somewhere I'd gravitate towards the alternative, but the way things stand, we need a `ConstAllocation` case anyway to support byte strings, and then we might as well support arbitrary slices. (Or we say that byte strings don't get an optimized representation at all. Such a performance cliff between `str` and byte strings is probably unexpected, though due to the lack of interning for byte strings I think there might already be a performance cliff there.)

Author: bors

compiler/rustc_codegen_cranelift/src/constant.rs

@@ -205,15 +205,11 @@ pub(crate) fn codegen_const_value<'tcx>(
                 .offset_i64(fx, i64::try_from(offset.bytes()).unwrap()),
             layout,
         ),
-        ConstValue::Slice { data, start, end } => {
+        ConstValue::Slice { data, meta } => {
             let alloc_id = fx.tcx.reserve_and_set_memory_alloc(data);
-            let ptr = pointer_for_allocation(fx, alloc_id)
-                .offset_i64(fx, i64::try_from(start).unwrap())
-                .get_addr(fx);
-            let len = fx
-                .bcx
-                .ins()
-                .iconst(fx.pointer_type, i64::try_from(end.checked_sub(start).unwrap()).unwrap());
+            let ptr = pointer_for_allocation(fx, alloc_id).get_addr(fx);
+            // FIXME: the `try_from` here can actually fail, e.g. for very long ZST slices.
+            let len = fx.bcx.ins().iconst(fx.pointer_type, i64::try_from(meta).unwrap());
             CValue::by_val_pair(ptr, len, layout)
         }
     }

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -100,23 +100,20 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
                 OperandValue::Immediate(llval)
             }
             ConstValue::ZeroSized => return OperandRef::zero_sized(layout),
-            ConstValue::Slice { data, start, end } => {
+            ConstValue::Slice { data, meta } => {
                 let Abi::ScalarPair(a_scalar, _) = layout.abi else {
                     bug!("from_const: invalid ScalarPair layout: {:#?}", layout);
                 };
                 let a = Scalar::from_pointer(
-                    Pointer::new(
-                        bx.tcx().reserve_and_set_memory_alloc(data),
-                        Size::from_bytes(start),
-                    ),
+                    Pointer::new(bx.tcx().reserve_and_set_memory_alloc(data), Size::ZERO),
                     &bx.tcx(),
                 );
                 let a_llval = bx.scalar_to_backend(
                     a,
                     a_scalar,
                     bx.scalar_pair_element_backend_type(layout, 0, true),
                 );
-                let b_llval = bx.const_usize((end - start) as u64);
+                let b_llval = bx.const_usize(meta);
                 OperandValue::Pair(a_llval, b_llval)
             }
             ConstValue::Indirect { alloc_id, offset } => {

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -152,19 +152,28 @@ pub(super) fn op_to_const<'tcx>(
             Immediate::Scalar(x) => ConstValue::Scalar(x),
             Immediate::ScalarPair(a, b) => {
                 debug!("ScalarPair(a: {:?}, b: {:?})", a, b);
-                // FIXME: assert that this has an appropriate type.
-                // Currently we actually get here for non-[u8] slices during valtree construction!
-                let msg = "`op_to_const` on an immediate scalar pair must only be used on slice references to actually allocated memory";
+                // This codepath solely exists for `valtree_to_const_value` to not need to generate
+                // a `ConstValue::Indirect` for wide references, so it is tightly restricted to just
+                // that case.
+                let pointee_ty = imm.layout.ty.builtin_deref(false).unwrap().ty; // `false` = no raw ptrs
+                debug_assert!(
+                    matches!(
+                        ecx.tcx.struct_tail_without_normalization(pointee_ty).kind(),
+                        ty::Str | ty::Slice(..),
+                    ),
+                    "`ConstValue::Slice` is for slice-tailed types only, but got {}",
+                    imm.layout.ty,
+                );
+                let msg = "`op_to_const` on an immediate scalar pair must only be used on slice references to the beginning of an actual allocation";
                 // We know `offset` is relative to the allocation, so we can use `into_parts`.
-                // We use `ConstValue::Slice` so that we don't have to generate an allocation for
-                // `ConstValue::Indirect` here.
                 let (alloc_id, offset) = a.to_pointer(ecx).expect(msg).into_parts();
                 let alloc_id = alloc_id.expect(msg);
                 let data = ecx.tcx.global_alloc(alloc_id).unwrap_memory();
-                let start = offset.bytes_usize();
-                let len = b.to_target_usize(ecx).expect(msg);
-                let len: usize = len.try_into().unwrap();
-                ConstValue::Slice { data, start, end: start + len }
+                if offset.bytes() != 0 {
+                    panic!("{}", msg);
+                }
+                let meta = b.to_target_usize(ecx).expect(msg);
+                ConstValue::Slice { data, meta }
             }
             Immediate::Uninit => bug!("`Uninit` is not a valid value for {}", op.layout.ty),
         },

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -351,7 +351,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
         match (&src_pointee_ty.kind(), &dest_pointee_ty.kind()) {
             (&ty::Array(_, length), &ty::Slice(_)) => {
-                let ptr = self.read_scalar(src)?;
+                let ptr = self.read_pointer(src)?;
                 // u64 cast is from usize to u64, which is always good
                 let val = Immediate::new_slice(
                     ptr,
@@ -367,6 +367,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     return self.write_immediate(*val, dest);
                 }
                 let (old_data, old_vptr) = val.to_scalar_pair();
+                let old_data = old_data.to_pointer(self)?;
                 let old_vptr = old_vptr.to_pointer(self)?;
                 let (ty, old_trait) = self.get_ptr_vtable(old_vptr)?;
                 if old_trait != data_a.principal() {
@@ -378,7 +379,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             (_, &ty::Dynamic(data, _, ty::Dyn)) => {
                 // Initial cast from sized to dyn trait
                 let vtable = self.get_vtable_ptr(src_pointee_ty, data.principal())?;
-                let ptr = self.read_scalar(src)?;
+                let ptr = self.read_pointer(src)?;
                 let val = Immediate::new_dyn_trait(ptr, vtable, &*self.tcx);
                 self.write_immediate(val, dest)
             }

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -64,7 +64,7 @@ pub(crate) fn eval_nullary_intrinsic<'tcx>(
         sym::type_name => {
             ensure_monomorphic_enough(tcx, tp_ty)?;
             let alloc = alloc_type_name(tcx, tp_ty);
-            ConstValue::Slice { data: alloc, start: 0, end: alloc.inner().len() }
+            ConstValue::Slice { data: alloc, meta: alloc.inner().size().bytes() }
         }
         sym::needs_drop => {
             ensure_monomorphic_enough(tcx, tp_ty)?;

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -45,24 +45,30 @@ impl<Prov: Provenance> From<Scalar<Prov>> for Immediate<Prov> {
 }
 
 impl<Prov: Provenance> Immediate<Prov> {
-    pub fn from_pointer(p: Pointer<Prov>, cx: &impl HasDataLayout) -> Self {
-        Immediate::Scalar(Scalar::from_pointer(p, cx))
+    pub fn from_pointer(ptr: Pointer<Prov>, cx: &impl HasDataLayout) -> Self {
+        Immediate::Scalar(Scalar::from_pointer(ptr, cx))
     }
 
-    pub fn from_maybe_pointer(p: Pointer<Option<Prov>>, cx: &impl HasDataLayout) -> Self {
-        Immediate::Scalar(Scalar::from_maybe_pointer(p, cx))
+    pub fn from_maybe_pointer(ptr: Pointer<Option<Prov>>, cx: &impl HasDataLayout) -> Self {
+        Immediate::Scalar(Scalar::from_maybe_pointer(ptr, cx))
     }
 
-    pub fn new_slice(val: Scalar<Prov>, len: u64, cx: &impl HasDataLayout) -> Self {
-        Immediate::ScalarPair(val, Scalar::from_target_usize(len, cx))
+    pub fn new_slice(ptr: Pointer<Option<Prov>>, len: u64, cx: &impl HasDataLayout) -> Self {
+        Immediate::ScalarPair(
+            Scalar::from_maybe_pointer(ptr, cx),
+            Scalar::from_target_usize(len, cx),
+        )
     }
 
     pub fn new_dyn_trait(
-        val: Scalar<Prov>,
+        val: Pointer<Option<Prov>>,
         vtable: Pointer<Option<Prov>>,
         cx: &impl HasDataLayout,
     ) -> Self {
-        Immediate::ScalarPair(val, Scalar::from_maybe_pointer(vtable, cx))
+        Immediate::ScalarPair(
+            Scalar::from_maybe_pointer(val, cx),
+            Scalar::from_maybe_pointer(vtable, cx),
+        )
     }
 
     #[inline]
@@ -764,16 +770,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
             ConstValue::Scalar(x) => Operand::Immediate(adjust_scalar(x)?.into()),
             ConstValue::ZeroSized => Operand::Immediate(Immediate::Uninit),
-            ConstValue::Slice { data, start, end } => {
+            ConstValue::Slice { data, meta } => {
                 // We rely on mutability being set correctly in `data` to prevent writes
                 // where none should happen.
-                let ptr = Pointer::new(
-                    self.tcx.reserve_and_set_memory_alloc(data),
-                    Size::from_bytes(start), // offset: `start`
-                );
+                let ptr = Pointer::new(self.tcx.reserve_and_set_memory_alloc(data), Size::ZERO);
                 Operand::Immediate(Immediate::new_slice(
-                    Scalar::from_pointer(self.global_base_pointer(ptr)?, &*self.tcx),
-                    u64::try_from(end.checked_sub(start).unwrap()).unwrap(), // len: `end - start`
+                    self.global_base_pointer(ptr)?.into(),
+                    meta,
                     self,
                 ))
             }

compiler/rustc_middle/src/mir/interpret/value.rs

@@ -41,12 +41,20 @@ pub enum ConstValue<'tcx> {
     /// Only for ZSTs.
     ZeroSized,
 
-    /// Used for `&[u8]` and `&str`.
+    /// Used for references to unsized types with slice tail.
     ///
-    /// This is worth an optimized representation since Rust has literals of these types.
-    /// Not having to indirect those through an `AllocId` (or two, if we used `Indirect`) has shown
-    /// measurable performance improvements on stress tests.
-    Slice { data: ConstAllocation<'tcx>, start: usize, end: usize },
+    /// This is worth an optimized representation since Rust has literals of type `&str` and
+    /// `&[u8]`. Not having to indirect those through an `AllocId` (or two, if we used `Indirect`)
+    /// has shown measurable performance improvements on stress tests. We then reuse this
+    /// optimization for slice-tail types more generally during valtree-to-constval conversion.
+    Slice {
+        /// The allocation storing the slice contents.
+        /// This always points to the beginning of the allocation.
+        data: ConstAllocation<'tcx>,
+        /// The metadata field of the reference.
+        /// This is a "target usize", so we use `u64` as in the interpreter.
+        meta: u64,
+    },
 
     /// A value not representable by the other variants; needs to be stored in-memory.
     ///
@@ -65,7 +73,7 @@ pub enum ConstValue<'tcx> {
 }
 
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
-static_assert_size!(ConstValue<'_>, 32);
+static_assert_size!(ConstValue<'_>, 24);
 
 impl<'tcx> ConstValue<'tcx> {
     #[inline]
@@ -124,7 +132,7 @@ impl<'tcx> ConstValue<'tcx> {
             ConstValue::Scalar(_) | ConstValue::ZeroSized => {
                 bug!("`try_get_slice_bytes` on non-slice constant")
             }
-            &ConstValue::Slice { data, start, end } => (data, start, end),
+            &ConstValue::Slice { data, meta } => (data, 0, meta),
             &ConstValue::Indirect { alloc_id, offset } => {
                 // The reference itself is stored behind an indirection.
                 // Load the reference, and then load the actual slice contents.
@@ -151,18 +159,19 @@ impl<'tcx> ConstValue<'tcx> {
                     )
                     .ok()?;
                 let len = len.to_target_usize(&tcx).ok()?;
-                let len: usize = len.try_into().ok()?;
                 if len == 0 {
                     return Some(&[]);
                 }
                 // Non-empty slice, must have memory. We know this is a relative pointer.
                 let (inner_alloc_id, offset) = ptr.into_parts();
                 let data = tcx.global_alloc(inner_alloc_id?).unwrap_memory();
-                (data, offset.bytes_usize(), offset.bytes_usize() + len)
+                (data, offset.bytes(), offset.bytes() + len)
             }
         };
 
         // This is for diagnostics only, so we are okay to use `inspect_with_uninit_and_ptr_outside_interpreter`.
+        let start = start.try_into().unwrap();
+        let end = end.try_into().unwrap();
         Some(data.inner().inspect_with_uninit_and_ptr_outside_interpreter(start..end))
     }
 }

compiler/rustc_mir_build/src/build/expr/as_constant.rs

@@ -133,14 +133,14 @@ fn lit_to_mir_constant<'tcx>(
             let s = s.as_str();
             let allocation = Allocation::from_bytes_byte_aligned_immutable(s.as_bytes());
             let allocation = tcx.mk_const_alloc(allocation);
-            ConstValue::Slice { data: allocation, start: 0, end: s.len() }
+            ConstValue::Slice { data: allocation, meta: allocation.inner().size().bytes() }
         }
         (ast::LitKind::ByteStr(data, _), ty::Ref(_, inner_ty, _))
             if matches!(inner_ty.kind(), ty::Slice(_)) =>
         {
             let allocation = Allocation::from_bytes_byte_aligned_immutable(data as &[u8]);
             let allocation = tcx.mk_const_alloc(allocation);
-            ConstValue::Slice { data: allocation, start: 0, end: data.len() }
+            ConstValue::Slice { data: allocation, meta: allocation.inner().size().bytes() }
         }
         (ast::LitKind::ByteStr(data, _), ty::Ref(_, inner_ty, _)) if inner_ty.is_array() => {
             let id = tcx.allocate_bytes(data);
@@ -150,7 +150,7 @@ fn lit_to_mir_constant<'tcx>(
         {
             let allocation = Allocation::from_bytes_byte_aligned_immutable(data as &[u8]);
             let allocation = tcx.mk_const_alloc(allocation);
-            ConstValue::Slice { data: allocation, start: 0, end: data.len() }
+            ConstValue::Slice { data: allocation, meta: allocation.inner().size().bytes() }
         }
         (ast::LitKind::Byte(n), ty::Uint(ty::UintTy::U8)) => {
             ConstValue::Scalar(Scalar::from_uint(*n, Size::from_bytes(1)))

compiler/rustc_monomorphize/src/collector.rs

@@ -1448,7 +1448,7 @@ fn collect_const_value<'tcx>(
     match value {
         ConstValue::Scalar(Scalar::Ptr(ptr, _size)) => collect_alloc(tcx, ptr.provenance, output),
         ConstValue::Indirect { alloc_id, .. } => collect_alloc(tcx, alloc_id, output),
-        ConstValue::Slice { data, start: _, end: _ } => {
+        ConstValue::Slice { data, meta: _ } => {
             for &id in data.inner().provenance().ptrs().values() {
                 collect_alloc(tcx, id, output);
             }

compiler/rustc_smir/src/rustc_smir/alloc.rs

@@ -44,14 +44,12 @@ pub fn new_allocation<'tcx>(
                 tables.tcx.layout_of(rustc_middle::ty::ParamEnv::empty().and(ty)).unwrap().align;
             new_empty_allocation(align.abi)
         }
-        ConstValue::Slice { data, start, end } => {
+        ConstValue::Slice { data, meta } => {
             let alloc_id = tables.tcx.reserve_and_set_memory_alloc(data);
-            let ptr = Pointer::new(alloc_id, rustc_target::abi::Size::from_bytes(start));
+            let ptr = Pointer::new(alloc_id, rustc_target::abi::Size::ZERO);
             let scalar_ptr = rustc_middle::mir::interpret::Scalar::from_pointer(ptr, &tables.tcx);
-            let scalar_len = rustc_middle::mir::interpret::Scalar::from_target_usize(
-                (end - start) as u64,
-                &tables.tcx,
-            );
+            let scalar_meta =
+                rustc_middle::mir::interpret::Scalar::from_target_usize(meta, &tables.tcx);
             let layout =
                 tables.tcx.layout_of(rustc_middle::ty::ParamEnv::reveal_all().and(ty)).unwrap();
             let mut allocation =
@@ -66,8 +64,8 @@ pub fn new_allocation<'tcx>(
             allocation
                 .write_scalar(
                     &tables.tcx,
-                    alloc_range(tables.tcx.data_layout.pointer_size, scalar_len.size()),
-                    scalar_len,
+                    alloc_range(tables.tcx.data_layout.pointer_size, scalar_meta.size()),
+                    scalar_meta,
                 )
                 .unwrap();
             allocation.stable(tables)

src/tools/miri/src/shims/backtrace.rs

@@ -89,7 +89,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 }
 
                 this.write_immediate(
-                    Immediate::new_slice(Scalar::from_maybe_pointer(alloc.ptr(), this), len, this),
+                    Immediate::new_slice(alloc.ptr(), len, this),
                     dest,
                 )?;
             }