Skip to content

Commit

Permalink
interpret: clean up deduplicating allocation functions
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Dec 9, 2024
1 parent f33a8c6 commit 73dc95d
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 138 deletions.
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.

0 comments on commit 73dc95d

Please sign in to comment.