Skip to content
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

Fix perf regression from #113569 #113630

Closed
wants to merge 3 commits into from
Closed

Fix perf regression from #113569 #113630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d63e43
perf: don't deinit protected fn args
RalfJung Jul 13, 2023
0ee0889
interpret: avoid some clone() in argument handling
RalfJung Jul 13, 2023
9834a41
TEMP: try to reproduce earlier perf results
RalfJung Jul 18, 2023
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
12 changes: 8 additions & 4 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 @@ -421,13 +421,17 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
/// Called on places used for in-place function argument and return value handling.
///
/// These places need to be protected to make sure the program cannot tell whether the
/// argument/return value was actually copied or passed in-place..
/// 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>,
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
_place: &PlaceTy<'tcx, Self::Provenance>,
) -> InterpResult<'tcx> {
// Without an aliasing model, all we can do is put `Uninit` into the place.
ecx.write_uninit(place)
// This can only be violated with custom MIR though so we avoid the perf hit.
//if ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks {
// ecx.write_uninit(place)?;
//}
Ok(())
}

/// Called immediately before a new stack frame gets pushed.
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ impl<'tcx, Prov: Provenance> FnArg<'tcx, Prov> {
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Make a copy of the given fn_arg. Any `InPlace` are degenerated to copies, no protection of the
/// original memory occurs.
pub fn copy_fn_arg(
pub fn copy_fn_arg<'a>(
&self,
arg: &FnArg<'tcx, M::Provenance>,
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
arg: &'a FnArg<'tcx, M::Provenance>,
) -> InterpResult<'tcx, Cow<'a, OpTy<'tcx, M::Provenance>>> {
match arg {
FnArg::Copy(op) => Ok(op.clone()),
FnArg::InPlace(place) => self.place_to_op(&place),
FnArg::Copy(op) => Ok(Cow::Borrowed(op)),
FnArg::InPlace(place) => Ok(Cow::Owned(self.place_to_op(&place)?)),
}
}

Expand All @@ -56,7 +56,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
&self,
args: &[FnArg<'tcx, M::Provenance>],
) -> InterpResult<'tcx, Vec<OpTy<'tcx, M::Provenance>>> {
args.iter().map(|fn_arg| self.copy_fn_arg(fn_arg)).collect()
args.iter().map(|fn_arg| self.copy_fn_arg(fn_arg).map(|x| x.into_owned())).collect()
}

pub fn fn_arg_field(
Expand Down Expand Up @@ -637,7 +637,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// An `InPlace` does nothing here, we keep the original receiver intact. We can't
// really pass the argument in-place anyway, and we are constructing a new
// `Immediate` receiver.
let mut receiver = self.copy_fn_arg(&args[0])?;
let mut receiver = self.copy_fn_arg(&args[0])?.into_owned();
let receiver_place = loop {
match receiver.layout.ty.kind() {
ty::Ref(..) | ty::RawPtr(..) => {
Expand Down