Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some cleanups around AllocId management #56461

Merged
merged 11 commits into from
Dec 13, 2018
2 changes: 1 addition & 1 deletion src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ impl_stable_hash_for!(
);

impl_stable_hash_for!(
impl<'tcx, M> for enum mir::interpret::AllocType<'tcx, M> [ mir::interpret::AllocType ] {
impl<'tcx> for enum mir::interpret::AllocKind<'tcx> [ mir::interpret::AllocKind ] {
Function(instance),
Static(def_id),
Memory(mem),
Expand Down
116 changes: 69 additions & 47 deletions src/librustc/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ use ty::{self, TyCtxt, Instance};
use ty::layout::{self, Size};
use middle::region;
use std::io;
use std::hash::Hash;
use rustc_serialize::{Encoder, Decodable, Encodable};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::{Lock as Mutex, HashMapExt};
Expand Down Expand Up @@ -90,7 +89,7 @@ impl ::rustc_serialize::UseSpecializedEncodable for AllocId {}
impl ::rustc_serialize::UseSpecializedDecodable for AllocId {}

#[derive(RustcDecodable, RustcEncodable)]
enum AllocKind {
enum AllocDiscriminant {
Alloc,
Fn,
Static,
Expand All @@ -104,23 +103,23 @@ pub fn specialized_encode_alloc_id<
tcx: TyCtxt<'a, 'tcx, 'tcx>,
alloc_id: AllocId,
) -> Result<(), E::Error> {
let alloc_type: AllocType<'tcx, &'tcx Allocation> =
let alloc_kind: AllocKind<'tcx> =
tcx.alloc_map.lock().get(alloc_id).expect("no value for AllocId");
match alloc_type {
AllocType::Memory(alloc) => {
match alloc_kind {
AllocKind::Memory(alloc) => {
trace!("encoding {:?} with {:#?}", alloc_id, alloc);
AllocKind::Alloc.encode(encoder)?;
AllocDiscriminant::Alloc.encode(encoder)?;
alloc.encode(encoder)?;
}
AllocType::Function(fn_instance) => {
AllocKind::Function(fn_instance) => {
trace!("encoding {:?} with {:#?}", alloc_id, fn_instance);
AllocKind::Fn.encode(encoder)?;
AllocDiscriminant::Fn.encode(encoder)?;
fn_instance.encode(encoder)?;
}
AllocType::Static(did) => {
AllocKind::Static(did) => {
// referring to statics doesn't need to know about their allocations,
// just about its DefId
AllocKind::Static.encode(encoder)?;
AllocDiscriminant::Static.encode(encoder)?;
did.encode(encoder)?;
}
}
Expand Down Expand Up @@ -189,10 +188,10 @@ impl<'s> AllocDecodingSession<'s> {
let idx = decoder.read_u32()? as usize;
let pos = self.state.data_offsets[idx] as usize;

// Decode the AllocKind now so that we know if we have to reserve an
// Decode the AllocDiscriminant now so that we know if we have to reserve an
// AllocId.
let (alloc_kind, pos) = decoder.with_position(pos, |decoder| {
let alloc_kind = AllocKind::decode(decoder)?;
let alloc_kind = AllocDiscriminant::decode(decoder)?;
Ok((alloc_kind, decoder.position()))
})?;

Expand All @@ -208,7 +207,7 @@ impl<'s> AllocDecodingSession<'s> {
ref mut entry @ State::Empty => {
// We are allowed to decode
match alloc_kind {
AllocKind::Alloc => {
AllocDiscriminant::Alloc => {
// If this is an allocation, we need to reserve an
// AllocId so we can decode cyclic graphs.
let alloc_id = decoder.tcx().alloc_map.lock().reserve();
Expand All @@ -217,7 +216,7 @@ impl<'s> AllocDecodingSession<'s> {
alloc_id);
Some(alloc_id)
},
AllocKind::Fn | AllocKind::Static => {
AllocDiscriminant::Fn | AllocDiscriminant::Static => {
// Fns and statics cannot be cyclic and their AllocId
// is determined later by interning
*entry = State::InProgressNonAlloc(
Expand Down Expand Up @@ -251,23 +250,23 @@ impl<'s> AllocDecodingSession<'s> {
// Now decode the actual data
let alloc_id = decoder.with_position(pos, |decoder| {
match alloc_kind {
AllocKind::Alloc => {
AllocDiscriminant::Alloc => {
let allocation = <&'tcx Allocation as Decodable>::decode(decoder)?;
// We already have a reserved AllocId.
let alloc_id = alloc_id.unwrap();
trace!("decoded alloc {:?} {:#?}", alloc_id, allocation);
decoder.tcx().alloc_map.lock().set_id_same_memory(alloc_id, allocation);
decoder.tcx().alloc_map.lock().set_alloc_id_same_memory(alloc_id, allocation);
Ok(alloc_id)
},
AllocKind::Fn => {
AllocDiscriminant::Fn => {
assert!(alloc_id.is_none());
trace!("creating fn alloc id");
let instance = ty::Instance::decode(decoder)?;
trace!("decoded fn alloc instance: {:?}", instance);
let alloc_id = decoder.tcx().alloc_map.lock().create_fn_alloc(instance);
Ok(alloc_id)
},
AllocKind::Static => {
AllocDiscriminant::Static => {
assert!(alloc_id.is_none());
trace!("creating extern static alloc id at");
let did = DefId::decode(decoder)?;
Expand All @@ -292,39 +291,42 @@ impl fmt::Display for AllocId {
}

#[derive(Debug, Clone, Eq, PartialEq, Hash, RustcDecodable, RustcEncodable)]
pub enum AllocType<'tcx, M> {
pub enum AllocKind<'tcx> {
/// The alloc id is used as a function pointer
Function(Instance<'tcx>),
/// The alloc id points to a "lazy" static variable that did not get computed (yet).
/// This is also used to break the cycle in recursive statics.
Static(DefId),
/// The alloc id points to memory
Memory(M)
Memory(&'tcx Allocation),
}

pub struct AllocMap<'tcx, M> {
pub struct AllocMap<'tcx> {
/// Lets you know what an AllocId refers to
id_to_type: FxHashMap<AllocId, AllocType<'tcx, M>>,
id_to_kind: FxHashMap<AllocId, AllocKind<'tcx>>,

/// Used to ensure that functions and statics only get one associated AllocId
type_interner: FxHashMap<AllocType<'tcx, M>, AllocId>,
/// Used to ensure that statics only get one associated AllocId
type_interner: FxHashMap<AllocKind<'tcx>, AllocId>,

/// The AllocId to assign to the next requested id.
/// Always incremented, never gets smaller.
next_id: AllocId,
}

impl<'tcx, M: fmt::Debug + Eq + Hash + Clone> AllocMap<'tcx, M> {
impl<'tcx> AllocMap<'tcx> {
pub fn new() -> Self {
AllocMap {
id_to_type: Default::default(),
id_to_kind: Default::default(),
type_interner: Default::default(),
next_id: AllocId(0),
}
}

/// obtains a new allocation ID that can be referenced but does not
/// Obtains a new allocation ID that can be referenced but does not
/// yet have an allocation backing it.
///
/// Make sure to call `set_alloc_id_memory` or `set_alloc_id_same_memory` before returning such
/// an `AllocId` from a query.
pub fn reserve(
&mut self,
) -> AllocId {
Expand All @@ -337,53 +339,73 @@ impl<'tcx, M: fmt::Debug + Eq + Hash + Clone> AllocMap<'tcx, M> {
next
}

fn intern(&mut self, alloc_type: AllocType<'tcx, M>) -> AllocId {
if let Some(&alloc_id) = self.type_interner.get(&alloc_type) {
fn intern(&mut self, alloc_kind: AllocKind<'tcx>) -> AllocId {
if let Some(&alloc_id) = self.type_interner.get(&alloc_kind) {
return alloc_id;
}
let id = self.reserve();
debug!("creating alloc_type {:?} with id {}", alloc_type, id);
self.id_to_type.insert(id, alloc_type.clone());
self.type_interner.insert(alloc_type, id);
debug!("creating alloc_kind {:?} with id {}", alloc_kind, id);
self.id_to_kind.insert(id, alloc_kind.clone());
self.type_interner.insert(alloc_kind, id);
id
}

// FIXME: Check if functions have identity. If not, we should not intern these,
// but instead create a new id per use.
// Alternatively we could just make comparing function pointers an error.
/// Functions cannot be identified by pointers, as asm-equal functions can get deduplicated
/// by the linker and functions can be duplicated across crates.
/// We thus generate a new `AllocId` for every mention of a function. This means that
/// `main as fn() == main as fn()` is false, while `let x = main as fn(); x == x` is true.
pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> AllocId {
self.intern(AllocType::Function(instance))
let id = self.reserve();
self.id_to_kind.insert(id, AllocKind::Function(instance));
id
}

pub fn get(&self, id: AllocId) -> Option<AllocType<'tcx, M>> {
self.id_to_type.get(&id).cloned()
/// Returns `None` in case the `AllocId` is dangling. An `EvalContext` can still have a
/// local `Allocation` for that `AllocId`, but having such an `AllocId` in a constant is
/// illegal and will likely ICE.
/// This function exists to allow const eval to detect the difference between evaluation-
/// local dangling pointers and allocations in constants/statics.
pub fn get(&self, id: AllocId) -> Option<AllocKind<'tcx>> {
self.id_to_kind.get(&id).cloned()
}

pub fn unwrap_memory(&self, id: AllocId) -> M {
/// Panics if the `AllocId` does not refer to an `Allocation`
pub fn unwrap_memory(&self, id: AllocId) -> &'tcx Allocation {
match self.get(id) {
Some(AllocType::Memory(mem)) => mem,
Some(AllocKind::Memory(mem)) => mem,
_ => bug!("expected allocation id {} to point to memory", id),
}
}

/// Generate an `AllocId` for a static or return a cached one in case this function has been
/// called on the same static before.
pub fn intern_static(&mut self, static_id: DefId) -> AllocId {
self.intern(AllocType::Static(static_id))
self.intern(AllocKind::Static(static_id))
}

pub fn allocate(&mut self, mem: M) -> AllocId {
/// Intern the `Allocation` and return a new `AllocId`, even if there's already an identical
/// `Allocation` with a different `AllocId`.
// FIXME: is this really necessary? Can we ensure `FOO` and `BAR` being different after codegen
// in `static FOO: u32 = 42; static BAR: u32 = 42;` even if they reuse the same allocation
// inside rustc?
pub fn allocate(&mut self, mem: &'tcx Allocation) -> AllocId {
let id = self.reserve();
self.set_id_memory(id, mem);
self.set_alloc_id_memory(id, mem);
id
}

pub fn set_id_memory(&mut self, id: AllocId, mem: M) {
if let Some(old) = self.id_to_type.insert(id, AllocType::Memory(mem)) {
/// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. Trying to
/// call this function twice, even with the same `Allocation` will ICE the compiler.
pub fn set_alloc_id_memory(&mut self, id: AllocId, mem: &'tcx Allocation) {
if let Some(old) = self.id_to_kind.insert(id, AllocKind::Memory(mem)) {
bug!("tried to set allocation id {}, but it was already existing as {:#?}", id, old);
}
}

pub fn set_id_same_memory(&mut self, id: AllocId, mem: M) {
self.id_to_type.insert_same(id, AllocType::Memory(mem));
/// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. May be called
/// twice for the same `(AllocId, Allocation)` pair.
fn set_alloc_id_same_memory(&mut self, id: AllocId, mem: &'tcx Allocation) {
self.id_to_kind.insert_same(id, AllocKind::Memory(mem));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use super::{EvalResult, Pointer, PointerArithmetic, Allocation, AllocId, sign_ex
/// 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 `AllocType::Memory`
// the value lives here, at offset 0, and that allocation definitely is a `AllocKind::Memory`
// (so you can use `AllocMap::unwrap_memory`).
pub alloc_id: AllocId,
pub ty: Ty<'tcx>,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2629,7 +2629,7 @@ pub fn fmt_const_val(f: &mut impl Write, const_val: &ty::Const<'_>) -> fmt::Resu
if let Ref(_, &ty::TyS { sty: Str, .. }, _) = ty.sty {
return ty::tls::with(|tcx| {
let alloc = tcx.alloc_map.lock().get(ptr.alloc_id);
if let Some(interpret::AllocType::Memory(alloc)) = alloc {
if let Some(interpret::AllocKind::Memory(alloc)) = alloc {
assert_eq!(len as usize as u128, len);
let slice =
&alloc.bytes[(ptr.offset.bytes() as usize)..][..(len as usize)];
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ pub struct GlobalCtxt<'tcx> {
/// Stores the value of constants (and deduplicates the actual memory)
allocation_interner: Lock<FxHashMap<&'tcx Allocation, ()>>,

pub alloc_map: Lock<interpret::AllocMap<'tcx, &'tcx Allocation>>,
pub alloc_map: Lock<interpret::AllocMap<'tcx>>,

layout_interner: Lock<FxHashMap<&'tcx LayoutDetails, ()>>,

Expand Down
12 changes: 6 additions & 6 deletions src/librustc_codegen_llvm/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use value::Value;
use rustc_codegen_ssa::traits::*;

use rustc::ty::layout::{HasDataLayout, LayoutOf, self, TyLayout, Size};
use rustc::mir::interpret::{Scalar, AllocType, Allocation};
use rustc::mir::interpret::{Scalar, AllocKind, Allocation};
use consts::const_alloc_to_llvm;
use rustc_codegen_ssa::mir::place::PlaceRef;

Expand Down Expand Up @@ -316,20 +316,20 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
}
},
Scalar::Ptr(ptr) => {
let alloc_type = self.tcx.alloc_map.lock().get(ptr.alloc_id);
let base_addr = match alloc_type {
Some(AllocType::Memory(alloc)) => {
let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id);
let base_addr = match alloc_kind {
Some(AllocKind::Memory(alloc)) => {
let init = const_alloc_to_llvm(self, alloc);
if alloc.mutability == Mutability::Mutable {
self.static_addr_of_mut(init, alloc.align, None)
} else {
self.static_addr_of(init, alloc.align, None)
}
}
Some(AllocType::Function(fn_instance)) => {
Some(AllocKind::Function(fn_instance)) => {
self.get_fn(fn_instance)
}
Some(AllocType::Static(def_id)) => {
Some(AllocKind::Static(def_id)) => {
assert!(self.tcx.is_static(def_id).is_some());
self.get_static(def_id)
}
Expand Down
Loading