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

Miri: avoid tracking current location three times #72879

Merged
merged 7 commits into from
Jun 15, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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: 1 addition & 0 deletions src/librustc_middle/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,7 @@ impl<'tcx> ty::TyS<'tcx> {
/// optimization as well as the rules around static values. Note
/// that the `Freeze` trait is not exposed to end users and is
/// effectively an implementation detail.
// FIXME: use `TyCtxtAt` instead of separate `Span`.
pub fn is_freeze(
&'tcx self,
tcx: TyCtxt<'tcx>,
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_mir/const_eval/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::error::Error;
use std::fmt;

use rustc_middle::mir::AssertKind;
use rustc_span::Symbol;
use rustc_span::{Span, Symbol};

use super::InterpCx;
use crate::interpret::{ConstEvalErr, InterpErrorInfo, Machine};
Expand Down Expand Up @@ -53,8 +53,9 @@ impl Error for ConstEvalErrKind {}
pub fn error_to_const_error<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>(
ecx: &InterpCx<'mir, 'tcx, M>,
error: InterpErrorInfo<'tcx>,
span: Option<Span>,
) -> ConstEvalErr<'tcx> {
error.print_backtrace();
let stacktrace = ecx.generate_stacktrace();
ConstEvalErr { error: error.kind, stacktrace, span: ecx.tcx.span }
ConstEvalErr { error: error.kind, stacktrace, span: span.unwrap_or_else(|| ecx.cur_span()) }
}
31 changes: 19 additions & 12 deletions src/librustc_mir/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ 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.tcx;
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);
Expand Down Expand Up @@ -81,13 +81,14 @@ fn eval_body_using_ecx<'mir, 'tcx>(
/// parameter. These bounds are passed to `mk_eval_cx` via the `ParamEnv` argument.
pub(super) fn mk_eval_cx<'mir, 'tcx>(
tcx: TyCtxt<'tcx>,
span: Span,
root_span: Span,
param_env: ty::ParamEnv<'tcx>,
can_access_statics: bool,
) -> CompileTimeEvalContext<'mir, 'tcx> {
debug!("mk_eval_cx: {:?}", param_env);
InterpCx::new(
tcx.at(span),
tcx,
root_span,
param_env,
CompileTimeInterpreter::new(tcx.sess.const_eval_limit()),
MemoryExtra { can_access_statics },
Expand Down Expand Up @@ -163,7 +164,7 @@ pub(super) fn op_to_const<'tcx>(
0,
),
};
let len = b.to_machine_usize(&ecx.tcx.tcx).unwrap();
let len = b.to_machine_usize(ecx).unwrap();
let start = start.try_into().unwrap();
let len: usize = len.try_into().unwrap();
ConstValue::Slice { data, start, end: start + len }
Expand Down Expand Up @@ -212,8 +213,8 @@ fn validate_and_turn_into_const<'tcx>(
})();

val.map_err(|error| {
let err = error_to_const_error(&ecx, error);
err.struct_error(ecx.tcx, "it is undefined behavior to use this value", |mut diag| {
let err = error_to_const_error(&ecx, error, None);
err.struct_error(ecx.tcx_at(), "it is undefined behavior to use this value", |mut diag| {
diag.note(note_on_undefined_behavior_error());
diag.emit();
})
Expand Down Expand Up @@ -299,9 +300,9 @@ pub fn const_eval_raw_provider<'tcx>(

let is_static = tcx.is_static(def_id);

let span = tcx.def_span(cid.instance.def_id());
let mut ecx = InterpCx::new(
tcx.at(span),
tcx,
tcx.def_span(cid.instance.def_id()),
key.param_env,
CompileTimeInterpreter::new(tcx.sess.const_eval_limit()),
MemoryExtra { can_access_statics: is_static },
Expand All @@ -311,12 +312,15 @@ pub fn const_eval_raw_provider<'tcx>(
res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, &body))
.map(|place| RawConst { alloc_id: place.ptr.assert_ptr().alloc_id, ty: place.layout.ty })
.map_err(|error| {
let err = error_to_const_error(&ecx, error);
let err = error_to_const_error(&ecx, error, None);
// errors in statics are always emitted as fatal errors
if is_static {
// Ensure that if the above error was either `TooGeneric` or `Reported`
// an error must be reported.
let v = err.report_as_error(ecx.tcx, "could not evaluate static initializer");
let v = err.report_as_error(
ecx.tcx.at(ecx.cur_span()),
"could not evaluate static initializer",
);

// If this is `Reveal:All`, then we need to make sure an error is reported but if
// this is `Reveal::UserFacing`, then it's expected that we could get a
Expand Down Expand Up @@ -372,13 +376,16 @@ pub fn const_eval_raw_provider<'tcx>(
// anything else (array lengths, enum initializers, constant patterns) are
// reported as hard errors
} else {
err.report_as_error(ecx.tcx, "evaluation of constant value failed")
err.report_as_error(
ecx.tcx.at(ecx.cur_span()),
"evaluation of constant value failed",
)
}
}
}
} else {
// use of broken constant from other crate
err.report_as_error(ecx.tcx, "could not evaluate constant")
err.report_as_error(ecx.tcx.at(ecx.cur_span()), "could not evaluate constant")
}
})
}
13 changes: 5 additions & 8 deletions src/librustc_mir/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

