Skip to content

[beta] backport various PRs #50027

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

Merged
merged 16 commits into from
Apr 19, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Stop referring to statics' AllocIds directly
  • Loading branch information
oli-obk committed Apr 17, 2018
commit 170ea6500a3643fa673b511a32d32f8f66cbba17
3 changes: 1 addition & 2 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,7 @@ impl<'a> HashStable<StableHashingContext<'a>> 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);
Expand Down
21 changes: 6 additions & 15 deletions src/librustc/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl ::rustc_serialize::UseSpecializedDecodable for AllocId {}
enum AllocKind {
Alloc,
Fn,
ExternStatic,
Static,
}

pub fn specialized_encode_alloc_id<
Expand All @@ -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);
Expand Down Expand Up @@ -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::<DefId>::decode(decoder)? {
tcx.interpret_interner.cache(glob, alloc_id);
}

Ok(alloc_id)
},
AllocKind::Fn => {
Expand All @@ -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)
},
}
Expand Down
39 changes: 16 additions & 23 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,18 +935,16 @@ struct InterpretInternerInner<'tcx> {
/// Allows obtaining const allocs via a unique identifier
alloc_by_id: FxHashMap<interpret::AllocId, &'tcx interpret::Allocation>,

/// Reverse map of `alloc_cache`
global_cache: FxHashMap<interpret::AllocId, DefId>,
/// Allows obtaining static def ids via a unique id
statics: FxHashMap<interpret::AllocId, DefId>,

/// 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<DefId, interpret::AllocId>,
/// Inverse map of `statics`
/// Used so we don't allocate a new pointer every time we need one
static_cache: FxHashMap<DefId, interpret::AllocId>,

/// A cache for basic byte allocations keyed by their contents. This is used to deduplicate
/// allocations for string and bytestring literals.
Expand Down Expand Up @@ -980,30 +978,25 @@ impl<'tcx> InterpretInterner<'tcx> {
self.inner.borrow().alloc_by_id.get(&id).cloned()
}

pub fn get_cached(
&self,
static_id: DefId,
) -> Option<interpret::AllocId> {
self.inner.borrow().alloc_cache.get(&static_id).cloned()
}

pub fn cache(
pub fn cache_static(
&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<DefId> {
self.inner.borrow().global_cache.get(&ptr).cloned()
self.inner.borrow().statics.get(&ptr).cloned()
}

pub fn intern_at_reserved(
Expand Down
136 changes: 31 additions & 105 deletions src/librustc_mir/interpret/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
};
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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<AllocId>,
) {
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);
}
}
}
}
14 changes: 6 additions & 8 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -929,16 +929,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)
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,7 @@ fn collect_miri<'a, 'tcx>(
alloc_id: AllocId,
output: &mut Vec<MonoItem<'tcx>>,
) {
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);
Expand Down
15 changes: 7 additions & 8 deletions src/librustc_trans/mir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
}
Expand Down