Skip to content

interpret: clean up deduplicating allocation functions #134055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 17 additions & 27 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
use std::assert_matches::assert_matches;

use either::{Either, Left, Right};
use rustc_abi::{Align, BackendRepr, HasDataLayout, Size};
use rustc_ast::Mutability;
use rustc_abi::{BackendRepr, HasDataLayout, Size};
use rustc_middle::ty::Ty;
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
use rustc_middle::{bug, mir, span_bug};
Expand Down Expand Up @@ -1018,40 +1017,31 @@ where
self.allocate_dyn(layout, kind, MemPlaceMeta::None)
}

/// Allocates a sequence of bytes in the interpreter's memory.
/// For immutable allocations, uses deduplication to reuse existing memory.
/// For mutable allocations, creates a new unique allocation.
pub fn allocate_bytes(
/// Allocates a sequence of bytes in the interpreter's memory with alignment 1.
/// This is allocated in immutable global memory and deduplicated.
pub fn allocate_bytes_dedup(
&mut self,
bytes: &[u8],
align: Align,
kind: MemoryKind<M::MemoryKind>,
mutbl: Mutability,
) -> InterpResult<'tcx, Pointer<M::Provenance>> {
// Use cache for immutable strings.
if mutbl.is_not() {
// Use dedup'd allocation function.
let salt = M::get_global_alloc_salt(self, None);
let id = self.tcx.allocate_bytes_dedup(bytes, salt);

// Turn untagged "global" pointers (obtained via `tcx`) into the machine pointer to the allocation.
M::adjust_alloc_root_pointer(&self, Pointer::from(id), Some(kind))
} else {
// Allocate new memory for mutable data.
self.allocate_bytes_ptr(bytes, align, kind, mutbl)
}
let salt = M::get_global_alloc_salt(self, None);
let id = self.tcx.allocate_bytes_dedup(bytes, salt);

// Turn untagged "global" pointers (obtained via `tcx`) into the machine pointer to the allocation.
M::adjust_alloc_root_pointer(
&self,
Pointer::from(id),
M::GLOBAL_KIND.map(MemoryKind::Machine),
)
}

/// Allocates a string in the interpreter's memory with metadata for length.
/// Uses `allocate_bytes` internally but adds string-specific metadata handling.
pub fn allocate_str(
/// Allocates a string in the interpreter's memory, returning it as a (wide) place.
/// This is allocated in immutable global memory and deduplicated.
pub fn allocate_str_dedup(
&mut self,
str: &str,
kind: MemoryKind<M::MemoryKind>,
mutbl: Mutability,
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
let bytes = str.as_bytes();
let ptr = self.allocate_bytes(bytes, Align::ONE, kind, mutbl)?;
let ptr = self.allocate_bytes_dedup(bytes)?;

// Create length metadata for the string.
let meta = Scalar::from_target_usize(u64::try_from(bytes.len()).unwrap(), self);
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_const_eval/src/util/caller_location.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rustc_hir::LangItem;
use rustc_middle::query::TyCtxtAt;
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::{self, Mutability};
use rustc_middle::ty::{self};
use rustc_middle::{bug, mir};
use rustc_span::symbol::Symbol;
use tracing::trace;
Expand All @@ -20,12 +20,9 @@ fn alloc_caller_location<'tcx>(
// This can fail if rustc runs out of memory right here. Trying to emit an error would be
// pointless, since that would require allocating more memory than these short strings.
let file = if loc_details.file {
ecx.allocate_str(filename.as_str(), MemoryKind::CallerLocation, Mutability::Not).unwrap()
ecx.allocate_str_dedup(filename.as_str()).unwrap()
} else {
// FIXME: This creates a new allocation each time. It might be preferable to
// perform this allocation only once, and re-use the `MPlaceTy`.
// See https://github.com/rust-lang/rust/pull/89920#discussion_r730012398
ecx.allocate_str("<redacted>", MemoryKind::CallerLocation, Mutability::Not).unwrap()
ecx.allocate_str_dedup("<redacted>").unwrap()
};
let file = file.map_provenance(CtfeProvenance::as_immutable);
let line = if loc_details.line { Scalar::from_u32(line) } else { Scalar::from_u32(0) };
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1479,7 +1479,7 @@ impl<'tcx> TyCtxt<'tcx> {
self.mk_adt_def_from_data(ty::AdtDefData::new(self, did, kind, variants, repr))
}

/// Allocates a read-only byte or string literal for `mir::interpret`.
/// Allocates a read-only byte or string literal for `mir::interpret` with alignment 1.
/// Returns the same `AllocId` if called again with the same bytes.
pub fn allocate_bytes_dedup(self, bytes: &[u8], salt: usize) -> interpret::AllocId {
// Create an allocation that just contains these bytes.
Expand Down
10 changes: 1 addition & 9 deletions src/tools/miri/src/shims/backtrace.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use rustc_abi::{ExternAbi, Size};
use rustc_ast::ast::Mutability;
use rustc_middle::ty::layout::LayoutOf as _;
use rustc_middle::ty::{self, Instance, Ty};
use rustc_span::{BytePos, Loc, Symbol, hygiene};
Expand Down Expand Up @@ -179,14 +178,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

match flags {
0 => {
// These are "mutable" allocations as we consider them to be owned by the callee.
let name_alloc =
this.allocate_str(&name, MiriMemoryKind::Rust.into(), Mutability::Mut)?;
let filename_alloc =
this.allocate_str(&filename, MiriMemoryKind::Rust.into(), Mutability::Mut)?;

this.write_immediate(name_alloc.to_ref(this), &this.project_field(dest, 0)?)?;
this.write_immediate(filename_alloc.to_ref(this), &this.project_field(dest, 1)?)?;
throw_unsup_format!("miri_resolve_frame: v0 is not supported any more");
}
1 => {
this.write_scalar(
Expand Down
5 changes: 2 additions & 3 deletions src/tools/miri/src/shims/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
//! metadata we remembered when pushing said frame.

use rustc_abi::ExternAbi;
use rustc_ast::Mutability;
use rustc_middle::{mir, ty};
use rustc_target::spec::PanicStrategy;

Expand Down Expand Up @@ -161,7 +160,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let this = self.eval_context_mut();

// First arg: message.
let msg = this.allocate_str(msg, MiriMemoryKind::Machine.into(), Mutability::Not)?;
let msg = this.allocate_str_dedup(msg)?;

// Call the lang item.
let panic = this.tcx.lang_items().panic_fn().unwrap();
Expand All @@ -180,7 +179,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let this = self.eval_context_mut();

// First arg: message.
let msg = this.allocate_str(msg, MiriMemoryKind::Machine.into(), Mutability::Not)?;
let msg = this.allocate_str_dedup(msg)?;

// Call the lang item.
let panic = this.tcx.lang_items().panic_nounwind().unwrap();
Expand Down
69 changes: 0 additions & 69 deletions src/tools/miri/tests/pass/backtrace/backtrace-api-v0.rs

This file was deleted.

18 changes: 0 additions & 18 deletions src/tools/miri/tests/pass/backtrace/backtrace-api-v0.stderr

This file was deleted.

5 changes: 0 additions & 5 deletions src/tools/miri/tests/pass/backtrace/backtrace-api-v0.stdout

This file was deleted.

Loading