Skip to content

Commit

Permalink
interpret: make Place always refer to the current frame
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Jul 22, 2023
1 parent 42f5419 commit dcd018c
Show file tree
Hide file tree
Showing 15 changed files with 75 additions and 88 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
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
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
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
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
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

0 comments on commit dcd018c

Please sign in to comment.