From 7cb74ed1917cc982830d6b7b7bac7e8a7f4d271c Mon Sep 17 00:00:00 2001 From: Tunahan Karlibas Date: Wed, 9 Dec 2020 01:17:02 +0300 Subject: [PATCH 1/4] Remove memoization leftovers closes #79667 --- compiler/rustc_mir/src/const_eval/machine.rs | 63 ++------------------ 1 file changed, 4 insertions(+), 59 deletions(-) diff --git a/compiler/rustc_mir/src/const_eval/machine.rs b/compiler/rustc_mir/src/const_eval/machine.rs index 187f6fab5181f..cc5f5dda7d495 100644 --- a/compiler/rustc_mir/src/const_eval/machine.rs +++ b/compiler/rustc_mir/src/const_eval/machine.rs @@ -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; @@ -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. @@ -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, // unwinding is not supported in consts ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> { debug!("find_mir_or_eval_fn: {:?}", instance); @@ -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)?; From de1cd4b36d57b33f4217a46c0c52fbe1eb4530a1 Mon Sep 17 00:00:00 2001 From: Tunahan Karlibas Date: Wed, 9 Dec 2020 14:53:35 +0300 Subject: [PATCH 2/4] Extra assertions in eval_body_using_ecx to disallow queries for functions that does allocations --- compiler/rustc_middle/src/mir/interpret/mod.rs | 1 - compiler/rustc_mir/src/const_eval/eval_queries.rs | 7 +++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/mir/interpret/mod.rs b/compiler/rustc_middle/src/mir/interpret/mod.rs index bcf85797313f2..80b58642136ee 100644 --- a/compiler/rustc_middle/src/mir/interpret/mod.rs +++ b/compiler/rustc_middle/src/mir/interpret/mod.rs @@ -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> { diff --git a/compiler/rustc_mir/src/const_eval/eval_queries.rs b/compiler/rustc_mir/src/const_eval/eval_queries.rs index 6e09ae4340645..4d14e45c2e935 100644 --- a/compiler/rustc_mir/src/const_eval/eval_queries.rs +++ b/compiler/rustc_mir/src/const_eval/eval_queries.rs @@ -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), + Some(ConstContext::Const | ConstContext::Static(_)) + ) + ); let tcx = *ecx.tcx; let layout = ecx.layout_of(body.return_ty().subst(tcx, cid.instance.substs))?; assert!(!layout.is_unsized()); From b6f7eef946d4b9830bef2a4264c4b62f880f032b Mon Sep 17 00:00:00 2001 From: Tunahan Karlibas Date: Fri, 11 Dec 2020 01:59:05 +0300 Subject: [PATCH 3/4] Remove unnecessary check and fix local_def_id parameter --- compiler/rustc_mir/src/const_eval/eval_queries.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_mir/src/const_eval/eval_queries.rs b/compiler/rustc_mir/src/const_eval/eval_queries.rs index 4d14e45c2e935..058a5ced2abd3 100644 --- a/compiler/rustc_mir/src/const_eval/eval_queries.rs +++ b/compiler/rustc_mir/src/const_eval/eval_queries.rs @@ -7,7 +7,7 @@ use crate::interpret::{ }; use rustc_errors::ErrorReported; -use rustc_hir::def::DefKind; +use rustc_hir::{ConstContext, def::DefKind}; use rustc_middle::mir; use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::traits::Reveal; @@ -30,14 +30,14 @@ 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); + let tcx = *ecx.tcx; assert!( cid.promoted.is_some() || matches!( - ecx.tcx.hir().body_const_context(def_id), + ecx.tcx.hir().body_const_context(cid.instance.def_id().expect_local()), Some(ConstContext::Const | ConstContext::Static(_)) ) ); - let tcx = *ecx.tcx; let layout = ecx.layout_of(body.return_ty().subst(tcx, cid.instance.substs))?; assert!(!layout.is_unsized()); let ret = ecx.allocate(layout, MemoryKind::Stack); @@ -47,14 +47,7 @@ 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, From a03feaae550b17d53ed5edd137ffaeaa530cae92 Mon Sep 17 00:00:00 2001 From: Tunahan Karlibas Date: Fri, 11 Dec 2020 18:19:30 +0300 Subject: [PATCH 4/4] add missing constraints --- .../rustc_mir/src/const_eval/eval_queries.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_mir/src/const_eval/eval_queries.rs b/compiler/rustc_mir/src/const_eval/eval_queries.rs index 058a5ced2abd3..f13b4b7b91924 100644 --- a/compiler/rustc_mir/src/const_eval/eval_queries.rs +++ b/compiler/rustc_mir/src/const_eval/eval_queries.rs @@ -7,7 +7,7 @@ use crate::interpret::{ }; use rustc_errors::ErrorReported; -use rustc_hir::{ConstContext, def::DefKind}; +use rustc_hir::def::DefKind; use rustc_middle::mir; use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::traits::Reveal; @@ -34,9 +34,15 @@ fn eval_body_using_ecx<'mir, 'tcx>( assert!( cid.promoted.is_some() || matches!( - ecx.tcx.hir().body_const_context(cid.instance.def_id().expect_local()), - Some(ConstContext::Const | ConstContext::Static(_)) - ) + 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()); @@ -47,8 +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); - - ecx.push_stack_frame( cid.instance, body,