From 7c966de04e085ef87d36a193dd82e2d61ff0790a Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 10 Feb 2019 14:39:58 +0100 Subject: [PATCH 01/23] Setup `ty::Const` to allow `ConstValue::Scalar` to be backed by an allocation --- src/librustc/ich/impls_ty.rs | 6 ++- src/librustc/mir/interpret/value.rs | 14 +++-- src/librustc/mir/mod.rs | 1 + src/librustc/ty/structural_impls.rs | 46 ++++++---------- src/librustc/ty/sty.rs | 20 +++++-- src/librustc_codegen_llvm/consts.rs | 9 ++-- src/librustc_codegen_ssa/mir/operand.rs | 5 +- src/librustc_codegen_ssa/mir/place.rs | 5 +- src/librustc_mir/const_eval.rs | 10 ++-- src/librustc_mir/hair/constant.rs | 5 +- src/librustc_mir/hair/pattern/_match.rs | 61 +++++++++++++--------- src/librustc_mir/interpret/operand.rs | 7 ++- src/librustc_mir/monomorphize/collector.rs | 11 ++-- src/librustc_typeck/check/mod.rs | 4 +- 14 files changed, 110 insertions(+), 94 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 1e1dbd0b621ec..5f01b4f6abec9 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -300,12 +300,13 @@ impl_stable_hash_for!(struct ty::FieldDef { }); impl_stable_hash_for!( - impl<'tcx> for enum mir::interpret::ConstValue<'tcx> [ mir::interpret::ConstValue ] { + impl<'tcx> for enum mir::interpret::ConstValue [ mir::interpret::ConstValue ] { Scalar(val), Slice(a, b), - ByRef(id, alloc, offset), + ByRef, } ); + impl_stable_hash_for!(struct crate::mir::interpret::RawConst<'tcx> { alloc_id, ty, @@ -374,6 +375,7 @@ impl_stable_hash_for!(enum ::syntax::ast::Mutability { impl_stable_hash_for!(struct ty::Const<'tcx> { ty, + alloc, val }); diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 5ec7de4308a13..ba04b85dce45c 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -2,7 +2,7 @@ use std::fmt; use crate::ty::{Ty, layout::{HasDataLayout, Size}}; -use super::{EvalResult, Pointer, PointerArithmetic, Allocation, AllocId, sign_extend, truncate}; +use super::{EvalResult, Pointer, PointerArithmetic, AllocId, sign_extend, truncate}; /// Represents the result of a raw const operation, pre-validation. #[derive(Copy, Clone, Debug, Eq, PartialEq, RustcEncodable, RustcDecodable, Hash)] @@ -16,7 +16,7 @@ pub struct RawConst<'tcx> { /// Represents a constant value in Rust. `Scalar` and `ScalarPair` are optimizations that /// match the `LocalState` optimizations for easy conversions between `Value` and `ConstValue`. #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, RustcEncodable, RustcDecodable, Hash)] -pub enum ConstValue<'tcx> { +pub enum ConstValue { /// Used only for types with `layout::abi::Scalar` ABI and ZSTs. /// /// Not using the enum `Value` to encode that this must not be `Undef`. @@ -31,19 +31,17 @@ pub enum ConstValue<'tcx> { /// it. Slice(Scalar, u64), - /// An allocation together with an offset into the allocation. - /// Invariant: the `AllocId` matches the allocation. - ByRef(AllocId, &'tcx Allocation, Size), + ByRef, } #[cfg(target_arch = "x86_64")] -static_assert!(CONST_SIZE: ::std::mem::size_of::>() == 40); +static_assert!(CONST_SIZE: ::std::mem::size_of::() == 40); -impl<'tcx> ConstValue<'tcx> { +impl ConstValue { #[inline] pub fn try_to_scalar(&self) -> Option { match *self { - ConstValue::ByRef(..) | + ConstValue::ByRef | ConstValue::Slice(..) => None, ConstValue::Scalar(val) => Some(val), } diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 3513d652b5346..a8b6f21d83b2f 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1665,6 +1665,7 @@ impl<'tcx> TerminatorKind<'tcx> { }.into(), ), ty: switch_ty, + alloc: None, }; fmt_const_val(&mut s, c).unwrap(); s.into() diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs index d09cfa84a1690..f7f54a1309d8d 100644 --- a/src/librustc/ty/structural_impls.rs +++ b/src/librustc/ty/structural_impls.rs @@ -4,7 +4,6 @@ //! `BraceStructLiftImpl!`) to help with the tedium. use crate::mir::ProjectionKind; -use crate::mir::interpret::ConstValue; use crate::ty::{self, Lift, Ty, TyCtxt}; use crate::ty::fold::{TypeFoldable, TypeFolder, TypeVisitor}; use rustc_data_structures::indexed_vec::{IndexVec, Idx}; @@ -486,23 +485,20 @@ BraceStructLiftImpl! { } } -BraceStructLiftImpl! { - impl<'a, 'tcx> Lift<'tcx> for ty::Const<'a> { - type Lifted = ty::Const<'tcx>; - val, ty - } -} - -impl<'a, 'tcx> Lift<'tcx> for ConstValue<'a> { - type Lifted = ConstValue<'tcx>; +impl<'a, 'tcx> Lift<'tcx> for ty::Const<'a> { + type Lifted = ty::Const<'tcx>; fn lift_to_tcx<'b, 'gcx>(&self, tcx: TyCtxt<'b, 'gcx, 'tcx>) -> Option { - match *self { - ConstValue::Scalar(x) => Some(ConstValue::Scalar(x)), - ConstValue::Slice(x, y) => Some(ConstValue::Slice(x, y)), - ConstValue::ByRef(x, alloc, z) => Some(ConstValue::ByRef( - x, alloc.lift_to_tcx(tcx)?, z, - )), - } + let ty = self.ty.lift_to_tcx(tcx)?; + let alloc = match self.alloc { + // can't use `and_then` or `map`, because the inner `lift_to_tcx` needs to early return + Some((a, p)) => Some((a.lift_to_tcx(tcx)?, p)), + None => None, + }; + Some(ty::Const { + val: self.val, + ty, + alloc, + }) } } @@ -1064,24 +1060,14 @@ impl<'tcx> TypeFoldable<'tcx> for &'tcx ty::LazyConst<'tcx> { impl<'tcx> TypeFoldable<'tcx> for ty::Const<'tcx> { fn super_fold_with<'gcx: 'tcx, F: TypeFolder<'gcx, 'tcx>>(&self, folder: &mut F) -> Self { let ty = self.ty.fold_with(folder); - let val = self.val.fold_with(folder); ty::Const { ty, - val + val: self.val, + alloc: self.alloc, } } fn super_visit_with>(&self, visitor: &mut V) -> bool { - self.ty.visit_with(visitor) || self.val.visit_with(visitor) - } -} - -impl<'tcx> TypeFoldable<'tcx> for ConstValue<'tcx> { - fn super_fold_with<'gcx: 'tcx, F: TypeFolder<'gcx, 'tcx>>(&self, _folder: &mut F) -> Self { - *self - } - - fn super_visit_with>(&self, _visitor: &mut V) -> bool { - false + self.ty.visit_with(visitor) } } diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index 66efd2aea155a..6344539946bdf 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -11,7 +11,7 @@ use crate::ty::subst::{Substs, Subst, Kind, UnpackedKind}; use crate::ty::{self, AdtDef, TypeFlags, Ty, TyCtxt, TypeFoldable}; use crate::ty::{List, TyS, ParamEnvAnd, ParamEnv}; use crate::util::captures::Captures; -use crate::mir::interpret::{Scalar, Pointer}; +use crate::mir::interpret::{Scalar, Pointer, Allocation}; use smallvec::SmallVec; use std::iter; @@ -2065,7 +2065,7 @@ pub enum LazyConst<'tcx> { } #[cfg(target_arch = "x86_64")] -static_assert!(LAZY_CONST_SIZE: ::std::mem::size_of::>() == 56); +static_assert!(LAZY_CONST_SIZE: ::std::mem::size_of::>() == 80); impl<'tcx> LazyConst<'tcx> { pub fn map_evaluated(self, f: impl FnOnce(Const<'tcx>) -> Option) -> Option { @@ -2090,11 +2090,22 @@ impl<'tcx> LazyConst<'tcx> { pub struct Const<'tcx> { pub ty: Ty<'tcx>, - pub val: ConstValue<'tcx>, + /// This field is an optimization for caching commonly needed values of constants like `usize` + /// (or other integers for enum discriminants) and slices (e.g. from `b"foo"` and `"foo"` + /// literals) + pub val: ConstValue, + + /// The actual backing storage of the constant and a pointer which can be resolved back to the + /// `allocation` field + /// + /// Can be `None` for trivial constants created from literals or directly. Is always `Some` for + /// aggregate constants or any named constant that you can actually end up taking a reference + /// to. This will get unwrapped in situations where we do know that it's a referencable + pub alloc: Option<(&'tcx Allocation, Pointer)>, } #[cfg(target_arch = "x86_64")] -static_assert!(CONST_SIZE: ::std::mem::size_of::>() == 48); +static_assert!(CONST_SIZE: ::std::mem::size_of::>() == 72); impl<'tcx> Const<'tcx> { #[inline] @@ -2105,6 +2116,7 @@ impl<'tcx> Const<'tcx> { Self { val: ConstValue::Scalar(val), ty, + alloc: None, } } diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index ca9e2c87be237..77c677af9b0dd 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -69,11 +69,12 @@ pub fn codegen_static_initializer( }; let param_env = ty::ParamEnv::reveal_all(); let static_ = cx.tcx.const_eval(param_env.and(cid))?; - - let alloc = match static_.val { - ConstValue::ByRef(_, alloc, n) if n.bytes() == 0 => alloc, + let (alloc, ptr) = static_.alloc.unwrap(); + assert_eq!(ptr.offset.bytes(), 0); + match static_.val { + ConstValue::ByRef => {}, _ => bug!("static const eval returned {:#?}", static_), - }; + } Ok((const_alloc_to_llvm(cx, alloc), alloc)) } diff --git a/src/librustc_codegen_ssa/mir/operand.rs b/src/librustc_codegen_ssa/mir/operand.rs index 2c6d968bb032a..294acbb75c4fc 100644 --- a/src/librustc_codegen_ssa/mir/operand.rs +++ b/src/librustc_codegen_ssa/mir/operand.rs @@ -101,8 +101,9 @@ impl<'a, 'tcx: 'a, V: CodegenObject> OperandRef<'tcx, V> { let b_llval = bx.cx().const_usize(b); OperandValue::Pair(a_llval, b_llval) }, - ConstValue::ByRef(_, alloc, offset) => { - return Ok(bx.load_operand(bx.cx().from_const_alloc(layout, alloc, offset))); + ConstValue::ByRef => { + let (alloc, ptr) = val.alloc.unwrap(); + return Ok(bx.load_operand(bx.cx().from_const_alloc(layout, alloc, ptr.offset))); }, }; diff --git a/src/librustc_codegen_ssa/mir/place.rs b/src/librustc_codegen_ssa/mir/place.rs index 9d6826d8756b7..f1674d36287c2 100644 --- a/src/librustc_codegen_ssa/mir/place.rs +++ b/src/librustc_codegen_ssa/mir/place.rs @@ -417,8 +417,9 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let layout = cx.layout_of(self.monomorphize(&ty)); match bx.tcx().const_eval(param_env.and(cid)) { Ok(val) => match val.val { - mir::interpret::ConstValue::ByRef(_, alloc, offset) => { - bx.cx().from_const_alloc(layout, alloc, offset) + mir::interpret::ConstValue::ByRef => { + let (alloc, ptr) = val.alloc.unwrap(); + bx.cx().from_const_alloc(layout, alloc, ptr.offset) } _ => bug!("promoteds should have an allocation: {:?}", val), }, diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 7be7f4b439289..33d4113ef4779 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -83,7 +83,7 @@ pub fn op_to_const<'tcx>( Operand::Immediate(val) => Ok(val) } }; - let val = match normalized_op { + let (val, alloc) = match normalized_op { Err(MemPlace { ptr, align, meta }) => { // extract alloc-offset pair assert!(meta.is_none()); @@ -96,14 +96,14 @@ pub fn op_to_const<'tcx>( // FIXME shouldn't it be the case that `mark_static_initialized` has already // interned this? I thought that is the entire point of that `FinishStatic` stuff? let alloc = ecx.tcx.intern_const_alloc(alloc); - ConstValue::ByRef(ptr.alloc_id, alloc, ptr.offset) + (ConstValue::ByRef, Some((alloc, ptr))) }, Ok(Immediate::Scalar(x)) => - ConstValue::Scalar(x.not_undef()?), + (ConstValue::Scalar(x.not_undef()?), None), Ok(Immediate::ScalarPair(a, b)) => - ConstValue::Slice(a.not_undef()?, b.to_usize(ecx)?), + (ConstValue::Slice(a.not_undef()?, b.to_usize(ecx)?), None), }; - Ok(ty::Const { val, ty: op.layout.ty }) + Ok(ty::Const { val, ty: op.layout.ty, alloc }) } fn eval_body_and_ecx<'a, 'mir, 'tcx>( diff --git a/src/librustc_mir/hair/constant.rs b/src/librustc_mir/hair/constant.rs index 21c471d49ee66..80416be3e8f56 100644 --- a/src/librustc_mir/hair/constant.rs +++ b/src/librustc_mir/hair/constant.rs @@ -42,6 +42,7 @@ crate fn lit_to_const<'a, 'gcx, 'tcx>( let id = tcx.allocate_bytes(s.as_bytes()); return Ok(ty::Const { val: ConstValue::new_slice(Scalar::Ptr(id.into()), s.len() as u64), + alloc: None, ty: tcx.types.err, }); }, @@ -72,14 +73,14 @@ crate fn lit_to_const<'a, 'gcx, 'tcx>( LitKind::Bool(b) => ConstValue::Scalar(Scalar::from_bool(b)), LitKind::Char(c) => ConstValue::Scalar(Scalar::from_char(c)), }; - Ok(ty::Const { val: lit, ty }) + Ok(ty::Const { val: lit, ty, alloc: None }) } fn parse_float<'tcx>( num: Symbol, fty: ast::FloatTy, neg: bool, -) -> Result, ()> { +) -> Result { let num = num.as_str(); use rustc_apfloat::ieee::{Single, Double}; use rustc_apfloat::Float; diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 1c7e1aa4d71e0..8e9a536b94273 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -172,7 +172,7 @@ use rustc::ty::{self, Ty, TyCtxt, TypeFoldable, Const}; use rustc::ty::layout::{Integer, IntegerExt, VariantIdx, Size}; use rustc::mir::Field; -use rustc::mir::interpret::{ConstValue, Pointer, Scalar}; +use rustc::mir::interpret::{ConstValue, Scalar, Allocation, Pointer}; use rustc::util::common::ErrorReported; use syntax::attr::{SignedInt, UnsignedInt}; @@ -205,32 +205,41 @@ impl<'a, 'tcx> LiteralExpander<'a, 'tcx> { /// the array to a slice in that case. fn fold_const_value_deref( &mut self, - val: ConstValue<'tcx>, + val: ConstValue, + alloc: Option<(&'tcx Allocation, Pointer)>, // the pattern's pointee type rty: Ty<'tcx>, // the constant's pointee type crty: Ty<'tcx>, - ) -> ConstValue<'tcx> { + ) -> ty::Const<'tcx> { match (val, &crty.sty, &rty.sty) { // the easy case, deref a reference - (ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => ConstValue::ByRef( - p.alloc_id, - self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id), - p.offset, - ), + (ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => ty::Const { + val: ConstValue::ByRef, + alloc: Some((self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id), p)), + ty: rty, + }, // unsize array to slice if pattern is array but match value or other patterns are slice (ConstValue::Scalar(Scalar::Ptr(p)), ty::Array(t, n), ty::Slice(u)) => { assert_eq!(t, u); - ConstValue::Slice( - Scalar::Ptr(p), - n.map_evaluated(|val| val.val.try_to_scalar()) - .unwrap() - .to_usize(&self.tcx) - .unwrap(), - ) + ty::Const { + val: ConstValue::Slice( + Scalar::Ptr(p), + n.map_evaluated(|val| val.val.try_to_scalar()) + .unwrap() + .to_usize(&self.tcx) + .unwrap(), + ), + ty: rty, + alloc: None, + } }, // fat pointers stay the same - (ConstValue::Slice(..), _, _) => val, + (ConstValue::Slice(..), _, _) => ty::Const { + val, + alloc, + ty: rty, + }, // FIXME(oli-obk): this is reachable for `const FOO: &&&u32 = &&&42;` being used _ => bug!("cannot deref {:#?}, {} -> {}", val, crty, rty), } @@ -244,6 +253,7 @@ impl<'a, 'tcx> PatternFolder<'tcx> for LiteralExpander<'a, 'tcx> { &ty::Ref(_, rty, _), &PatternKind::Constant { value: Const { val, + alloc, ty: ty::TyS { sty: ty::Ref(_, crty, _), .. }, } }, ) => { @@ -254,10 +264,9 @@ impl<'a, 'tcx> PatternFolder<'tcx> for LiteralExpander<'a, 'tcx> { subpattern: Pattern { ty: rty, span: pat.span, - kind: box PatternKind::Constant { value: Const { - val: self.fold_const_value_deref(val, rty, crty), - ty: rty, - } }, + kind: box PatternKind::Constant { + value: self.fold_const_value_deref(val, alloc, rty, crty), + }, } } } @@ -1428,7 +1437,7 @@ fn slice_pat_covered_by_const<'tcx>( suffix: &[Pattern<'tcx>] ) -> Result { let data: &[u8] = match (const_val.val, &const_val.ty.sty) { - (ConstValue::ByRef(id, alloc, offset), ty::Array(t, n)) => { + (ConstValue::ByRef, ty::Array(t, n)) => { if *t != tcx.types.u8 { // FIXME(oli-obk): can't mix const patterns with slice patterns and get // any sort of exhaustiveness/unreachable check yet @@ -1436,7 +1445,7 @@ fn slice_pat_covered_by_const<'tcx>( // are definitely unreachable. return Ok(false); } - let ptr = Pointer::new(id, offset); + let (alloc, ptr) = const_val.alloc.expect("ByRef ty::Const with None alloc field"); let n = n.assert_usize(tcx).unwrap(); alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap() }, @@ -1778,8 +1787,8 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( let (opt_ptr, n, ty) = match value.ty.sty { ty::TyKind::Array(t, n) => { match value.val { - ConstValue::ByRef(id, alloc, offset) => ( - Some((Pointer::new(id, offset), alloc)), + ConstValue::ByRef => ( + value.alloc, n.unwrap_usize(cx.tcx), t, ), @@ -1793,8 +1802,8 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( match value.val { ConstValue::Slice(ptr, n) => ( ptr.to_ptr().ok().map(|ptr| ( - ptr, cx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id), + ptr, )), n, t, @@ -1817,7 +1826,7 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( // convert a constant slice/array pattern to a list of patterns. match (n, opt_ptr) { (0, _) => Some(SmallVec::new()), - (_, Some((ptr, alloc))) => { + (_, Some((alloc, ptr))) => { let layout = cx.tcx.layout_of(cx.param_env.and(ty)).ok()?; (0..n).map(|i| { let ptr = ptr.offset(layout.size * i, &cx.tcx).ok()?; diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 7da907028eebf..d928ad20f676e 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -585,11 +585,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> ty::LazyConst::Evaluated(c) => c, }; match val.val { - ConstValue::ByRef(id, alloc, offset) => { + ConstValue::ByRef => { + let (alloc, ptr) = val.alloc.expect( + "ByRef ty::Const must have corresponding Some alloc field", + ); // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen -- and for `static mut`, we copy on demand anyway. Ok(Operand::Indirect( - MemPlace::from_ptr(Pointer::new(id, offset), alloc.align) + MemPlace::from_ptr(ptr, alloc.align) ).with_default_tag()) }, ConstValue::Slice(a, b) => diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index a76aa7454cbe4..82ce244de092e 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -1257,11 +1257,12 @@ fn collect_const<'a, 'tcx>( ConstValue::Slice(Scalar::Ptr(ptr), _) | ConstValue::Scalar(Scalar::Ptr(ptr)) => collect_miri(tcx, ptr.alloc_id, output), - ConstValue::ByRef(_id, alloc, _offset) => { - for &((), id) in alloc.relocations.values() { - collect_miri(tcx, id, output); - } - } + ConstValue::ByRef => {} _ => {}, } + if let Some((alloc, _)) = constant.alloc { + for &((), id) in alloc.relocations.values() { + collect_miri(tcx, id, output); + } + } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 91e44a1588268..1bb4aeb5c05e1 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1464,8 +1464,8 @@ fn maybe_check_static_with_link_section(tcx: TyCtxt, id: DefId, span: Span) { }; let param_env = ty::ParamEnv::reveal_all(); if let Ok(static_) = tcx.const_eval(param_env.and(cid)) { - let alloc = if let ConstValue::ByRef(_, allocation, _) = static_.val { - allocation + let alloc = if let ConstValue::ByRef = static_.val { + static_.alloc.unwrap().0 } else { bug!("Matching on non-ByRef static") }; From a5a5ef1399cfb549913e9bdde2e0110eecda67ef Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 10 Feb 2019 15:20:13 +0100 Subject: [PATCH 02/23] Fiddle the corresponding allocation through all `RawConst` uses --- src/librustc/ich/impls_ty.rs | 2 ++ src/librustc/mir/interpret/value.rs | 9 ++++++--- src/librustc_mir/const_eval.rs | 16 ++++++++++------ src/librustc_mir/interpret/memory.rs | 6 +++--- src/librustc_mir/transform/const_prop.rs | 2 +- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 5f01b4f6abec9..7cef032b3abd4 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -308,8 +308,10 @@ impl_stable_hash_for!( ); impl_stable_hash_for!(struct crate::mir::interpret::RawConst<'tcx> { + // FIXME(oli-obk): is ignoring the `alloc_id` for perf reasons ok? alloc_id, ty, + alloc, }); impl_stable_hash_for! { diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index ba04b85dce45c..cde6005bc3d6f 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -2,15 +2,18 @@ use std::fmt; use crate::ty::{Ty, layout::{HasDataLayout, Size}}; -use super::{EvalResult, Pointer, PointerArithmetic, AllocId, sign_extend, truncate}; +use super::{EvalResult, Pointer, PointerArithmetic, AllocId, sign_extend, truncate, Allocation}; /// Represents the result of a raw const operation, pre-validation. #[derive(Copy, Clone, Debug, Eq, PartialEq, RustcEncodable, RustcDecodable, Hash)] pub struct RawConst<'tcx> { - // the value lives here, at offset 0, and that allocation definitely is a `AllocKind::Memory` - // (so you can use `AllocMap::unwrap_memory`). + /// the value lives here, at offset 0, and the allocation that it refers to is the one in the + /// `alloc` field pub alloc_id: AllocId, pub ty: Ty<'tcx>, + /// the allocation that would be returned by using + /// `tcx.alloc_map.lock().unwrap_memory(self.alloc_id)` + pub alloc: &'tcx Allocation, } /// Represents a constant value in Rust. `Scalar` and `ScalarPair` are optimizations that diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 33d4113ef4779..118eeb8d87532 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -56,7 +56,7 @@ pub(crate) fn eval_promoted<'a, 'mir, 'tcx>( cid: GlobalId<'tcx>, mir: &'mir mir::Mir<'tcx>, param_env: ty::ParamEnv<'tcx>, -) -> EvalResult<'tcx, MPlaceTy<'tcx>> { +) -> EvalResult<'tcx, (MPlaceTy<'tcx>, &'tcx Allocation)> { let span = tcx.def_span(cid.instance.def_id()); let mut ecx = mk_eval_cx(tcx, span, param_env); eval_body_using_ecx(&mut ecx, cid, Some(mir), param_env) @@ -111,7 +111,10 @@ fn eval_body_and_ecx<'a, 'mir, 'tcx>( cid: GlobalId<'tcx>, mir: Option<&'mir mir::Mir<'tcx>>, param_env: ty::ParamEnv<'tcx>, -) -> (EvalResult<'tcx, MPlaceTy<'tcx>>, CompileTimeEvalContext<'a, 'mir, 'tcx>) { +) -> ( + EvalResult<'tcx, (MPlaceTy<'tcx>, &'tcx Allocation)>, + CompileTimeEvalContext<'a, 'mir, 'tcx>, +) { // we start out with the best span we have // and try improving it down the road when more information is available let span = tcx.def_span(cid.instance.def_id()); @@ -127,7 +130,7 @@ fn eval_body_using_ecx<'mir, 'tcx>( cid: GlobalId<'tcx>, mir: Option<&'mir mir::Mir<'tcx>>, param_env: ty::ParamEnv<'tcx>, -) -> EvalResult<'tcx, MPlaceTy<'tcx>> { +) -> EvalResult<'tcx, (MPlaceTy<'tcx>, &'tcx Allocation)> { debug!("eval_body_using_ecx: {:?}, {:?}", cid, param_env); let tcx = ecx.tcx.tcx; let mut mir = match mir { @@ -164,10 +167,10 @@ fn eval_body_using_ecx<'mir, 'tcx>( } else { Mutability::Immutable }; - ecx.memory.intern_static(ret.ptr.to_ptr()?.alloc_id, mutability)?; + let alloc = ecx.memory.intern_static(ret.ptr.to_ptr()?.alloc_id, mutability)?; debug!("eval_body_using_ecx done: {:?}", *ret); - Ok(ret) + Ok((ret, alloc)) } impl<'tcx> Into> for ConstEvalError { @@ -634,9 +637,10 @@ pub fn const_eval_raw_provider<'a, 'tcx>( }; let (res, ecx) = eval_body_and_ecx(tcx, cid, None, key.param_env); - res.and_then(|place| { + res.and_then(|(place, alloc)| { Ok(RawConst { alloc_id: place.to_ptr().expect("we allocated this ptr!").alloc_id, + alloc, ty: place.layout.ty }) }).map_err(|error| { diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 88b936afaa4c1..c0d580646fe47 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -621,7 +621,7 @@ where &mut self, alloc_id: AllocId, mutability: Mutability, - ) -> EvalResult<'tcx> { + ) -> EvalResult<'tcx, &'tcx Allocation> { trace!( "mark_static_initialized {:?}, mutability: {:?}", alloc_id, @@ -647,7 +647,7 @@ where // does not permit code that would break this! if self.alloc_map.contains_key(&alloc) { // Not yet interned, so proceed recursively - self.intern_static(alloc, mutability)?; + let _alloc = self.intern_static(alloc, mutability)?; } else if self.dead_alloc_map.contains_key(&alloc) { // dangling pointer return err!(ValidationFailure( @@ -655,7 +655,7 @@ where )) } } - Ok(()) + Ok(alloc) } } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 7da00c4ea0c36..7631ac5fd33a6 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -296,7 +296,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { }; // cannot use `const_eval` here, because that would require having the MIR // for the current function available, but we're producing said MIR right now - let res = self.use_ecx(source_info, |this| { + let (res, _) = self.use_ecx(source_info, |this| { eval_promoted(this.tcx, cid, this.mir, this.param_env) })?; trace!("evaluated promoted {:?} to {:?}", promoted, res); From f5fb34cc96938804d80213499e53cb0f2c68b5f9 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 14 Feb 2019 17:27:45 +0100 Subject: [PATCH 03/23] Make const eval not use the hacky `op_to_const` function --- src/librustc_mir/const_eval.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 118eeb8d87532..7e8f41204e6be 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -523,7 +523,6 @@ fn validate_and_turn_into_const<'a, 'tcx>( constant: RawConst<'tcx>, key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>, ) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> { - let cid = key.value; let ecx = mk_eval_cx(tcx, tcx.def_span(key.value.instance.def_id()), key.param_env); let val = (|| { let op = ecx.raw_const_to_mplace(constant)?.into(); @@ -539,9 +538,17 @@ fn validate_and_turn_into_const<'a, 'tcx>( )?; } // Now that we validated, turn this into a proper constant. - let def_id = cid.instance.def.def_id(); - let normalize = tcx.is_static(def_id).is_none() && cid.promoted.is_none(); - op_to_const(&ecx, op, normalize) + let val = match op.layout.abi { + layout::Abi::Scalar(..) => ConstValue::Scalar(ecx.read_immediate(op)?.to_scalar()?), + layout::Abi::ScalarPair(..) if op.layout.ty.is_slice() => { + let (a, b) = ecx.read_immediate(op)?.to_scalar_pair()?; + ConstValue::Slice(a, b.to_usize(&ecx)?) + }, + _ => ConstValue::ByRef, + }; + let ptr = Pointer::from(constant.alloc_id); + let alloc = constant.alloc; + Ok(ty::Const { val, ty: op.layout.ty, alloc: Some((alloc, ptr))}) })(); val.map_err(|error| { From db168bc10cfc8c9dcb39fb7358d6d7dbe46b80ae Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 13:06:56 +0100 Subject: [PATCH 04/23] Fix equality check for constants --- src/librustc/ty/sty.rs | 43 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index 6344539946bdf..699ba8f165394 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -13,6 +13,7 @@ use crate::ty::{List, TyS, ParamEnvAnd, ParamEnv}; use crate::util::captures::Captures; use crate::mir::interpret::{Scalar, Pointer, Allocation}; +use std::hash::{Hash, Hasher}; use smallvec::SmallVec; use std::iter; use std::cmp::Ordering; @@ -2086,7 +2087,7 @@ impl<'tcx> LazyConst<'tcx> { } /// Typed constant value. -#[derive(Copy, Clone, Debug, Hash, RustcEncodable, RustcDecodable, Eq, PartialEq, Ord, PartialOrd)] +#[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable, Eq, Ord, PartialOrd)] pub struct Const<'tcx> { pub ty: Ty<'tcx>, @@ -2104,6 +2105,46 @@ pub struct Const<'tcx> { pub alloc: Option<(&'tcx Allocation, Pointer)>, } +impl<'tcx> PartialEq for Const<'tcx> { + fn eq(&self, other: &Self) -> bool { + + self.ty == other.ty && match (self.val, other.val) { + (ConstValue::ByRef, ConstValue::ByRef) => { + let (a, pa) = self.alloc.unwrap(); + let (b, pb) = other.alloc.unwrap(); + // only use the alloc ids to not have to compare the full allocations + // the ids may differ if the allocation is the same + (pa.offset == pb.offset) && (pa.alloc_id == pb.alloc_id || a == b) + }, + // ignore the actual allocation, just compare the values + (ConstValue::Scalar(a), ConstValue::Scalar(b)) => a == b, + (ConstValue::Slice(a, an), ConstValue::Slice(b, bn)) => an == bn && a == b, + // if the values don't match, the consts can't be equal and the type equality should + // have already failed, because we make the decision for non-byref solely based on the + // type + _ => bug!("same type but different value kind in constant: {:#?} {:#?}", self, other), + } + } +} + +impl<'tcx> Hash for Const<'tcx> { + fn hash(&self, hasher: &mut H) { + let Const { ty, val, alloc } = self; + ty.hash(hasher); + val.hash(hasher); + if let ConstValue::ByRef = val { + let (alloc, ptr) = alloc.unwrap(); + // type check for future changes + let alloc: &'tcx Allocation = alloc; + alloc.hash(hasher); + ptr.offset.hash(hasher); + // do not hash the alloc id in the pointer. It does not add anything new to the hash. + // If the hash of the alloc id is the same, then the hash of the allocation would also + // be the same. + } + } +} + #[cfg(target_arch = "x86_64")] static_assert!(CONST_SIZE: ::std::mem::size_of::>() == 72); From 9a63fc8ffa69a93b4ee48c467d23ddca99714d81 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 13:07:38 +0100 Subject: [PATCH 05/23] Promoteds always have a backing allocation --- src/librustc_codegen_ssa/mir/place.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/place.rs b/src/librustc_codegen_ssa/mir/place.rs index f1674d36287c2..e8ce3ede2abe1 100644 --- a/src/librustc_codegen_ssa/mir/place.rs +++ b/src/librustc_codegen_ssa/mir/place.rs @@ -416,11 +416,8 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }; let layout = cx.layout_of(self.monomorphize(&ty)); match bx.tcx().const_eval(param_env.and(cid)) { - Ok(val) => match val.val { - mir::interpret::ConstValue::ByRef => { - let (alloc, ptr) = val.alloc.unwrap(); - bx.cx().from_const_alloc(layout, alloc, ptr.offset) - } + Ok(val) => match val.alloc { + Some((alloc, ptr)) => bx.cx().from_const_alloc(layout, alloc, ptr.offset), _ => bug!("promoteds should have an allocation: {:?}", val), }, Err(_) => { From 78c36b9982d44c0735e36466b306a18fc7042a7d Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 13:15:00 +0100 Subject: [PATCH 06/23] Fixup: statics are always backed by an allocation --- src/librustc_codegen_llvm/consts.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index 77c677af9b0dd..b80d0556c967a 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -1,7 +1,7 @@ use libc::c_uint; use llvm::{self, SetUnnamedAddr, True}; use rustc::hir::def_id::DefId; -use rustc::mir::interpret::{ConstValue, Allocation, read_target_uint, +use rustc::mir::interpret::{Allocation, read_target_uint, Pointer, ErrorHandled, GlobalId}; use rustc::hir::Node; use debuginfo; @@ -69,12 +69,11 @@ pub fn codegen_static_initializer( }; let param_env = ty::ParamEnv::reveal_all(); let static_ = cx.tcx.const_eval(param_env.and(cid))?; - let (alloc, ptr) = static_.alloc.unwrap(); - assert_eq!(ptr.offset.bytes(), 0); - match static_.val { - ConstValue::ByRef => {}, + let (alloc, ptr) = match static_.alloc { + Some(alloc) => alloc, _ => bug!("static const eval returned {:#?}", static_), - } + }; + assert_eq!(ptr.offset.bytes(), 0); Ok((const_alloc_to_llvm(cx, alloc), alloc)) } From 785302d3d9160ea527cce704370b3a1aef47cc7e Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 15:18:36 +0100 Subject: [PATCH 07/23] Update comment and make it truer by actually doing what it says --- src/librustc/mir/interpret/value.rs | 3 +++ src/librustc_mir/interpret/memory.rs | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index cde6005bc3d6f..477507c9e9583 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -13,6 +13,9 @@ pub struct RawConst<'tcx> { pub ty: Ty<'tcx>, /// the allocation that would be returned by using /// `tcx.alloc_map.lock().unwrap_memory(self.alloc_id)` + /// + /// This is an optimization so we don't actually have to go fetch said allocation from the + /// `alloc_map` at most use sites of the `const_eval_raw` query pub alloc: &'tcx Allocation, } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index c0d580646fe47..bb85f07d06651 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -348,10 +348,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { ErrorHandled::TooGeneric => EvalErrorKind::TooGeneric.into(), } }).map(|raw_const| { - let allocation = tcx.alloc_map.lock().unwrap_memory(raw_const.alloc_id); // We got tcx memory. Let the machine figure out whether and how to // turn that into memory with the right pointer tag. - M::adjust_static_allocation(allocation, memory_extra) + M::adjust_static_allocation(raw_const.alloc, memory_extra) }) } From 8b0dd6de10db7142c1f5912b5d518a35a7bf6cc8 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 15:27:28 +0100 Subject: [PATCH 08/23] Explain the `val` caching in `ty::Const` --- src/librustc_mir/const_eval.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 7e8f41204e6be..35061db7b74d4 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -538,6 +538,14 @@ fn validate_and_turn_into_const<'a, 'tcx>( )?; } // Now that we validated, turn this into a proper constant. + + // We also store a simpler version of certain constants in the `val` field of `ty::Const` + // This helps us reduce the effort required to access e.g. the `usize` constant value for + // array lengths. Since array lengths make up a non-insignificant amount of all of the + // constants in the compiler, this caching has a very noticeable effect. + + // FIXME(oli-obk): see if creating a query to go from an `Allocation` + offset to a + // `ConstValue` is just as effective as proactively generating the `ConstValue`. let val = match op.layout.abi { layout::Abi::Scalar(..) => ConstValue::Scalar(ecx.read_immediate(op)?.to_scalar()?), layout::Abi::ScalarPair(..) if op.layout.ty.is_slice() => { From 6906379b00e84243e3b781cf68fa04baad3665ee Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 15:32:25 +0100 Subject: [PATCH 09/23] Remove a now-dead function argument --- src/librustc_mir/const_eval.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 35061db7b74d4..0c8e56aa813cb 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -66,15 +66,13 @@ pub(crate) fn eval_promoted<'a, 'mir, 'tcx>( pub fn op_to_const<'tcx>( ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, op: OpTy<'tcx>, - may_normalize: bool, ) -> EvalResult<'tcx, ty::Const<'tcx>> { // We do not normalize just any data. Only scalar layout and slices. - let normalize = may_normalize - && match op.layout.abi { - layout::Abi::Scalar(..) => true, - layout::Abi::ScalarPair(..) => op.layout.ty.is_slice(), - _ => false, - }; + let normalize = match op.layout.abi { + layout::Abi::Scalar(..) => true, + layout::Abi::ScalarPair(..) => op.layout.ty.is_slice(), + _ => false, + }; let normalized_op = if normalize { ecx.try_read_immediate(op)? } else { @@ -489,7 +487,7 @@ pub fn const_field<'a, 'tcx>( let field = ecx.operand_field(down, field.index() as u64)?; // and finally move back to the const world, always normalizing because // this is not called for statics. - op_to_const(&ecx, field, true) + op_to_const(&ecx, field) })(); result.map_err(|error| { let err = error_to_const_error(&ecx, error); From 1b93edae74763205e0837e1276c28ce55720b9f2 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 15:32:49 +0100 Subject: [PATCH 10/23] Mark a function private that is unused anywhere else --- src/librustc_mir/const_eval.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 0c8e56aa813cb..d30654e103215 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -63,7 +63,7 @@ pub(crate) fn eval_promoted<'a, 'mir, 'tcx>( } // FIXME: These two conversion functions are bad hacks. We should just always use allocations. -pub fn op_to_const<'tcx>( +fn op_to_const<'tcx>( ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, op: OpTy<'tcx>, ) -> EvalResult<'tcx, ty::Const<'tcx>> { From b3731eec53511c1a26c47b4fdf171334d3a48c89 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 15:34:58 +0100 Subject: [PATCH 11/23] Fold an if on a match returning a bool directly into the match --- src/librustc_mir/const_eval.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index d30654e103215..9a567d700f6d0 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -68,18 +68,13 @@ fn op_to_const<'tcx>( op: OpTy<'tcx>, ) -> EvalResult<'tcx, ty::Const<'tcx>> { // We do not normalize just any data. Only scalar layout and slices. - let normalize = match op.layout.abi { - layout::Abi::Scalar(..) => true, - layout::Abi::ScalarPair(..) => op.layout.ty.is_slice(), - _ => false, - }; - let normalized_op = if normalize { - ecx.try_read_immediate(op)? - } else { - match *op { + let normalized_op = match op.layout.abi { + layout::Abi::Scalar(..) => ecx.try_read_immediate(op)?, + layout::Abi::ScalarPair(..) if op.layout.ty.is_slice() => ecx.try_read_immediate(op)?, + _ => match *op { Operand::Indirect(mplace) => Err(mplace), Operand::Immediate(val) => Ok(val) - } + }, }; let (val, alloc) = match normalized_op { Err(MemPlace { ptr, align, meta }) => { From b0857a71c98c02af34cb3c6538332f249dee0bac Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 15:59:43 +0100 Subject: [PATCH 12/23] Synchronize StableHash with the updated PartialEq impl --- src/librustc/ich/impls_ty.rs | 29 ++++++++++++++++++++++++----- src/librustc/ty/sty.rs | 2 ++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 7cef032b3abd4..cddba5fc1613e 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -375,11 +375,30 @@ impl_stable_hash_for!(enum ::syntax::ast::Mutability { Mutable }); -impl_stable_hash_for!(struct ty::Const<'tcx> { - ty, - alloc, - val -}); + +impl<'a, 'gcx> HashStable> for ty::Const<'gcx> { + fn hash_stable( + &self, + hcx: &mut StableHashingContext<'a>, + hasher: &mut StableHasher, + ) { + let ty::Const { ty, val, alloc } = self; + ty.hash_stable(hcx, hasher); + val.hash_stable(hcx, hasher); + // don't hash the memory for `Scalar` and `Slice`. There's nothing to be gained + // by it. All the relevant info is contained in the value. + if let mir::interpret::ConstValue::ByRef = val { + let (alloc, ptr) = alloc.unwrap(); + // type check for future changes + let alloc: &'gcx mir::interpret::Allocation = alloc; + alloc.hash_stable(hcx, hasher); + ptr.offset.hash_stable(hcx, hasher); + // do not hash the alloc id in the pointer. It does not add anything new to the hash. + // If the hash of the alloc id is the same, then the hash of the allocation would also + // be the same. + } + } +} impl_stable_hash_for!(impl<'tcx> for enum ty::LazyConst<'tcx> [ty::LazyConst] { Unevaluated(did, substs), diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index 699ba8f165394..23e16d49199e3 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -2132,6 +2132,8 @@ impl<'tcx> Hash for Const<'tcx> { let Const { ty, val, alloc } = self; ty.hash(hasher); val.hash(hasher); + // don't hash the memory for `Scalar` and `Slice`. There's nothing to be gained + // by it. All the relevant info is contained in the value. if let ConstValue::ByRef = val { let (alloc, ptr) = alloc.unwrap(); // type check for future changes From 1495b6b7667648e03c2167b11ec44539df353e60 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 17:49:29 +0100 Subject: [PATCH 13/23] Don't create an intermediate place when reading discriminants --- src/librustc_mir/interpret/operand.rs | 5 +++-- src/librustc_mir/interpret/place.rs | 2 +- src/librustc_mir/interpret/step.rs | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index d928ad20f676e..87733d6bb502f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -508,7 +508,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // Evaluate a place with the goal of reading from it. This lets us sometimes // avoid allocations. - fn eval_place_to_op( + pub(super) fn eval_place_to_op( &self, mir_place: &mir::Place<'tcx>, layout: Option>, @@ -522,7 +522,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let op = self.eval_place_to_op(&proj.base, None)?; self.operand_projection(op, &proj.elem)? } - + // FIXME(oli-obk): statics and promoteds have preevaluated `val` fields nowadays + // add nonallocating variants here _ => self.eval_place_to_mplace(mir_place)?.into(), }; diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index b29e09900f6b1..b876384258b06 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -611,7 +611,7 @@ where } /// Computes a place. You should only use this if you intend to write into this - /// place; for reading, a more efficient alternative is `eval_place_for_read`. + /// place; for reading, a more efficient alternative is `eval_place_to_op`. pub fn eval_place( &mut self, mir_place: &mir::Place<'tcx> diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 97ef2b5fa3485..656c13c16d9ed 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -266,8 +266,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } Discriminant(ref place) => { - let place = self.eval_place(place)?; - let discr_val = self.read_discriminant(self.place_to_op(place)?)?.0; + let op = self.eval_place_to_op(place, None)?; + let discr_val = self.read_discriminant(op)?.0; let size = dest.layout.size; self.write_scalar(Scalar::from_uint(discr_val, size), dest)?; } From 92e447175de9cb4a5b7fc732a2ecb7c2b1de09a1 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 19:45:17 +0100 Subject: [PATCH 14/23] Note to self --- src/librustc_mir/const_eval.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 9a567d700f6d0..d5b08f1e133ff 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -486,6 +486,9 @@ pub fn const_field<'a, 'tcx>( })(); result.map_err(|error| { let err = error_to_const_error(&ecx, error); + // FIXME(oli-obk): I believe this is unreachable and we can just ICE here. Since a constant + // is checked for validity before being in a place that could pass it to `const_field`, + // we can't possibly have errors. All fields have already been checked. err.report_as_error(ecx.tcx, "could not access field of constant"); ErrorHandled::Reported }) From 0075868acf049847842c09a22a72436cce4acefb Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 20:27:11 +0100 Subject: [PATCH 15/23] Insert satisfying `op_to_const` bbqing sound here. --- src/librustc_mir/const_eval.rs | 80 ++++++++++++--------------- src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_mir/interpret/place.rs | 2 +- 3 files changed, 36 insertions(+), 48 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index d5b08f1e133ff..9ca75b063951a 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -21,7 +21,7 @@ use syntax::ast::Mutability; use syntax::source_map::{Span, DUMMY_SP}; use crate::interpret::{self, - PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Operand, Immediate, Scalar, Pointer, + PlaceTy, MPlaceTy, OpTy, ImmTy, Scalar, Pointer, RawConst, ConstValue, EvalResult, EvalError, EvalErrorKind, GlobalId, EvalContext, StackPopCleanup, Allocation, AllocId, MemoryKind, @@ -62,43 +62,6 @@ pub(crate) fn eval_promoted<'a, 'mir, 'tcx>( eval_body_using_ecx(&mut ecx, cid, Some(mir), param_env) } -// FIXME: These two conversion functions are bad hacks. We should just always use allocations. -fn op_to_const<'tcx>( - ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, - op: OpTy<'tcx>, -) -> EvalResult<'tcx, ty::Const<'tcx>> { - // We do not normalize just any data. Only scalar layout and slices. - let normalized_op = match op.layout.abi { - layout::Abi::Scalar(..) => ecx.try_read_immediate(op)?, - layout::Abi::ScalarPair(..) if op.layout.ty.is_slice() => ecx.try_read_immediate(op)?, - _ => match *op { - Operand::Indirect(mplace) => Err(mplace), - Operand::Immediate(val) => Ok(val) - }, - }; - let (val, alloc) = match normalized_op { - Err(MemPlace { ptr, align, meta }) => { - // extract alloc-offset pair - assert!(meta.is_none()); - let ptr = ptr.to_ptr()?; - let alloc = ecx.memory.get(ptr.alloc_id)?; - assert!(alloc.align >= align); - assert!(alloc.bytes.len() as u64 - ptr.offset.bytes() >= op.layout.size.bytes()); - let mut alloc = alloc.clone(); - alloc.align = align; - // FIXME shouldn't it be the case that `mark_static_initialized` has already - // interned this? I thought that is the entire point of that `FinishStatic` stuff? - let alloc = ecx.tcx.intern_const_alloc(alloc); - (ConstValue::ByRef, Some((alloc, ptr))) - }, - Ok(Immediate::Scalar(x)) => - (ConstValue::Scalar(x.not_undef()?), None), - Ok(Immediate::ScalarPair(a, b)) => - (ConstValue::Slice(a.not_undef()?, b.to_usize(ecx)?), None), - }; - Ok(ty::Const { val, ty: op.layout.ty, alloc }) -} - fn eval_body_and_ecx<'a, 'mir, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, cid: GlobalId<'tcx>, @@ -471,18 +434,43 @@ pub fn const_field<'a, 'tcx>( trace!("const_field: {:?}, {:?}", field, value); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); let result = (|| { - // get the operand again - let op = ecx.lazy_const_to_op(ty::LazyConst::Evaluated(value), value.ty)?; + let (alloc, ptr) = value.alloc.expect( + "const_field can only be called on aggregates, which should never be created without + a corresponding allocation", + ); + let mplace = MPlaceTy::from_aligned_ptr(ptr, ecx.layout_of(value.ty)?); // downcast let down = match variant { - None => op, - Some(variant) => ecx.operand_downcast(op, variant)? + None => mplace, + Some(variant) => ecx.mplace_downcast(mplace, variant)?, }; // then project - let field = ecx.operand_field(down, field.index() as u64)?; - // and finally move back to the const world, always normalizing because - // this is not called for statics. - op_to_const(&ecx, field) + let field = ecx.mplace_field(down, field.index() as u64)?; + let val = match field.layout.abi { + layout::Abi::Scalar(..) => { + let scalar = ecx.try_read_immediate_from_mplace(field)?.unwrap().to_scalar()?; + ConstValue::Scalar(scalar) + } + layout::Abi::ScalarPair(..) if field.layout.ty.is_slice() => { + let (a, b) = ecx.try_read_immediate_from_mplace(field)?.unwrap().to_scalar_pair()?; + ConstValue::Slice(a, b.to_usize(&ecx)?) + }, + _ => ConstValue::ByRef, + }; + let field_ptr = field.to_ptr().unwrap(); + assert_eq!( + ptr.alloc_id, + field_ptr.alloc_id, + "field access of aggregate moved to different allocation", + ); + Ok(ty::Const { + val, + ty: field.layout.ty, + alloc: Some(( + alloc, + field_ptr, + )), + }) })(); result.map_err(|error| { let err = error_to_const_error(&ecx, error); diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 87733d6bb502f..1f58043de3ae9 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -269,7 +269,7 @@ pub(super) fn from_known_layout<'tcx>( impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { /// Try reading an immediate in memory; this is interesting particularly for ScalarPair. /// Returns `None` if the layout does not permit loading this as a value. - pub(super) fn try_read_immediate_from_mplace( + crate fn try_read_immediate_from_mplace( &self, mplace: MPlaceTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, Option>> { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index b876384258b06..e0f07de723e8b 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -212,7 +212,7 @@ impl<'tcx, Tag> MPlaceTy<'tcx, Tag> { } #[inline] - fn from_aligned_ptr(ptr: Pointer, layout: TyLayout<'tcx>) -> Self { + crate fn from_aligned_ptr(ptr: Pointer, layout: TyLayout<'tcx>) -> Self { MPlaceTy { mplace: MemPlace::from_ptr(ptr, layout.align.abi), layout } } From a13cfdbcb8cacc05b8e5c432bcdb6e124627345c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 21:05:47 +0100 Subject: [PATCH 16/23] Make validity checking use `MPlaceTy` instead of `OpTy` --- src/librustc_mir/const_eval.rs | 23 ++++++++++++----------- src/librustc_mir/interpret/place.rs | 2 +- src/librustc_mir/interpret/validity.rs | 23 +++++++++++------------ 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 9ca75b063951a..3b033b73a6aa6 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -509,13 +509,11 @@ fn validate_and_turn_into_const<'a, 'tcx>( ) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> { let ecx = mk_eval_cx(tcx, tcx.def_span(key.value.instance.def_id()), key.param_env); let val = (|| { - let op = ecx.raw_const_to_mplace(constant)?.into(); - // FIXME: Once the visitor infrastructure landed, change validation to - // work directly on `MPlaceTy`. - let mut ref_tracking = RefTracking::new(op); - while let Some((op, path)) = ref_tracking.todo.pop() { + let mplace = ecx.raw_const_to_mplace(constant)?; + let mut ref_tracking = RefTracking::new(mplace); + while let Some((mplace, path)) = ref_tracking.todo.pop() { ecx.validate_operand( - op, + mplace.into(), path, Some(&mut ref_tracking), true, // const mode @@ -530,17 +528,20 @@ fn validate_and_turn_into_const<'a, 'tcx>( // FIXME(oli-obk): see if creating a query to go from an `Allocation` + offset to a // `ConstValue` is just as effective as proactively generating the `ConstValue`. - let val = match op.layout.abi { - layout::Abi::Scalar(..) => ConstValue::Scalar(ecx.read_immediate(op)?.to_scalar()?), - layout::Abi::ScalarPair(..) if op.layout.ty.is_slice() => { - let (a, b) = ecx.read_immediate(op)?.to_scalar_pair()?; + let val = match mplace.layout.abi { + layout::Abi::Scalar(..) => { + let scalar = ecx.try_read_immediate_from_mplace(mplace)?.unwrap().to_scalar()?; + ConstValue::Scalar(scalar) + } + layout::Abi::ScalarPair(..) if mplace.layout.ty.is_slice() => { + let (a, b) = ecx.try_read_immediate_from_mplace(mplace)?.unwrap().to_scalar_pair()?; ConstValue::Slice(a, b.to_usize(&ecx)?) }, _ => ConstValue::ByRef, }; let ptr = Pointer::from(constant.alloc_id); let alloc = constant.alloc; - Ok(ty::Const { val, ty: op.layout.ty, alloc: Some((alloc, ptr))}) + Ok(ty::Const { val, ty: mplace.layout.ty, alloc: Some((alloc, ptr))}) })(); val.map_err(|error| { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index e0f07de723e8b..a4c08883e1d53 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -58,7 +58,7 @@ impl<'tcx, Tag> ::std::ops::Deref for PlaceTy<'tcx, Tag> { } /// A MemPlace with its layout. Constructing it is only possible in this module. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] pub struct MPlaceTy<'tcx, Tag=()> { mplace: MemPlace, pub layout: TyLayout<'tcx>, diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 8b97d9ded7449..0163d53f027cf 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -11,7 +11,7 @@ use rustc::mir::interpret::{ }; use super::{ - OpTy, Machine, EvalContext, ValueVisitor, + OpTy, Machine, EvalContext, ValueVisitor, MPlaceTy, }; macro_rules! validation_failure { @@ -74,13 +74,13 @@ pub enum PathElem { } /// State for tracking recursive validation of references -pub struct RefTracking<'tcx, Tag> { - pub seen: FxHashSet<(OpTy<'tcx, Tag>)>, - pub todo: Vec<(OpTy<'tcx, Tag>, Vec)>, +pub struct RefTracking { + pub seen: FxHashSet, + pub todo: Vec<(T, Vec)>, } -impl<'tcx, Tag: Copy+Eq+Hash> RefTracking<'tcx, Tag> { - pub fn new(op: OpTy<'tcx, Tag>) -> Self { +impl<'tcx, T: Copy + Eq + Hash> RefTracking { + pub fn new(op: T) -> Self { let mut ref_tracking = RefTracking { seen: FxHashSet::default(), todo: vec![(op, Vec::new())], @@ -151,7 +151,7 @@ struct ValidityVisitor<'rt, 'a: 'rt, 'mir: 'rt, 'tcx: 'a+'rt+'mir, M: Machine<'a /// starts must not be changed! `visit_fields` and `visit_array` rely on /// this stack discipline. path: Vec, - ref_tracking: Option<&'rt mut RefTracking<'tcx, M::PointerTag>>, + ref_tracking: Option<&'rt mut RefTracking>>, const_mode: bool, ecx: &'rt EvalContext<'a, 'mir, 'tcx, M>, } @@ -399,16 +399,15 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // before. Proceed recursively even for integer pointers, no // reason to skip them! They are (recursively) valid for some ZST, // but not for others (e.g., `!` is a ZST). - let op = place.into(); - if ref_tracking.seen.insert(op) { - trace!("Recursing below ptr {:#?}", *op); + if ref_tracking.seen.insert(place) { + trace!("Recursing below ptr {:#?}", *place); // We need to clone the path anyway, make sure it gets created // with enough space for the additional `Deref`. let mut new_path = Vec::with_capacity(self.path.len()+1); new_path.clone_from(&self.path); new_path.push(PathElem::Deref); // Remember to come back to this later. - ref_tracking.todo.push((op, new_path)); + ref_tracking.todo.push((place, new_path)); } } } @@ -598,7 +597,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> &self, op: OpTy<'tcx, M::PointerTag>, path: Vec, - ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>, + ref_tracking: Option<&mut RefTracking>>, const_mode: bool, ) -> EvalResult<'tcx> { trace!("validate_operand: {:?}, {:?}", *op, op.layout.ty); From 981a79e1d2fa2e5f7c1a42bdcb40345fe07b0557 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 21:16:19 +0100 Subject: [PATCH 17/23] Eliminate another use of `lazy_const_to_op` --- src/librustc_mir/const_eval.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 3b033b73a6aa6..bb4c1f42223c9 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -489,8 +489,12 @@ pub fn const_variant_index<'a, 'tcx>( ) -> EvalResult<'tcx, VariantIdx> { trace!("const_variant_index: {:?}", val); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); - let op = ecx.lazy_const_to_op(ty::LazyConst::Evaluated(val), val.ty)?; - Ok(ecx.read_discriminant(op)?.1) + let (_, ptr) = val.alloc.expect( + "const_variant_index can only be called on aggregates, which should never be created without + a corresponding allocation", + ); + let mplace = MPlaceTy::from_aligned_ptr(ptr, ecx.layout_of(val.ty)?); + Ok(ecx.read_discriminant(mplace.into())?.1) } pub fn error_to_const_error<'a, 'mir, 'tcx>( From c07162c1574fec25bda79bdf024d69e2dd78b47a Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 21:16:48 +0100 Subject: [PATCH 18/23] Simplify `raw_const_to_mplace` --- src/librustc_mir/interpret/place.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index a4c08883e1d53..ea57571505148 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -995,11 +995,14 @@ where &self, raw: RawConst<'tcx>, ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - // This must be an allocation in `tcx` - assert!(self.tcx.alloc_map.lock().get(raw.alloc_id).is_some()); + // This must be an allocation in `tcx` and match the `alloc` field + debug_assert_eq!( + self.tcx.alloc_map.lock().unwrap_memory(raw.alloc_id) as *const _, + raw.alloc as *const _, + ); let layout = self.layout_of(raw.ty)?; Ok(MPlaceTy::from_aligned_ptr( - Pointer::new(raw.alloc_id, Size::ZERO).with_default_tag(), + Pointer::from(raw.alloc_id).with_default_tag(), layout, )) } From defa8d5f15d7e177e8a11405e3eb2327b991c3b4 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 21:21:48 +0100 Subject: [PATCH 19/23] Update a comment --- src/librustc_mir/interpret/operand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 1f58043de3ae9..c991d04b0fd2b 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -713,7 +713,7 @@ where M::MemoryMap: AllocMap, Allocation<(), M::AllocExtra>)>, M::AllocExtra: AllocationExtra<(), M::MemoryExtra>, { - // FIXME: CTFE should use allocations, then we can remove this. + /// FIXME: still used by const propagation, do not add new uses of this! pub(crate) fn lazy_const_to_op( &self, cnst: ty::LazyConst<'tcx>, From b653dc2fee6c92de4130d5baa4f10a87b41eb9af Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 21:49:07 +0100 Subject: [PATCH 20/23] Another bad function bites the dust --- src/librustc_mir/interpret/operand.rs | 20 +------------------- src/librustc_mir/transform/const_prop.rs | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index c991d04b0fd2b..2f217d9c4a5d2 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -12,7 +12,7 @@ use rustc::mir::interpret::{ EvalResult, EvalErrorKind, }; use super::{ - EvalContext, Machine, AllocMap, Allocation, AllocationExtra, + EvalContext, Machine, MemPlace, MPlaceTy, PlaceTy, Place, MemoryKind, }; pub use rustc::mir::interpret::ScalarMaybeUndef; @@ -705,21 +705,3 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - -impl<'a, 'mir, 'tcx, M> EvalContext<'a, 'mir, 'tcx, M> -where - M: Machine<'a, 'mir, 'tcx, PointerTag=()>, - // FIXME: Working around https://github.com/rust-lang/rust/issues/24159 - M::MemoryMap: AllocMap, Allocation<(), M::AllocExtra>)>, - M::AllocExtra: AllocationExtra<(), M::MemoryExtra>, -{ - /// FIXME: still used by const propagation, do not add new uses of this! - pub(crate) fn lazy_const_to_op( - &self, - cnst: ty::LazyConst<'tcx>, - ty: ty::Ty<'tcx>, - ) -> EvalResult<'tcx, OpTy<'tcx>> { - let op = self.const_value_to_op(cnst)?; - Ok(OpTy { op, layout: self.layout_of(ty)? }) - } -} diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 7631ac5fd33a6..e1ca8c06495d3 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -3,7 +3,7 @@ use rustc::hir::def::Def; -use rustc::mir::{Constant, Location, Place, Mir, Operand, Rvalue, Local}; +use rustc::mir::{Location, Place, Mir, Operand, Rvalue, Local}; use rustc::mir::{NullOp, UnOp, StatementKind, Statement, BasicBlock, LocalKind}; use rustc::mir::{TerminatorKind, ClearCrossCrate, SourceInfo, BinOp, ProjectionElem}; use rustc::mir::visit::{Visitor, PlaceContext, MutatingUseContext, NonMutatingUseContext}; @@ -249,13 +249,13 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { fn eval_constant( &mut self, - c: &Constant<'tcx>, + op: &Operand<'tcx>, source_info: SourceInfo, ) -> Option> { self.ecx.tcx.span = source_info.span; - match self.ecx.lazy_const_to_op(*c.literal, c.ty) { + match self.ecx.eval_operand(op, None) { Ok(op) => { - Some((op, c.span)) + Some((op, source_info.span)) }, Err(error) => { let err = error_to_const_error(&self.ecx, error); @@ -308,7 +308,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { fn eval_operand(&mut self, op: &Operand<'tcx>, source_info: SourceInfo) -> Option> { match *op { - Operand::Constant(ref c) => self.eval_constant(c, source_info), + Operand::Constant(_) => self.eval_constant(op, source_info), | Operand::Move(ref place) | Operand::Copy(ref place) => self.eval_place(place, source_info), } @@ -532,15 +532,17 @@ impl<'tcx> Visitor<'tcx> for CanConstProp { } impl<'b, 'a, 'tcx> Visitor<'tcx> for ConstPropagator<'b, 'a, 'tcx> { - fn visit_constant( + fn visit_operand( &mut self, - constant: &Constant<'tcx>, + op: &Operand<'tcx>, location: Location, ) { - trace!("visit_constant: {:?}", constant); - self.super_constant(constant, location); + trace!("visit_operand: {:?}", op); + self.super_operand(op, location); let source_info = *self.mir.source_info(location); - self.eval_constant(constant, source_info); + if let Operand::Constant(_) = op { + self.eval_constant(op, source_info); + } } fn visit_statement( From d3d145e230ba50ba1de1290247457fa275cb1856 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 21:58:01 +0100 Subject: [PATCH 21/23] Update an outdated comment --- src/librustc_mir/interpret/operand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 2f217d9c4a5d2..e72fc81f4344b 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -569,7 +569,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> .collect() } - // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. + // Used when Miri runs into a constant. fn const_value_to_op( &self, val: ty::LazyConst<'tcx>, From 1f718b0c737d42720a7ddf2b3e560791ab555673 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 22:44:40 +0100 Subject: [PATCH 22/23] Keep MPlaceTy internals private --- src/librustc_mir/const_eval.rs | 16 ++++------------ src/librustc_mir/interpret/place.rs | 14 +++++++++++++- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index bb4c1f42223c9..ed073f5637e3f 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -434,11 +434,7 @@ pub fn const_field<'a, 'tcx>( trace!("const_field: {:?}, {:?}", field, value); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); let result = (|| { - let (alloc, ptr) = value.alloc.expect( - "const_field can only be called on aggregates, which should never be created without - a corresponding allocation", - ); - let mplace = MPlaceTy::from_aligned_ptr(ptr, ecx.layout_of(value.ty)?); + let (mplace, alloc) = ecx.const_to_mplace(value)?; // downcast let down = match variant { None => mplace, @@ -458,8 +454,8 @@ pub fn const_field<'a, 'tcx>( _ => ConstValue::ByRef, }; let field_ptr = field.to_ptr().unwrap(); - assert_eq!( - ptr.alloc_id, + debug_assert_eq!( + mplace.to_ptr().unwrap().alloc_id, field_ptr.alloc_id, "field access of aggregate moved to different allocation", ); @@ -489,11 +485,7 @@ pub fn const_variant_index<'a, 'tcx>( ) -> EvalResult<'tcx, VariantIdx> { trace!("const_variant_index: {:?}", val); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); - let (_, ptr) = val.alloc.expect( - "const_variant_index can only be called on aggregates, which should never be created without - a corresponding allocation", - ); - let mplace = MPlaceTy::from_aligned_ptr(ptr, ecx.layout_of(val.ty)?); + let (mplace, _) = ecx.const_to_mplace(val)?; Ok(ecx.read_discriminant(mplace.into())?.1) } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index ea57571505148..d83b79c8e8dfc 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -212,7 +212,7 @@ impl<'tcx, Tag> MPlaceTy<'tcx, Tag> { } #[inline] - crate fn from_aligned_ptr(ptr: Pointer, layout: TyLayout<'tcx>) -> Self { + fn from_aligned_ptr(ptr: Pointer, layout: TyLayout<'tcx>) -> Self { MPlaceTy { mplace: MemPlace::from_ptr(ptr, layout.align.abi), layout } } @@ -991,6 +991,18 @@ where Ok(()) } + /// Do not call on compiler generated constants (e.g. constants created via `Const::from_bool`) + pub fn const_to_mplace( + &self, + value: ty::Const<'tcx>, + ) -> EvalResult<'tcx, (MPlaceTy<'tcx, M::PointerTag>, &'tcx Allocation)> { + let (alloc, ptr) = value.alloc.expect( + "const_to_mplace can only be called on user written constants, which should never be + created without a corresponding allocation", + ); + Ok((MPlaceTy::from_aligned_ptr(ptr.with_default_tag(), self.layout_of(value.ty)?), alloc)) + } + pub fn raw_const_to_mplace( &self, raw: RawConst<'tcx>, From 5ef183d5724c25853809b740d754f80079093972 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 22:54:03 +0100 Subject: [PATCH 23/23] Don't access the stack if not strictly necessary --- src/librustc_mir/interpret/operand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index e72fc81f4344b..30b4a2301a1f5 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -548,7 +548,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Constant(ref constant) => { let layout = from_known_layout(layout, || { - let ty = self.monomorphize(mir_op.ty(self.mir(), *self.tcx))?; + let ty = self.monomorphize(constant.ty)?; self.layout_of(ty) })?; let op = self.const_value_to_op(*constant.literal)?;