Skip to content

Commit

Permalink
Rollup merge of rust-lang#134055 - RalfJung:interpret-alloc-dedup, r=…
Browse files Browse the repository at this point in the history
…oli-obk

interpret: clean up deduplicating allocation functions

The "align" and "kind" arguments would be largely ignored in the "dedup" case, so let's move that to entirely separate function.

Let's also remove support for old-style miri_resolve_frame while we are at it. The docs have already said for a while that this must be set to 1.
  • Loading branch information
fmease authored Dec 9, 2024
2 parents 76fbe8d + 73dc95d commit e0bec9d
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 e0bec9d

Please sign in to comment.