Skip to content

Commit

Permalink
Auto merge of rust-lang#79840 - dvtkrlbs:issue-79667, r=oli-obk
Browse files Browse the repository at this point in the history
Remove memoization leftovers from constant evaluation machine

Closes rust-lang#79667
  • Loading branch information
bors committed Dec 17, 2020
2 parents bdd0a78 + a03feaa commit 001bd77
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 69 deletions.
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
22 changes: 13 additions & 9 deletions compiler/rustc_mir/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ fn eval_body_using_ecx<'mir, 'tcx>(
) -> InterpResult<'tcx, MPlaceTy<'tcx>> {
debug!("eval_body_using_ecx: {:?}, {:?}", cid, ecx.param_env);
let tcx = *ecx.tcx;
assert!(
cid.promoted.is_some()
|| matches!(
ecx.tcx.def_kind(cid.instance.def_id()),
DefKind::Const
| DefKind::Static
| DefKind::ConstParam
| DefKind::AnonConst
| DefKind::AssocConst
),
"Unexpected DefKind: {:?}",
ecx.tcx.def_kind(cid.instance.def_id())
);
let layout = ecx.layout_of(body.return_ty().subst(tcx, cid.instance.substs))?;
assert!(!layout.is_unsized());
let ret = ecx.allocate(layout, MemoryKind::Stack);
Expand All @@ -40,15 +53,6 @@ fn eval_body_using_ecx<'mir, 'tcx>(
let prom = cid.promoted.map_or(String::new(), |p| format!("::promoted[{:?}]", p));
trace!("eval_body_using_ecx: pushing stack frame for global: {}{}", name, prom);

// Assert all args (if any) are zero-sized types; `eval_body_using_ecx` doesn't
// make sense if the body is expecting nontrivial arguments.
// (The alternative would be to use `eval_fn_call` with an args slice.)
for arg in body.args_iter() {
let decl = body.local_decls.get(arg).expect("arg missing from local_decls");
let layout = ecx.layout_of(decl.ty.subst(tcx, cid.instance.substs))?;
assert!(layout.is_zst())
}

ecx.push_stack_frame(
cid.instance,
body,
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)?;

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

0 comments on commit 001bd77

Please sign in to comment.