Skip to content

Commit 5c84886

Browse files
committed
Auto merge of #127638 - adwinwhite:cache_string, r=oli-obk
Add cache for `allocate_str` Best effort cache for string allocation in const eval. Fixes [rust-lang/miri#3470](rust-lang/miri#3470).
2 parents cae4a84 + e595f3d commit 5c84886

File tree

4 files changed

+28
-10
lines changed

4 files changed

+28
-10
lines changed

Diff for: compiler/rustc_const_eval/src/interpret/place.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -995,13 +995,25 @@ where
995995
}
996996

997997
/// Returns a wide MPlace of type `str` to a new 1-aligned allocation.
998+
/// Immutable strings are deduplicated and stored in global memory.
998999
pub fn allocate_str(
9991000
&mut self,
10001001
str: &str,
10011002
kind: MemoryKind<M::MemoryKind>,
10021003
mutbl: Mutability,
10031004
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
1004-
let ptr = self.allocate_bytes_ptr(str.as_bytes(), Align::ONE, kind, mutbl)?;
1005+
let tcx = self.tcx.tcx;
1006+
1007+
// Use cache for immutable strings.
1008+
let ptr = if mutbl.is_not() {
1009+
// Use dedup'd allocation function.
1010+
let id = tcx.allocate_bytes_dedup(str.as_bytes());
1011+
1012+
// Turn untagged "global" pointers (obtained via `tcx`) into the machine pointer to the allocation.
1013+
M::adjust_alloc_root_pointer(&self, Pointer::from(id), Some(kind))?
1014+
} else {
1015+
self.allocate_bytes_ptr(str.as_bytes(), Align::ONE, kind, mutbl)?
1016+
};
10051017
let meta = Scalar::from_target_usize(u64::try_from(str.len()).unwrap(), self);
10061018
let layout = self.layout_of(self.tcx.types.str_).unwrap();
10071019
Ok(self.ptr_with_meta_to_mplace(ptr.into(), MemPlaceMeta::Meta(meta), layout))

Diff for: compiler/rustc_middle/src/mir/interpret/mod.rs

+11-6
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,6 @@ pub(crate) struct AllocMap<'tcx> {
393393
alloc_map: FxHashMap<AllocId, GlobalAlloc<'tcx>>,
394394

395395
/// Used to ensure that statics and functions only get one associated `AllocId`.
396-
/// Should never contain a `GlobalAlloc::Memory`!
397396
//
398397
// FIXME: Should we just have two separate dedup maps for statics and functions each?
399398
dedup: FxHashMap<GlobalAlloc<'tcx>, AllocId>,
@@ -433,13 +432,13 @@ impl<'tcx> TyCtxt<'tcx> {
433432
}
434433

435434
/// Reserves a new ID *if* this allocation has not been dedup-reserved before.
436-
/// Should only be used for "symbolic" allocations (function pointers, vtables, statics), we
437-
/// don't want to dedup IDs for "real" memory!
435+
/// Should not be used for mutable memory.
438436
fn reserve_and_set_dedup(self, alloc: GlobalAlloc<'tcx>) -> AllocId {
439437
let mut alloc_map = self.alloc_map.lock();
440-
match alloc {
441-
GlobalAlloc::Function { .. } | GlobalAlloc::Static(..) | GlobalAlloc::VTable(..) => {}
442-
GlobalAlloc::Memory(..) => bug!("Trying to dedup-reserve memory with real data!"),
438+
if let GlobalAlloc::Memory(mem) = alloc {
439+
if mem.inner().mutability.is_mut() {
440+
bug!("trying to dedup-reserve mutable memory");
441+
}
443442
}
444443
if let Some(&alloc_id) = alloc_map.dedup.get(&alloc) {
445444
return alloc_id;
@@ -451,6 +450,12 @@ impl<'tcx> TyCtxt<'tcx> {
451450
id
452451
}
453452

453+
/// Generates an `AllocId` for a memory allocation. If the exact same memory has been
454+
/// allocated before, this will return the same `AllocId`.
455+
pub fn reserve_and_set_memory_dedup(self, mem: ConstAllocation<'tcx>) -> AllocId {
456+
self.reserve_and_set_dedup(GlobalAlloc::Memory(mem))
457+
}
458+
454459
/// Generates an `AllocId` for a static or return a cached one in case this function has been
455460
/// called on the same static before.
456461
pub fn reserve_and_set_static_alloc(self, static_id: DefId) -> AllocId {

Diff for: compiler/rustc_middle/src/ty/context.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -1442,11 +1442,12 @@ impl<'tcx> TyCtxt<'tcx> {
14421442
}
14431443

14441444
/// Allocates a read-only byte or string literal for `mir::interpret`.
1445-
pub fn allocate_bytes(self, bytes: &[u8]) -> interpret::AllocId {
1445+
/// Returns the same `AllocId` if called again with the same bytes.
1446+
pub fn allocate_bytes_dedup(self, bytes: &[u8]) -> interpret::AllocId {
14461447
// Create an allocation that just contains these bytes.
14471448
let alloc = interpret::Allocation::from_bytes_byte_aligned_immutable(bytes);
14481449
let alloc = self.mk_const_alloc(alloc);
1449-
self.reserve_and_set_memory_alloc(alloc)
1450+
self.reserve_and_set_memory_dedup(alloc)
14501451
}
14511452

14521453
/// Returns a range of the start/end indices specified with the

Diff for: compiler/rustc_mir_build/src/build/expr/as_constant.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ fn lit_to_mir_constant<'tcx>(
140140
ConstValue::Slice { data: allocation, meta: allocation.inner().size().bytes() }
141141
}
142142
(ast::LitKind::ByteStr(data, _), ty::Ref(_, inner_ty, _)) if inner_ty.is_array() => {
143-
let id = tcx.allocate_bytes(data);
143+
let id = tcx.allocate_bytes_dedup(data);
144144
ConstValue::Scalar(Scalar::from_pointer(id.into(), &tcx))
145145
}
146146
(ast::LitKind::CStr(data, _), ty::Ref(_, inner_ty, _)) if matches!(inner_ty.kind(), ty::Adt(def, _) if tcx.is_lang_item(def.did(), LangItem::CStr)) =>

0 commit comments

Comments
 (0)