let instance = ty::Instance::resolve_for_fn_ptr(
*self.tcx,
self.tcx,
self.param_env,
def_id,
substs,
Expand Down Expand Up @@ -91,7 +91,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

let instance = ty::Instance::resolve_closure(
*self.tcx,
self.tcx,
def_id,
substs,
ty::ClosureKind::FnOnce,
Expand Down Expand Up @@ -140,7 +140,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Handle cast from a univariant (ZST) enum.
match src.layout.variants {
Variants::Single { index } => {
if let Some(discr) = src.layout.ty.discriminant_for_variant(*self.tcx, index) {
if let Some(discr) = src.layout.ty.discriminant_for_variant(self.tcx, index) {
assert!(src.layout.is_zst());
let discr_layout = self.layout_of(discr.ty)?;
return Ok(self.cast_from_scalar(discr.val, discr_layout, cast_ty).into());
Expand Down Expand Up @@ -268,11 +268,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
(&ty::Array(_, length), &ty::Slice(_)) => {
let ptr = self.read_immediate(src)?.to_scalar()?;
// u64 cast is from usize to u64, which is always good
let val = Immediate::new_slice(
ptr,
length.eval_usize(self.tcx.tcx, self.param_env),
self,
);
let val =
Immediate::new_slice(ptr, length.eval_usize(self.tcx, self.param_env), self);
self.write_immediate(val, dest)
}
(&ty::Dynamic(..), &ty::Dynamic(..)) => {
Expand Down
61 changes: 39 additions & 22 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
pub machine: M,

/// The results of the type checker, from rustc.
pub tcx: TyCtxtAt<'tcx>,
pub tcx: TyCtxt<'tcx>,

/// The span of the "root" of the evaluation, i.e., the const
/// we are evaluating (if this is CTFE).
pub(super) root_span: Span,

/// Bounds in scope for polymorphic evaluations.
pub(crate) param_env: ty::ParamEnv<'tcx>,
Expand Down Expand Up @@ -196,7 +200,7 @@ where
{
#[inline]
fn tcx(&self) -> TyCtxt<'tcx> {
*self.tcx
self.tcx
}
}

Expand All @@ -209,13 +213,13 @@ where
}
}

impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> LayoutOf for InterpCx<'mir, 'tcx, M> {
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> LayoutOf for InterpCx<'mir, 'tcx, M> {
type Ty = Ty<'tcx>;
type TyAndLayout = InterpResult<'tcx, TyAndLayout<'tcx>>;

#[inline]
fn layout_of(&self, ty: Ty<'tcx>) -> Self::TyAndLayout {
self.tcx
self.tcx_at()
.layout_of(self.param_env.and(ty))
.map_err(|layout| err_inval!(Layout(layout)).into())
}
Expand Down Expand Up @@ -292,24 +296,36 @@ pub(super) fn from_known_layout<'tcx>(

impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub fn new(
tcx: TyCtxtAt<'tcx>,
tcx: TyCtxt<'tcx>,
root_span: Span,
param_env: ty::ParamEnv<'tcx>,
machine: M,
memory_extra: M::MemoryExtra,
) -> Self {
InterpCx {
machine,
tcx,
root_span,
param_env,
memory: Memory::new(tcx, memory_extra),
vtables: FxHashMap::default(),
}
}

#[inline(always)]
pub fn set_span(&mut self, span: Span) {
self.tcx.span = span;
self.memory.tcx.span = span;
pub fn cur_span(&self) -> Span {
self.stack()
.last()
.and_then(|f| f.current_source_info())
.map(|si| si.span)
.unwrap_or(self.root_span)
}

#[inline(always)]
pub fn tcx_at(&self) -> TyCtxtAt<'tcx> {
// Computing the current span has a non-trivial cost, and for cycle errors
// the "root span" is good enough.
self.tcx.at(self.root_span)
}

#[inline(always)]
Expand Down Expand Up @@ -387,12 +403,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

#[inline]
pub fn type_is_sized(&self, ty: Ty<'tcx>) -> bool {
ty.is_sized(self.tcx, self.param_env)
ty.is_sized(self.tcx_at(), self.param_env)
}

#[inline]
pub fn type_is_freeze(&self, ty: Ty<'tcx>) -> bool {
ty.is_freeze(*self.tcx, self.param_env, DUMMY_SP)
ty.is_freeze(self.tcx, self.param_env, self.root_span)
}

pub fn load_mir(
Expand All @@ -403,20 +419,21 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// do not continue if typeck errors occurred (can only occur in local crate)
let did = instance.def_id();
if let Some(did) = did.as_local() {
if self.tcx.has_typeck_tables(did) {
if let Some(error_reported) = self.tcx.typeck_tables_of(did).tainted_by_errors {
if self.tcx_at().has_typeck_tables(did) {
if let Some(error_reported) = self.tcx_at().typeck_tables_of(did).tainted_by_errors
{
throw_inval!(TypeckError(error_reported))
}
}
}
trace!("load mir(instance={:?}, promoted={:?})", instance, promoted);
if let Some(promoted) = promoted {
return Ok(&self.tcx.promoted_mir(did)[promoted]);
return Ok(&self.tcx_at().promoted_mir(did)[promoted]);
}
match instance {
ty::InstanceDef::Item(def_id) => {
if self.tcx.is_mir_available(did) {
Ok(self.tcx.optimized_mir(did))
if self.tcx_at().is_mir_available(did) {
Ok(self.tcx_at().optimized_mir(did))
} else {
throw_unsup!(NoMirFor(def_id))
}
Expand Down Expand Up @@ -457,7 +474,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
trace!("resolve: {:?}, {:#?}", def_id, substs);
trace!("param_env: {:#?}", self.param_env);
trace!("substs: {:#?}", substs);
match ty::Instance::resolve(*self.tcx, self.param_env, def_id, substs) {
match ty::Instance::resolve(self.tcx, self.param_env, def_id, substs) {
Ok(Some(instance)) => Ok(instance),
Ok(None) => throw_inval!(TooGeneric),

Expand All @@ -476,7 +493,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// have to support that case (mostly by skipping all caching).
match frame.locals.get(local).and_then(|state| state.layout.get()) {
None => {
let layout = from_known_layout(self.tcx, layout, || {
let layout = from_known_layout(self.tcx_at(), layout, || {
let local_ty = frame.body.local_decls[local].ty;
let local_ty =
self.subst_from_frame_and_normalize_erasing_regions(frame, local_ty);
Expand Down Expand Up @@ -561,7 +578,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let size = size.align_to(align);

// Check if this brought us over the size limit.
if size.bytes() >= self.tcx.data_layout().obj_size_bound() {
if size.bytes() >= self.tcx.data_layout.obj_size_bound() {
throw_ub!(InvalidMeta("total size is bigger than largest supported object"));
}
Ok(Some((size, align)))
Expand All @@ -577,7 +594,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let elem = layout.field(self, 0)?;

// Make sure the slice is not too big.
let size = elem.size.checked_mul(len, &*self.tcx).ok_or_else(|| {
let size = elem.size.checked_mul(len, self).ok_or_else(|| {
err_ub!(InvalidMeta("slice is bigger than largest supported object"))
})?;
Ok(Some((size, elem.align.abi)))
Expand Down Expand Up @@ -628,7 +645,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let mut locals = IndexVec::from_elem(dummy, &body.local_decls);

// Now mark those locals as dead that we do not want to initialize
match self.tcx.def_kind(instance.def_id()) {
match self.tcx_at().def_kind(instance.def_id()) {
// statics and constants don't have `Storage*` statements, no need to look for them
//
// FIXME: The above is likely untrue. See
Expand Down Expand Up @@ -843,7 +860,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
} else {
self.param_env
};
let val = self.tcx.const_eval_global_id(param_env, gid, Some(self.tcx.span))?;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably either this call that is expensive...

let val = self.tcx.const_eval_global_id(param_env, gid, Some(self.cur_span()))?;

// Even though `ecx.const_eval` is called from `eval_const_to_op` we can never have a
// recursion deeper than one level, because the `tcx.const_eval` above is guaranteed to not
Expand Down Expand Up @@ -874,7 +891,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// FIXME: We can hit delay_span_bug if this is an invalid const, interning finds
// that problem, but we never run validation to show an error. Can we ensure
// this does not happen?
let val = self.tcx.const_eval_raw(param_env.and(gid))?;
let val = self.tcx.at(self.cur_span()).const_eval_raw(param_env.and(gid))?;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... or this.

self.raw_const_to_mplace(val)
}

Expand Down
Loading