Skip to content

interpret: make Place always refer to the current frame [perf experiment] #113964

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

interpret: make Place always refer to the current frame [perf experiment] #113964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
if Some(def_id) == self.tcx.lang_items().panic_display()
|| Some(def_id) == self.tcx.lang_items().begin_panic_fn()
{
let args = self.copy_fn_args(args)?;
let args = self.copy_fn_args(args);
// &str or &&str
assert!(args.len() == 1);

Expand All @@ -237,7 +237,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {

return Ok(Some(new_instance));
} else if Some(def_id) == self.tcx.lang_items().align_offset_fn() {
let args = self.copy_fn_args(args)?;
let args = self.copy_fn_args(args);
// For align_offset, we replace the function call if the pointer has no address.
match self.align_offset(instance, &args, dest, ret)? {
ControlFlow::Continue(()) => return Ok(Some(instance)),
Expand Down
20 changes: 8 additions & 12 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ use rustc_target::abi::{call::FnAbi, Align, HasDataLayout, Size, TargetDataLayou

use super::{
AllocId, GlobalId, Immediate, InterpErrorInfo, InterpResult, MPlaceTy, Machine, MemPlace,
MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, PointerArithmetic, Provenance,
Scalar, StackPopJump,
MemPlaceMeta, Memory, MemoryKind, Operand, Place, PointerArithmetic, Provenance, Scalar,
StackPopJump,
};
use crate::errors::{self, ErroneousConstUsed};
use crate::fluent_generated as fluent;
Expand Down Expand Up @@ -105,7 +105,7 @@ pub struct Frame<'mir, 'tcx, Prov: Provenance = AllocId, Extra = ()> {

/// The location where the result of the current stack frame should be written to,
/// and its layout in the caller.
pub return_place: PlaceTy<'tcx, Prov>,
pub return_place: MPlaceTy<'tcx, Prov>,

/// The list of locals for this stack frame, stored in order as
/// `[return_ptr, arguments..., variables..., temporaries...]`.
Expand Down Expand Up @@ -680,7 +680,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
&mut self,
instance: ty::Instance<'tcx>,
body: &'mir mir::Body<'tcx>,
return_place: &PlaceTy<'tcx, M::Provenance>,
return_place: &MPlaceTy<'tcx, M::Provenance>,
return_to_block: StackPopCleanup,
) -> InterpResult<'tcx> {
trace!("body: {:#?}", body);
Expand Down Expand Up @@ -810,7 +810,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let op = self
.local_to_op(self.frame(), mir::RETURN_PLACE, None)
.expect("return place should always be live");
let dest = self.frame().return_place.clone();
let dest = self.frame().return_place.into();
let err = self.copy_op(&op, &dest, /*allow_transmute*/ true);
trace!("return value: {:?}", self.dump_place(*dest));
// We delay actually short-circuiting on this error until *after* the stack frame is
Expand Down Expand Up @@ -1014,15 +1014,11 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> std::fmt::Debug
{
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self.place {
Place::Local { frame, local } => {
Place::Local { local } => {
let mut allocs = Vec::new();
write!(fmt, "{:?}", local)?;
if frame != self.ecx.frame_idx() {
write!(fmt, " ({} frames up)", self.ecx.frame_idx() - frame)?;
}
write!(fmt, ":")?;
write!(fmt, "{:?}:", local)?;

match self.ecx.stack()[frame].locals[local].value {
match self.ecx.frame().locals[local].value {
LocalValue::Dead => write!(fmt, " is dead")?,
LocalValue::Live(Operand::Immediate(Immediate::Uninit)) => {
write!(fmt, " is uninitialized")?
Expand Down
14 changes: 5 additions & 9 deletions compiler/rustc_const_eval/src/interpret/machine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::const_eval::CheckAlignment;

use super::{
AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
InterpResult, MemoryKind, OpTy, Operand, PlaceTy, Pointer, Provenance, Scalar,
InterpResult, MPlaceTy, MemoryKind, OpTy, Operand, PlaceTy, Pointer, Provenance, Scalar,
};

/// Data returned by Machine::stack_pop,
Expand Down Expand Up @@ -233,22 +233,18 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
right: &ImmTy<'tcx, Self::Provenance>,
) -> InterpResult<'tcx, (Scalar<Self::Provenance>, bool, Ty<'tcx>)>;

/// Called to write the specified `local` from the `frame`.
/// Called to write the specified `local` from the current frame.
/// Since writing a ZST is not actually accessing memory or locals, this is never invoked
/// for ZST reads.
///
/// Due to borrow checker trouble, we indicate the `frame` as an index rather than an `&mut
/// Frame`.
#[inline]
fn access_local_mut<'a>(
ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
frame: usize,
local: mir::Local,
) -> InterpResult<'tcx, &'a mut Operand<Self::Provenance>>
where
'tcx: 'mir,
{
ecx.stack_mut()[frame].locals[local].access_mut()
ecx.frame_mut().locals[local].access_mut()
}

/// Called before a basic block terminator is executed.
Expand Down Expand Up @@ -424,10 +420,10 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
/// argument/return value was actually copied or passed in-place..
fn protect_in_place_function_argument(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
place: &PlaceTy<'tcx, Self::Provenance>,
place: &MPlaceTy<'tcx, Self::Provenance>,
) -> InterpResult<'tcx> {
// Without an aliasing model, all we can do is put `Uninit` into the place.
ecx.write_uninit(place)
ecx.write_uninit(&place.into())
}

/// Called immediately before a new stack frame gets pushed.
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_const_eval/src/interpret/operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,9 +504,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
let op = match **place {
Place::Ptr(mplace) => Operand::Indirect(mplace),
Place::Local { frame, local } => {
*self.local_to_op(&self.stack()[frame], local, None)?
}
Place::Local { local } => *self.local_to_op(&self.frame(), local, None)?,
};
Ok(OpTy { op, layout: place.layout, align: Some(place.align) })
}
Expand Down
41 changes: 20 additions & 21 deletions compiler/rustc_const_eval/src/interpret/place.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ pub enum Place<Prov: Provenance = AllocId> {

/// To support alloc-free locals, we are able to write directly to a local.
/// (Without that optimization, we'd just always be a `MemPlace`.)
Local { frame: usize, local: mir::Local },
/// This always refers to a local in the current stack frame. That works because `Place` is
/// never stored anywhere long-term, only for the duration of evaluating a single statement.
Local { local: mir::Local },
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -169,9 +171,9 @@ impl<Prov: Provenance> Place<Prov> {
/// Returns the frame idx and the variable idx.
#[inline]
#[cfg_attr(debug_assertions, track_caller)] // only in debug builds due to perf (see #98980)
pub fn assert_local(&self) -> (usize, mir::Local) {
pub fn assert_local(&self) -> mir::Local {
match self {
Place::Local { frame, local } => (*frame, *local),
Place::Local { local } => *local,
_ => bug!("assert_local: expected Place::Local, got {:?}", self),
}
}
Expand Down Expand Up @@ -283,10 +285,10 @@ impl<'tcx, Prov: Provenance> OpTy<'tcx, Prov> {
impl<'tcx, Prov: Provenance> PlaceTy<'tcx, Prov> {
/// A place is either an mplace or some local.
#[inline]
pub fn as_mplace_or_local(&self) -> Either<MPlaceTy<'tcx, Prov>, (usize, mir::Local)> {
pub fn as_mplace_or_local(&self) -> Either<MPlaceTy<'tcx, Prov>, mir::Local> {
match **self {
Place::Ptr(mplace) => Left(MPlaceTy { mplace, layout: self.layout, align: self.align }),
Place::Local { frame, local } => Right((frame, local)),
Place::Local { local } => Right(local),
}
}

Expand Down Expand Up @@ -348,7 +350,7 @@ where
}

let mplace = self.ref_to_mplace(&val)?;
self.check_mplace(mplace)?;
self.check_mplace(&mplace)?;
Ok(mplace)
}

Expand Down Expand Up @@ -379,7 +381,7 @@ where
}

/// Check if this mplace is dereferenceable and sufficiently aligned.
pub fn check_mplace(&self, mplace: MPlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
pub fn check_mplace(&self, mplace: &MPlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
let (size, _align) = self
.size_and_align_of_mplace(&mplace)?
.unwrap_or((mplace.layout.size, mplace.layout.align.abi));
Expand Down Expand Up @@ -418,11 +420,10 @@ where

pub fn local_to_place(
&self,
frame: usize,
local: mir::Local,
) -> InterpResult<'tcx, PlaceTy<'tcx, M::Provenance>> {
let layout = self.layout_of_local(&self.stack()[frame], local, None)?;
let place = Place::Local { frame, local };
let layout = self.layout_of_local(&self.frame(), local, None)?;
let place = Place::Local { local };
Ok(PlaceTy { place, layout, align: layout.align.abi })
}

Expand All @@ -433,7 +434,7 @@ where
&mut self,
mir_place: mir::Place<'tcx>,
) -> InterpResult<'tcx, PlaceTy<'tcx, M::Provenance>> {
let mut place = self.local_to_place(self.frame_idx(), mir_place.local)?;
let mut place = self.local_to_place(mir_place.local)?;
// Using `try_fold` turned out to be bad for performance, hence the loop.
for elem in mir_place.projection.iter() {
place = self.place_projection(&place, elem)?
Expand Down Expand Up @@ -509,8 +510,8 @@ where
// See if we can avoid an allocation. This is the counterpart to `read_immediate_raw`,
// but not factored as a separate function.
let mplace = match dest.place {
Place::Local { frame, local } => {
match M::access_local_mut(self, frame, local)? {
Place::Local { local } => {
match M::access_local_mut(self, local)? {
Operand::Immediate(local) => {
// Local can be updated in-place.
*local = src;
Expand Down Expand Up @@ -593,8 +594,8 @@ where
pub fn write_uninit(&mut self, dest: &PlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
let mplace = match dest.as_mplace_or_local() {
Left(mplace) => mplace,
Right((frame, local)) => {
match M::access_local_mut(self, frame, local)? {
Right(local) => {
match M::access_local_mut(self, local)? {
Operand::Immediate(local) => {
*local = Immediate::Uninit;
return Ok(());
Expand Down Expand Up @@ -728,16 +729,15 @@ where
place: &PlaceTy<'tcx, M::Provenance>,
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
let mplace = match place.place {
Place::Local { frame, local } => {
match M::access_local_mut(self, frame, local)? {
Place::Local { local } => {
match M::access_local_mut(self, local)? {
&mut Operand::Immediate(local_val) => {
// We need to make an allocation.

// We need the layout of the local. We can NOT use the layout we got,
// that might e.g., be an inner field of a struct with `Scalar` layout,
// that has different alignment than the outer field.
let local_layout =
self.layout_of_local(&self.stack()[frame], local, None)?;
let local_layout = self.layout_of_local(&self.frame(), local, None)?;
if local_layout.is_unsized() {
throw_unsup_format!("unsized locals are not supported");
}
Expand All @@ -755,8 +755,7 @@ where
}
// Now we can call `access_mut` again, asserting it goes well,
// and actually overwrite things.
*M::access_local_mut(self, frame, local).unwrap() =
Operand::Indirect(mplace);
*M::access_local_mut(self, local).unwrap() = Operand::Indirect(mplace);
mplace
}
&mut Operand::Indirect(mplace) => mplace, // this already was an indirect local
Expand Down
Loading