From 62c0501be8a0fa5e511a45ed9ffbcef42094e9ad Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 10 Apr 2018 16:25:10 +0200 Subject: [PATCH] Stop referring to statics' AllocIds directly --- src/librustc/ich/impls_ty.rs | 3 +- src/librustc/mir/interpret/mod.rs | 21 +--- src/librustc/ty/context.rs | 39 +++--- src/librustc_mir/interpret/const_eval.rs | 136 +++++---------------- src/librustc_mir/interpret/eval_context.rs | 14 +-- src/librustc_mir/monomorphize/collector.rs | 2 +- src/librustc_trans/mir/constant.rs | 15 ++- 7 files changed, 68 insertions(+), 162 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 2425fef7e7f5..99ac5869b296 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -416,8 +416,7 @@ impl<'a> HashStable> for mir::interpret::AllocId { ty::tls::with_opt(|tcx| { trace!("hashing {:?}", *self); let tcx = tcx.expect("can't hash AllocIds during hir lowering"); - if let Some(def_id) = tcx.interpret_interner - .get_corresponding_static_def_id(*self) { + if let Some(def_id) = tcx.interpret_interner.get_static(*self) { AllocDiscriminant::Static.hash_stable(hcx, hasher); trace!("hashing {:?} as static {:?}", *self, def_id); def_id.hash_stable(hcx, hasher); diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 9003cca815ee..07768ac3a3b9 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -158,7 +158,7 @@ impl ::rustc_serialize::UseSpecializedDecodable for AllocId {} enum AllocKind { Alloc, Fn, - ExternStatic, + Static, } pub fn specialized_encode_alloc_id< @@ -173,17 +173,13 @@ pub fn specialized_encode_alloc_id< trace!("encoding {:?} with {:#?}", alloc_id, alloc); AllocKind::Alloc.encode(encoder)?; alloc.encode(encoder)?; - // encode whether this allocation is the root allocation of a static - tcx.interpret_interner - .get_corresponding_static_def_id(alloc_id) - .encode(encoder)?; } else if let Some(fn_instance) = tcx.interpret_interner.get_fn(alloc_id) { trace!("encoding {:?} with {:#?}", alloc_id, fn_instance); AllocKind::Fn.encode(encoder)?; fn_instance.encode(encoder)?; - } else if let Some(did) = tcx.interpret_interner.get_corresponding_static_def_id(alloc_id) { - // extern "C" statics don't have allocations, just encode its def_id - AllocKind::ExternStatic.encode(encoder)?; + } else if let Some(did) = tcx.interpret_interner.get_static(alloc_id) { + // referring to statics doesn't need to know about their allocations, just hash the DefId + AllocKind::Static.encode(encoder)?; did.encode(encoder)?; } else { bug!("alloc id without corresponding allocation: {}", alloc_id); @@ -212,10 +208,6 @@ pub fn specialized_decode_alloc_id< let allocation = tcx.intern_const_alloc(allocation); tcx.interpret_interner.intern_at_reserved(alloc_id, allocation); - if let Some(glob) = Option::::decode(decoder)? { - tcx.interpret_interner.cache(glob, alloc_id); - } - Ok(alloc_id) }, AllocKind::Fn => { @@ -227,12 +219,11 @@ pub fn specialized_decode_alloc_id< cache(decoder, id); Ok(id) }, - AllocKind::ExternStatic => { + AllocKind::Static => { trace!("creating extern static alloc id at"); let did = DefId::decode(decoder)?; - let alloc_id = tcx.interpret_interner.reserve(); + let alloc_id = tcx.interpret_interner.cache_static(did); cache(decoder, alloc_id); - tcx.interpret_interner.cache(did, alloc_id); Ok(alloc_id) }, } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index a508f33db3f7..ed3332f32d02 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -956,18 +956,16 @@ struct InterpretInternerInner<'tcx> { /// Allows obtaining const allocs via a unique identifier alloc_by_id: FxHashMap, - /// Reverse map of `alloc_cache` - global_cache: FxHashMap, + /// Allows obtaining static def ids via a unique id + statics: FxHashMap, /// The AllocId to assign to the next new regular allocation. /// Always incremented, never gets smaller. next_id: interpret::AllocId, - /// Allows checking whether a static already has an allocation - /// - /// This is only important for detecting statics referring to themselves - // FIXME(oli-obk) move it to the EvalContext? - alloc_cache: FxHashMap, + /// Inverse map of `statics` + /// Used so we don't allocate a new pointer every time we need one + static_cache: FxHashMap, /// A cache for basic byte allocations keyed by their contents. This is used to deduplicate /// allocations for string and bytestring literals. @@ -1001,30 +999,25 @@ impl<'tcx> InterpretInterner<'tcx> { self.inner.borrow().alloc_by_id.get(&id).cloned() } - pub fn get_cached( + pub fn cache_static( &self, static_id: DefId, - ) -> Option { - self.inner.borrow().alloc_cache.get(&static_id).cloned() - } - - pub fn cache( - &self, - static_id: DefId, - alloc_id: interpret::AllocId, - ) { - let mut inner = self.inner.borrow_mut(); - inner.global_cache.insert(alloc_id, static_id); - if let Some(old) = inner.alloc_cache.insert(static_id, alloc_id) { - bug!("tried to cache {:?}, but was already existing as {:#?}", static_id, old); + ) -> interpret::AllocId { + if let Some(alloc_id) = self.inner.borrow().static_cache.get(&static_id).cloned() { + return alloc_id; } + let alloc_id = self.reserve(); + let mut inner = self.inner.borrow_mut(); + inner.static_cache.insert(static_id, alloc_id); + inner.statics.insert(alloc_id, static_id); + alloc_id } - pub fn get_corresponding_static_def_id( + pub fn get_static( &self, ptr: interpret::AllocId, ) -> Option { - self.inner.borrow().global_cache.get(&ptr).cloned() + self.inner.borrow().statics.get(&ptr).cloned() } pub fn intern_at_reserved( diff --git a/src/librustc_mir/interpret/const_eval.rs b/src/librustc_mir/interpret/const_eval.rs index 57977b6201a6..954a3dbe5b9a 100644 --- a/src/librustc_mir/interpret/const_eval.rs +++ b/src/librustc_mir/interpret/const_eval.rs @@ -5,7 +5,6 @@ use rustc::mir; use rustc::ty::{self, TyCtxt, Ty, Instance}; use rustc::ty::layout::{self, LayoutOf}; use rustc::ty::subst::Subst; -use rustc::util::nodemap::FxHashSet; use syntax::ast::Mutability; use syntax::codemap::Span; @@ -110,53 +109,38 @@ fn eval_body_and_ecx<'a, 'mir, 'tcx>( } span = mir.span; let layout = ecx.layout_of(mir.return_ty().subst(tcx, cid.instance.substs))?; - let alloc = tcx.interpret_interner.get_cached(cid.instance.def_id()); - let is_static = tcx.is_static(cid.instance.def_id()).is_some(); - let alloc = match alloc { - Some(alloc) => { - assert!(cid.promoted.is_none()); - assert!(param_env.caller_bounds.is_empty()); - alloc - }, - None => { - assert!(!layout.is_unsized()); - let ptr = ecx.memory.allocate( - layout.size.bytes(), - layout.align, - None, - )?; - if is_static { - tcx.interpret_interner.cache(cid.instance.def_id(), ptr.alloc_id); - } - let internally_mutable = !layout.ty.is_freeze(tcx, param_env, mir.span); - let mutability = tcx.is_static(cid.instance.def_id()); - let mutability = if mutability == Some(hir::Mutability::MutMutable) || internally_mutable { - Mutability::Mutable - } else { - Mutability::Immutable - }; - let cleanup = StackPopCleanup::MarkStatic(mutability); - let name = ty::tls::with(|tcx| tcx.item_path_str(cid.instance.def_id())); - let prom = cid.promoted.map_or(String::new(), |p| format!("::promoted[{:?}]", p)); - trace!("const_eval: pushing stack frame for global: {}{}", name, prom); - assert!(mir.arg_count == 0); - ecx.push_stack_frame( - cid.instance, - mir.span, - mir, - Place::from_ptr(ptr, layout.align), - cleanup, - )?; - - while ecx.step()? {} - ptr.alloc_id - } + assert!(!layout.is_unsized()); + let ptr = ecx.memory.allocate( + layout.size.bytes(), + layout.align, + None, + )?; + let internally_mutable = !layout.ty.is_freeze(tcx, param_env, mir.span); + let mutability = tcx.is_static(cid.instance.def_id()); + let mutability = if mutability == Some(hir::Mutability::MutMutable) || internally_mutable { + Mutability::Mutable + } else { + Mutability::Immutable }; - let ptr = MemoryPointer::new(alloc, 0).into(); + let cleanup = StackPopCleanup::MarkStatic(mutability); + let name = ty::tls::with(|tcx| tcx.item_path_str(cid.instance.def_id())); + let prom = cid.promoted.map_or(String::new(), |p| format!("::promoted[{:?}]", p)); + trace!("const_eval: pushing stack frame for global: {}{}", name, prom); + assert!(mir.arg_count == 0); + ecx.push_stack_frame( + cid.instance, + mir.span, + mir, + Place::from_ptr(ptr, layout.align), + cleanup, + )?; + + while ecx.step()? {} + let ptr = ptr.into(); // always try to read the value and report errors let value = match ecx.try_read_value(ptr, layout.align, layout.ty)? { // if it's a constant (so it needs no address, directly compute its value) - Some(val) if !is_static => val, + Some(val) if tcx.is_static(cid.instance.def_id()).is_none() => val, // point at the allocation _ => Value::ByRef(ptr, layout.align), }; @@ -340,21 +324,10 @@ impl<'mir, 'tcx> super::Machine<'mir, 'tcx> for CompileTimeEvaluator { ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, cid: GlobalId<'tcx>, ) -> EvalResult<'tcx, AllocId> { - let alloc = ecx - .tcx - .interpret_interner - .get_cached(cid.instance.def_id()); - // Don't evaluate when already cached to prevent cycles - if let Some(alloc) = alloc { - return Ok(alloc) - } - // ensure the static is computed - ecx.const_eval(cid)?; Ok(ecx .tcx .interpret_interner - .get_cached(cid.instance.def_id()) - .expect("uncached static")) + .cache_static(cid.instance.def_id())) } fn box_alloc<'a>( @@ -460,16 +433,7 @@ pub fn const_eval_provider<'a, 'tcx>( let def_id = cid.instance.def.def_id(); if tcx.is_foreign_item(def_id) { - let id = tcx.interpret_interner.get_cached(def_id); - let id = match id { - // FIXME: due to caches this shouldn't happen, add some assertions - Some(id) => id, - None => { - let id = tcx.interpret_interner.reserve(); - tcx.interpret_interner.cache(def_id, id); - id - }, - }; + let id = tcx.interpret_interner.cache_static(def_id); let ty = tcx.type_of(def_id); let layout = tcx.layout_of(key.param_env.and(ty)).unwrap(); let ptr = MemoryPointer::new(id, 0); @@ -505,13 +469,7 @@ pub fn const_eval_provider<'a, 'tcx>( }; let (res, ecx) = eval_body_and_ecx(tcx, cid, None, key.param_env); - res.map(|(miri_value, ptr, miri_ty)| { - if tcx.is_static(def_id).is_some() { - if let Ok(ptr) = ptr.primval.to_ptr() { - let mut seen = FxHashSet::default(); - create_depgraph_edges(tcx, ptr.alloc_id, &mut seen); - } - } + res.map(|(miri_value, _, miri_ty)| { tcx.mk_const(ty::Const { val: ConstVal::Value(miri_value), ty: miri_ty, @@ -528,35 +486,3 @@ pub fn const_eval_provider<'a, 'tcx>( } }) } - -// This function creates dep graph edges from statics to all referred to statics. -// This is necessary, because the `const_eval` query cannot directly call itself -// for other statics, because we cannot prevent recursion in queries. -// -// see test/incremental/static_refering_to_other_static2/issue.rs for an example -// where not creating those edges would cause static A, which refers to static B -// to point to the old allocation of static B, even though B has changed. -// -// In the future we will want to remove this funcion in favour of a system that -// makes sure that statics don't need to have edges to other statics as long as -// they are only referring by reference and not inspecting the other static's body. -fn create_depgraph_edges<'a, 'tcx>( - tcx: TyCtxt<'a, 'tcx, 'tcx>, - alloc_id: AllocId, - seen: &mut FxHashSet, -) { - trace!("create_depgraph_edges: {:?}, {:?}", alloc_id, seen); - if seen.insert(alloc_id) { - trace!("seen: {:?}, {:?}", alloc_id, seen); - if let Some(alloc) = tcx.interpret_interner.get_alloc(alloc_id) { - trace!("get_alloc: {:?}, {:?}, {:?}", alloc_id, seen, alloc); - for (_, &reloc) in &alloc.relocations { - if let Some(did) = tcx.interpret_interner.get_corresponding_static_def_id(reloc) { - trace!("get_corresponding: {:?}, {:?}, {:?}, {:?}, {:?}", alloc_id, seen, alloc, did, reloc); - let _ = tcx.maybe_optimized_mir(did); - } - create_depgraph_edges(tcx, reloc, seen); - } - } - } -} diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 58ea8d48e971..158d674580b5 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -938,16 +938,14 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M } pub fn read_global_as_value(&self, gid: GlobalId<'tcx>, ty: Ty<'tcx>) -> EvalResult<'tcx, Value> { - if gid.promoted.is_none() { - let cached = self + if self.tcx.is_static(gid.instance.def_id()).is_some() { + let alloc_id = self .tcx .interpret_interner - .get_cached(gid.instance.def_id()); - if let Some(alloc_id) = cached { - let layout = self.layout_of(ty)?; - let ptr = MemoryPointer::new(alloc_id, 0); - return Ok(Value::ByRef(ptr.into(), layout.align)) - } + .cache_static(gid.instance.def_id()); + let layout = self.layout_of(ty)?; + let ptr = MemoryPointer::new(alloc_id, 0); + return Ok(Value::ByRef(ptr.into(), layout.align)) } let cv = self.const_eval(gid)?; self.const_to_value(&cv.val, ty) diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 83ef28e4f156..008165f33b2b 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -1142,7 +1142,7 @@ fn collect_miri<'a, 'tcx>( alloc_id: AllocId, output: &mut Vec>, ) { - if let Some(did) = tcx.interpret_interner.get_corresponding_static_def_id(alloc_id) { + if let Some(did) = tcx.interpret_interner.get_static(alloc_id) { let instance = Instance::mono(tcx, did); if should_monomorphize_locally(tcx, &instance) { trace!("collecting static {:?}", did); diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index 977c7c983d6f..6e07b8e73ef2 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -50,7 +50,7 @@ pub fn primval_to_llvm(cx: &CodegenCx, let static_ = cx .tcx .interpret_interner - .get_corresponding_static_def_id(ptr.alloc_id); + .get_static(ptr.alloc_id); let base_addr = if let Some(def_id) = static_ { assert!(cx.tcx.is_static(def_id).is_some()); consts::get_static(cx, def_id) @@ -126,18 +126,17 @@ pub fn trans_static_initializer<'a, 'tcx>( promoted: None }; let param_env = ty::ParamEnv::reveal_all(); - cx.tcx.const_eval(param_env.and(cid))?; + let static_ = cx.tcx.const_eval(param_env.and(cid))?; - let alloc_id = cx - .tcx - .interpret_interner - .get_cached(def_id) - .expect("global not cached"); + let ptr = match static_.val { + ConstVal::Value(MiriValue::ByRef(ptr, _)) => ptr, + _ => bug!("static const eval returned {:#?}", static_), + }; let alloc = cx .tcx .interpret_interner - .get_alloc(alloc_id) + .get_alloc(ptr.primval.to_ptr().expect("static has integer pointer").alloc_id) .expect("miri allocation never successfully created"); Ok(global_initializer(cx, alloc)) }