Auto merge of rust-lang#115764 - RalfJung:const-by-ref-alloc-id, r=<try>

use AllocId instead of Allocation in ConstValue::ByRef

This helps avoid redundant AllocIds when a  `ByRef` constant gets put back into the interpreter.

r? `@oli-obk`

compiler/rustc_codegen_cranelift/src/constant.rs

@@ -76,7 +76,7 @@ pub(crate) fn codegen_tls_ref<'tcx>(
 pub(crate) fn eval_mir_constant<'tcx>(
     fx: &FunctionCx<'_, '_, 'tcx>,
     constant: &Constant<'tcx>,
-) -> Option<(ConstValue<'tcx>, Ty<'tcx>)> {
+) -> Option<(ConstValue, Ty<'tcx>)> {
     let constant_kind = fx.monomorphize(constant.literal);
     let uv = match constant_kind {
         ConstantKind::Ty(const_) => match const_.kind() {
@@ -127,7 +127,7 @@ pub(crate) fn codegen_constant_operand<'tcx>(
 
 pub(crate) fn codegen_const_value<'tcx>(
     fx: &mut FunctionCx<'_, '_, 'tcx>,
-    const_val: ConstValue<'tcx>,
+    const_val: ConstValue,
     ty: Ty<'tcx>,
 ) -> CValue<'tcx> {
     let layout = fx.layout_of(ty);
@@ -222,12 +222,18 @@ pub(crate) fn codegen_const_value<'tcx>(
                 CValue::by_val(val, layout)
             }
         },
