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.

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

Remove memoization leftovers from constant evaluation machine #79840

Merged
merged 4 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ pub use self::pointer::{Pointer, PointerArithmetic};
/// Uniquely identifies one of the following:
/// - A constant
/// - A static
/// - A const fn where all arguments (if any) are zero-sized types
#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, TyEncodable, TyDecodable)]
#[derive(HashStable, Lift)]
pub struct GlobalId<'tcx> {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_mir/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ fn eval_body_using_ecx<'mir, 'tcx>(
body: &'mir mir::Body<'tcx>,
) -> InterpResult<'tcx, MPlaceTy<'tcx>> {
debug!("eval_body_using_ecx: {:?}, {:?}", cid, ecx.param_env);
assert!(
cid.promoted.is_some()
|| matches!(
ecx.tcx.hir().body_const_context(def_id),
dvtkrlbs marked this conversation as resolved.
Show resolved Hide resolved
Some(ConstContext::Const | ConstContext::Static(_))
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
)
dvtkrlbs marked this conversation as resolved.
Show resolved Hide resolved
);
let tcx = *ecx.tcx;
let layout = ecx.layout_of(body.return_ty().subst(tcx, cid.instance.substs))?;
assert!(!layout.is_unsized());
Expand Down
63 changes: 4 additions & 59 deletions compiler/rustc_mir/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use rustc_middle::mir;
use rustc_middle::ty::layout::HasTyCtxt;
use rustc_middle::ty::InstanceDef;
use rustc_middle::ty::{self, Ty};
use std::borrow::Borrow;
use std::collections::hash_map::Entry;
Expand All @@ -17,60 +15,13 @@ use rustc_span::symbol::{sym, Symbol};
use rustc_target::abi::{Align, Size};

use crate::interpret::{
self, compile_time_machine, AllocId, Allocation, Frame, GlobalId, ImmTy, InterpCx,
InterpResult, Memory, OpTy, PlaceTy, Pointer, Scalar,
self, compile_time_machine, AllocId, Allocation, Frame, ImmTy, InterpCx, InterpResult, Memory,
OpTy, PlaceTy, Pointer, Scalar,
};

use super::error::*;

impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> {
/// Evaluate a const function where all arguments (if any) are zero-sized types.
/// The evaluation is memoized thanks to the query system.
///
/// Returns `true` if the call has been evaluated.
fn try_eval_const_fn_call(
&mut self,
instance: ty::Instance<'tcx>,
ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>,
args: &[OpTy<'tcx>],
) -> InterpResult<'tcx, bool> {
trace!("try_eval_const_fn_call: {:?}", instance);
// Because `#[track_caller]` adds an implicit non-ZST argument, we also cannot
// perform this optimization on items tagged with it.
if instance.def.requires_caller_location(self.tcx()) {
return Ok(false);
}
// Only memoize instrinsics. This was added in #79594 while adding the `const_allocate` intrinsic.
// We only memoize intrinsics because it would be unsound to memoize functions
// which might interact with the heap.
// Additionally, const_allocate intrinsic is impure and thus should not be memoized;
// it will not be memoized because it has non-ZST args
if !matches!(instance.def, InstanceDef::Intrinsic(_)) {
return Ok(false);
}
// For the moment we only do this for functions which take no arguments
// (or all arguments are ZSTs) so that we don't memoize too much.
if args.iter().any(|a| !a.layout.is_zst()) {
return Ok(false);
}

let dest = match ret {
Some((dest, _)) => dest,
// Don't memoize diverging function calls.
None => return Ok(false),
};

let gid = GlobalId { instance, promoted: None };

let place = self.eval_to_allocation(gid)?;
dvtkrlbs marked this conversation as resolved.
Show resolved Hide resolved

self.copy_op(place.into(), dest)?;

self.return_to_block(ret.map(|r| r.1))?;
trace!("{:?}", self.dump_place(*dest));
Ok(true)
}

/// "Intercept" a function call to a panic-related function
/// because we have something special to do for it.
/// If this returns successfully (`Ok`), the function should just be evaluated normally.
Expand Down Expand Up @@ -253,7 +204,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
ecx: &mut InterpCx<'mir, 'tcx, Self>,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx>],
ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>,
_ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>,
_unwind: Option<mir::BasicBlock>, // unwinding is not supported in consts
) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> {
debug!("find_mir_or_eval_fn: {:?}", instance);
Expand All @@ -263,13 +214,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
// Execution might have wandered off into other crates, so we cannot do a stability-
// sensitive check here. But we can at least rule out functions that are not const
// at all.
if ecx.tcx.is_const_fn_raw(def.did) {
// If this function is a `const fn` then under certain circumstances we
// can evaluate call via the query system, thus memoizing all future calls.
if ecx.try_eval_const_fn_call(instance, ret, args)? {
return Ok(None);
}
} else {
if !ecx.tcx.is_const_fn_raw(def.did) {
// Some functions we support even if they are non-const -- but avoid testing
// that for const fn!
ecx.hook_panic_fn(instance, args)?;
Expand Down