-        ConstValue::ByRef { alloc, offset } => CValue::by_ref(
-            pointer_for_allocation(fx, alloc)
-                .offset_i64(fx, i64::try_from(offset.bytes()).unwrap()),
-            layout,
-        ),
-        ConstValue::Slice { data, start, end } => {
+        ConstValue::Indirect { alloc_id, offset } => {
+            let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory();
+            // FIXME: avoid creating multiple allocations for the same AllocId?
+            CValue::by_ref(
+                pointer_for_allocation(fx, alloc)
+                    .offset_i64(fx, i64::try_from(offset.bytes()).unwrap()),
+                layout,
+            )
+        }
+        ConstValue::Slice { alloc_id: Some(alloc_id), start, end } => {
+            let data = fx.tcx.global_alloc(alloc_id).unwrap_memory();
+            // FIXME: avoid creating multiple allocations for the same AllocId?
             let ptr = pointer_for_allocation(fx, data)
                 .offset_i64(fx, i64::try_from(start).unwrap())
                 .get_addr(fx);
@@ -237,14 +243,23 @@ pub(crate) fn codegen_const_value<'tcx>(
                 .iconst(fx.pointer_type, i64::try_from(end.checked_sub(start).unwrap()).unwrap());
             CValue::by_val_pair(ptr, len, layout)
         }
+        ConstValue::Slice { alloc_id: None, start, end } => {
+            assert_eq!(start, end);
+            // Empty slice at `start`.
+            // FIXME: `start` could in principle be big enough to overflow into the negative.
+            // How should the clif value be constructed then?
+            let addr = fx.bcx.ins().iconst(fx.pointer_type, i64::try_from(start).unwrap());
+            let len = fx.bcx.ins().iconst(fx.pointer_type, 0);
+            CValue::by_val_pair(addr, len, layout)
+        }
     }
 }
 
 fn pointer_for_allocation<'tcx>(
     fx: &mut FunctionCx<'_, '_, 'tcx>,
     alloc: ConstAllocation<'tcx>,
 ) -> crate::pointer::Pointer {
-    let alloc_id = fx.tcx.create_memory_alloc(alloc);
+    let alloc_id = fx.tcx.reserve_and_set_memory_alloc(alloc);
     let data_id = data_id_for_alloc_id(
         &mut fx.constants_cx,
         &mut *fx.module,
@@ -477,7 +492,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
 pub(crate) fn mir_operand_get_const_val<'tcx>(
     fx: &FunctionCx<'_, '_, 'tcx>,
     operand: &Operand<'tcx>,
-) -> Option<ConstValue<'tcx>> {
+) -> Option<ConstValue> {
     match operand {
         Operand::Constant(const_) => Some(eval_mir_constant(fx, const_).unwrap().0),
         // FIXME(rust-lang/rust#85105): Casts like `IMM8 as u32` result in the const being stored

compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs

@@ -172,7 +172,8 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
                     .expect("simd_shuffle idx not const");
 
                 let idx_bytes = match idx_const {
-                    ConstValue::ByRef { alloc, offset } => {
+                    ConstValue::Indirect { alloc_id, offset } => {
+                        let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory();
                         let size = Size::from_bytes(
                             4 * ret_lane_count, /* size_of([u32; ret_lane_count]) */
                         );

compiler/rustc_codegen_ssa/src/common.rs

@@ -194,7 +194,7 @@ pub fn shift_mask_val<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
 pub fn asm_const_to_str<'tcx>(
     tcx: TyCtxt<'tcx>,
     sp: Span,
-    const_value: ConstValue<'tcx>,
+    const_value: ConstValue,
     ty_and_layout: TyAndLayout<'tcx>,
 ) -> String {
     let ConstValue::Scalar(scalar) = const_value else {

compiler/rustc_codegen_ssa/src/mir/constant.rs

@@ -23,7 +23,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
     pub fn eval_mir_constant(
         &self,
         constant: &mir::Constant<'tcx>,
-    ) -> Result<ConstValue<'tcx>, ErrorHandled> {
+    ) -> Result<ConstValue, ErrorHandled> {
         let ct = self.monomorphize(constant.literal);
         let uv = match ct {
             mir::ConstantKind::Ty(ct) => match ct.kind() {

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -86,7 +86,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
 
     pub fn from_const<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
         bx: &mut Bx,
-        val: ConstValue<'tcx>,
+        val: ConstValue,
         ty: Ty<'tcx>,
     ) -> Self {
         let layout = bx.layout_of(ty);
@@ -100,12 +100,12 @@ 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 { alloc_id, start, end } => {
                 let Abi::ScalarPair(a_scalar, _) = layout.abi else {
                     bug!("from_const: invalid ScalarPair layout: {:#?}", layout);
                 };
-                let a = Scalar::from_pointer(
-                    Pointer::new(bx.tcx().create_memory_alloc(data), Size::from_bytes(start)),
+                let a = Scalar::from_maybe_pointer(
+                    Pointer::new(alloc_id, Size::from_bytes(start)),
                     &bx.tcx(),
                 );
                 let a_llval = bx.scalar_to_backend(
@@ -116,7 +116,9 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
                 let b_llval = bx.const_usize((end - start) as u64);
                 OperandValue::Pair(a_llval, b_llval)
             }
-            ConstValue::ByRef { alloc, offset } => {
+            ConstValue::Indirect { alloc_id, offset } => {
+                let alloc = bx.tcx().global_alloc(alloc_id).unwrap_memory();
+                // FIXME: should we attempt to avoid building the same AllocId multiple times?
                 return Self::from_const_alloc(bx, layout, alloc, offset);
             }
         };

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -18,9 +18,9 @@ use super::{CanAccessStatics, CompileTimeEvalContext, CompileTimeInterpreter};
 use crate::errors;
 use crate::interpret::eval_nullary_intrinsic;
 use crate::interpret::{
-    intern_const_alloc_recursive, Allocation, ConstAlloc, ConstValue, CtfeValidationMode, GlobalId,
-    Immediate, InternKind, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy,
-    RefTracking, StackPopCleanup,
+    intern_const_alloc_recursive, ConstAlloc, ConstValue, CtfeValidationMode, GlobalId, Immediate,
+    InternKind, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking,
+    StackPopCleanup,
 };
 
 // Returns a pointer to where the result lives
@@ -111,7 +111,7 @@ pub(super) fn mk_eval_cx<'mir, 'tcx>(
 pub(super) fn op_to_const<'tcx>(
     ecx: &CompileTimeEvalContext<'_, 'tcx>,
     op: &OpTy<'tcx>,
-) -> ConstValue<'tcx> {
+) -> ConstValue {
     // We do not have value optimizations for everything.
     // Only scalars and slices, since they are very common.
     // Note that further down we turn scalars of uninitialized bits back to `ByRef`. These can result
@@ -148,10 +148,7 @@ pub(super) fn op_to_const<'tcx>(
     let to_const_value = |mplace: &MPlaceTy<'_>| {
         debug!("to_const_value(mplace: {:?})", mplace);
         match mplace.ptr().into_parts() {
-            (Some(alloc_id), offset) => {
-                let alloc = ecx.tcx.global_alloc(alloc_id).unwrap_memory();
-                ConstValue::ByRef { alloc, offset }
-            }
+            (Some(alloc_id), offset) => ConstValue::Indirect { alloc_id, offset },
             (None, offset) => {
                 assert!(mplace.layout.is_zst());
                 assert_eq!(
@@ -173,21 +170,11 @@ pub(super) fn op_to_const<'tcx>(
             Immediate::ScalarPair(a, b) => {
                 debug!("ScalarPair(a: {:?}, b: {:?})", a, b);
                 // We know `offset` is relative to the allocation, so we can use `into_parts`.
-                let (data, start) = match a.to_pointer(ecx).unwrap().into_parts() {
-                    (Some(alloc_id), offset) => {
-                        (ecx.tcx.global_alloc(alloc_id).unwrap_memory(), offset.bytes())
-                    }
-                    (None, _offset) => (
-                        ecx.tcx.mk_const_alloc(Allocation::from_bytes_byte_aligned_immutable(
-                            b"" as &[u8],
-                        )),
-                        0,
-                    ),
-                };
+                let (alloc_id, start) = a.to_pointer(ecx).unwrap().into_parts();
                 let len = b.to_target_usize(ecx).unwrap();
-                let start = start.try_into().unwrap();
+                let start = start.bytes().try_into().unwrap();
                 let len: usize = len.try_into().unwrap();
-                ConstValue::Slice { data, start, end: start + len }
+                ConstValue::Slice { alloc_id, start, end: start + len }
             }
             Immediate::Uninit => to_const_value(&op.assert_mem_place()),
         },
@@ -199,7 +186,7 @@ pub(crate) fn turn_into_const_value<'tcx>(
     tcx: TyCtxt<'tcx>,
     constant: ConstAlloc<'tcx>,
     key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
-) -> ConstValue<'tcx> {
+) -> ConstValue {
     let cid = key.value;
     let def_id = cid.instance.def.def_id();
     let is_static = tcx.is_static(def_id);

compiler/rustc_const_eval/src/const_eval/mod.rs

@@ -22,7 +22,7 @@ pub(crate) use valtrees::{const_to_valtree_inner, valtree_to_const_value};
 pub(crate) fn const_caller_location(
     tcx: TyCtxt<'_>,
     (file, line, col): (Symbol, u32, u32),
-) -> ConstValue<'_> {
+) -> ConstValue {
     trace!("const_caller_location: {}:{}:{}", file, line, col);
     let mut ecx = mk_eval_cx(tcx, DUMMY_SP, ty::ParamEnv::reveal_all(), CanAccessStatics::No);
 
@@ -87,7 +87,7 @@ pub(crate) fn eval_to_valtree<'tcx>(
 #[instrument(skip(tcx), level = "debug")]
 pub(crate) fn try_destructure_mir_constant_for_diagnostics<'tcx>(
     tcx: TyCtxt<'tcx>,
-    val: ConstValue<'tcx>,
+    val: ConstValue,
     ty: Ty<'tcx>,
 ) -> Option<mir::DestructuredConstant<'tcx>> {
     let param_env = ty::ParamEnv::reveal_all();

compiler/rustc_const_eval/src/const_eval/valtrees.rs

@@ -207,7 +207,7 @@ pub fn valtree_to_const_value<'tcx>(
     tcx: TyCtxt<'tcx>,
     param_env_ty: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
     valtree: ty::ValTree<'tcx>,
-) -> ConstValue<'tcx> {
+) -> ConstValue {
     // Basic idea: We directly construct `Scalar` values from trivial `ValTree`s
     // (those for constants with type bool, int, uint, float or char).
     // For all other types we create an `MPlace` and fill that by walking

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -84,7 +84,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         )
                         .ok_or_else(|| err_inval!(TooGeneric))?;
 
-                        let fn_ptr = self.create_fn_alloc_ptr(FnVal::Instance(instance));
+                        let fn_ptr = self.fn_ptr(FnVal::Instance(instance));
                         self.write_pointer(fn_ptr, dest)?;
                     }
                     _ => span_bug!(self.cur_span(), "reify fn pointer on {:?}", src.layout.ty),
@@ -116,7 +116,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                             ty::ClosureKind::FnOnce,
                         )
                         .ok_or_else(|| err_inval!(TooGeneric))?;
-                        let fn_ptr = self.create_fn_alloc_ptr(FnVal::Instance(instance));
+                        let fn_ptr = self.fn_ptr(FnVal::Instance(instance));
                         self.write_pointer(fn_ptr, dest)?;
                     }
                     _ => span_bug!(self.cur_span(), "closure fn pointer on {:?}", src.layout.ty),

compiler/rustc_const_eval/src/interpret/intern.rs

@@ -24,8 +24,8 @@ use rustc_middle::ty::{self, layout::TyAndLayout, Ty};
 use rustc_ast::Mutability;
 
 use super::{
-    AllocId, Allocation, ConstAllocation, InterpCx, MPlaceTy, Machine, MemoryKind, PlaceTy,
-    Projectable, ValueVisitor,
+    AllocId, Allocation, InterpCx, MPlaceTy, Machine, MemoryKind, PlaceTy, Projectable,
+    ValueVisitor,
 };
 use crate::const_eval;
 use crate::errors::{DanglingPtrInFinal, UnsupportedUntypedPointer};
@@ -455,19 +455,23 @@ impl<'mir, 'tcx: 'mir, M: super::intern::CompileTimeMachine<'mir, 'tcx, !>>
 {
     /// A helper function that allocates memory for the layout given and gives you access to mutate
     /// it. Once your own mutation code is done, the backing `Allocation` is removed from the
-    /// current `Memory` and returned.
+    /// current `Memory` and interned as read-only into the global memory.
     pub fn intern_with_temp_alloc(
         &mut self,
         layout: TyAndLayout<'tcx>,
         f: impl FnOnce(
             &mut InterpCx<'mir, 'tcx, M>,
             &PlaceTy<'tcx, M::Provenance>,
         ) -> InterpResult<'tcx, ()>,
-    ) -> InterpResult<'tcx, ConstAllocation<'tcx>> {
+    ) -> InterpResult<'tcx, AllocId> {
+        // `allocate` picks a fresh AllocId that we will associate with its data below.
         let dest = self.allocate(layout, MemoryKind::Stack)?;
         f(self, &dest.clone().into())?;
         let mut alloc = self.memory.alloc_map.remove(&dest.ptr().provenance.unwrap()).unwrap().1;
         alloc.mutability = Mutability::Not;
-        Ok(self.tcx.mk_const_alloc(alloc))
+        let alloc = self.tcx.mk_const_alloc(alloc);
+        let alloc_id = dest.ptr().provenance.unwrap(); // this was just allocated, it must have provenance
+        self.tcx.set_alloc_id_memory(alloc_id, alloc);
+        Ok(alloc_id)
     }
 }

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -5,9 +5,7 @@
 use rustc_hir::def_id::DefId;
 use rustc_middle::mir::{
     self,
-    interpret::{
-        Allocation, ConstAllocation, ConstValue, GlobalId, InterpResult, PointerArithmetic, Scalar,
-    },
+    interpret::{Allocation, ConstValue, GlobalId, InterpResult, PointerArithmetic, Scalar},
     BinOp, NonDivergingIntrinsic,
 };
 use rustc_middle::ty;
@@ -43,28 +41,24 @@ fn numeric_intrinsic<Prov>(name: Symbol, bits: u128, kind: Primitive) -> Scalar<
     Scalar::from_uint(bits_out, size)
 }
 
-/// Directly returns an `Allocation` containing an absolute path representation of the given type.
-pub(crate) fn alloc_type_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> ConstAllocation<'tcx> {
-    let path = crate::util::type_name(tcx, ty);
-    let alloc = Allocation::from_bytes_byte_aligned_immutable(path.into_bytes());
-    tcx.mk_const_alloc(alloc)
-}
-
 /// The logic for all nullary intrinsics is implemented here. These intrinsics don't get evaluated
 /// inside an `InterpCx` and instead have their value computed directly from rustc internal info.
 pub(crate) fn eval_nullary_intrinsic<'tcx>(
     tcx: TyCtxt<'tcx>,
     param_env: ty::ParamEnv<'tcx>,
     def_id: DefId,
     args: GenericArgsRef<'tcx>,
-) -> InterpResult<'tcx, ConstValue<'tcx>> {
+) -> InterpResult<'tcx, ConstValue> {
     let tp_ty = args.type_at(0);
     let name = tcx.item_name(def_id);
     Ok(match name {
         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() }
+            let path = crate::util::type_name(tcx, tp_ty);
+            let alloc = Allocation::from_bytes_byte_aligned_immutable(path.into_bytes());
+            let alloc = tcx.mk_const_alloc(alloc);
+            let alloc_id = tcx.reserve_and_set_memory_alloc(alloc);
+            ConstValue::Slice { alloc_id: Some(alloc_id), start: 0, end: alloc.inner().len() }
         }
         sym::needs_drop => {
             ensure_monomorphic_enough(tcx, tp_ty)?;

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -552,7 +552,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
         def_id: DefId,
     ) -> InterpResult<$tcx, Pointer> {
         // Use the `AllocId` associated with the `DefId`. Any actual *access* will fail.
-        Ok(Pointer::new(ecx.tcx.create_static_alloc(def_id), Size::ZERO))
+        Ok(Pointer::new(ecx.tcx.reserve_and_set_static_alloc(def_id), Size::ZERO))
     }
 
     #[inline(always)]

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -176,12 +176,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         M::adjust_alloc_base_pointer(self, ptr)
     }
 
-    pub fn create_fn_alloc_ptr(
-        &mut self,
-        fn_val: FnVal<'tcx, M::ExtraFnVal>,
-    ) -> Pointer<M::Provenance> {
+    pub fn fn_ptr(&mut self, fn_val: FnVal<'tcx, M::ExtraFnVal>) -> Pointer<M::Provenance> {
         let id = match fn_val {
-            FnVal::Instance(instance) => self.tcx.create_fn_alloc(instance),
+            FnVal::Instance(instance) => self.tcx.reserve_and_set_fn_alloc(instance),
             FnVal::Other(extra) => {
                 // FIXME(RalfJung): Should we have a cache here?
                 let id = self.tcx.reserve_alloc_id();