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

Change vtable memory representation to use tcx allocated allocations. #86475

Merged
merged 3 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ pub(crate) struct FunctionCx<'m, 'clif, 'tcx: 'm> {
pub(crate) module: &'m mut dyn Module,
pub(crate) tcx: TyCtxt<'tcx>,
pub(crate) pointer_type: Type, // Cached from module
pub(crate) vtables: FxHashMap<(Ty<'tcx>, Option<ty::PolyExistentialTraitRef<'tcx>>), DataId>,
pub(crate) vtables: FxHashMap<(Ty<'tcx>, Option<ty::PolyExistentialTraitRef<'tcx>>), Pointer>,
pub(crate) constants_cx: ConstantCx,

pub(crate) instance: Instance<'tcx>,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ pub(crate) fn codegen_const_value<'tcx>(
}
}

fn pointer_for_allocation<'tcx>(
pub(crate) fn pointer_for_allocation<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
alloc: &'tcx Allocation,
) -> crate::pointer::Pointer {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ mod prelude {
pub(crate) use cranelift_codegen::isa::{self, CallConv};
pub(crate) use cranelift_codegen::Context;
pub(crate) use cranelift_frontend::{FunctionBuilder, FunctionBuilderContext, Variable};
pub(crate) use cranelift_module::{self, DataContext, DataId, FuncId, Linkage, Module};
pub(crate) use cranelift_module::{self, DataContext, FuncId, Linkage, Module};

pub(crate) use crate::abi::*;
pub(crate) use crate::base::{codegen_operand, codegen_place};
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_cranelift/src/unsize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ pub(crate) fn unsized_info<'tcx>(
// change to the vtable.
old_info.expect("unsized_info: missing old info for trait upcast")
}
(_, &ty::Dynamic(ref data, ..)) => {
crate::vtable::get_vtable(fx, fx.layout_of(source), data.principal())
}
(_, &ty::Dynamic(ref data, ..)) => crate::vtable::get_vtable(fx, source, data.principal()),
crlf0710 marked this conversation as resolved.
Show resolved Hide resolved
_ => bug!("unsized_info: invalid unsizing {:?} -> {:?}", source, target),
}
}
Expand Down
106 changes: 10 additions & 96 deletions compiler/rustc_codegen_cranelift/src/vtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// FIXME dedup this logic between miri, cg_llvm and cg_clif

use crate::prelude::*;
use ty::VtblEntry;
use super::constant::pointer_for_allocation;

fn vtable_memflags() -> MemFlags {
let mut flags = MemFlags::trusted(); // A vtable access is always aligned and will never trap.
Expand Down Expand Up @@ -66,105 +66,19 @@ pub(crate) fn get_ptr_and_method_ref<'tcx>(

pub(crate) fn get_vtable<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
layout: TyAndLayout<'tcx>,
ty: Ty<'tcx>,
trait_ref: Option<ty::PolyExistentialTraitRef<'tcx>>,
) -> Value {
let data_id = if let Some(data_id) = fx.vtables.get(&(layout.ty, trait_ref)) {
*data_id
let vtable_ptr = if let Some(vtable_ptr) = fx.vtables.get(&(ty, trait_ref)) {
*vtable_ptr
} else {
let data_id = build_vtable(fx, layout, trait_ref);
fx.vtables.insert((layout.ty, trait_ref), data_id);
data_id
};

let local_data_id = fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
fx.bcx.ins().global_value(fx.pointer_type, local_data_id)
}

fn build_vtable<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
layout: TyAndLayout<'tcx>,
trait_ref: Option<ty::PolyExistentialTraitRef<'tcx>>,
) -> DataId {
let tcx = fx.tcx;
let usize_size = fx.layout_of(fx.tcx.types.usize).size.bytes() as usize;
let vtable_alloc_id = fx.tcx.vtable_allocation(ty, trait_ref);
let vtable_allocation = fx.tcx.global_alloc(vtable_alloc_id).unwrap_memory();
let vtable_ptr = pointer_for_allocation(fx, vtable_allocation);

let drop_in_place_fn = import_function(
tcx,
fx.module,
Instance::resolve_drop_in_place(tcx, layout.ty).polymorphize(fx.tcx),
);

let vtable_entries = if let Some(trait_ref) = trait_ref {
tcx.vtable_entries(trait_ref.with_self_ty(tcx, layout.ty))
} else {
ty::COMMON_VTABLE_ENTRIES
fx.vtables.insert((ty, trait_ref), vtable_ptr);
vtable_ptr
};

let mut data_ctx = DataContext::new();
let mut data = ::std::iter::repeat(0u8)
.take(vtable_entries.len() * usize_size)
.collect::<Vec<u8>>()
.into_boxed_slice();

for (idx, entry) in vtable_entries.iter().enumerate() {
match entry {
VtblEntry::MetadataSize => {
write_usize(fx.tcx, &mut data, idx, layout.size.bytes());
}
VtblEntry::MetadataAlign => {
write_usize(fx.tcx, &mut data, idx, layout.align.abi.bytes());
}
VtblEntry::MetadataDropInPlace | VtblEntry::Vacant | VtblEntry::Method(_, _) => {}
}
}
data_ctx.define(data);

for (idx, entry) in vtable_entries.iter().enumerate() {
match entry {
VtblEntry::MetadataDropInPlace => {
let func_ref = fx.module.declare_func_in_data(drop_in_place_fn, &mut data_ctx);
data_ctx.write_function_addr((idx * usize_size) as u32, func_ref);
}
VtblEntry::Method(def_id, substs) => {
let func_id = import_function(
tcx,
fx.module,
Instance::resolve_for_vtable(tcx, ParamEnv::reveal_all(), *def_id, substs)
.unwrap()
.polymorphize(fx.tcx),
);
let func_ref = fx.module.declare_func_in_data(func_id, &mut data_ctx);
data_ctx.write_function_addr((idx * usize_size) as u32, func_ref);
}
VtblEntry::MetadataSize | VtblEntry::MetadataAlign | VtblEntry::Vacant => {}
}
}

data_ctx.set_align(fx.tcx.data_layout.pointer_align.pref.bytes());

let data_id = fx.module.declare_anonymous_data(false, false).unwrap();

fx.module.define_data(data_id, &data_ctx).unwrap();

data_id
}

fn write_usize(tcx: TyCtxt<'_>, buf: &mut [u8], idx: usize, num: u64) {
let pointer_size =
tcx.layout_of(ParamEnv::reveal_all().and(tcx.types.usize)).unwrap().size.bytes() as usize;
let target = &mut buf[idx * pointer_size..(idx + 1) * pointer_size];

match tcx.data_layout.endian {
rustc_target::abi::Endian::Little => match pointer_size {
4 => target.copy_from_slice(&(num as u32).to_le_bytes()),
8 => target.copy_from_slice(&(num as u64).to_le_bytes()),
_ => todo!("pointer size {} is not yet supported", pointer_size),
},
rustc_target::abi::Endian::Big => match pointer_size {
4 => target.copy_from_slice(&(num as u32).to_be_bytes()),
8 => target.copy_from_slice(&(num as u64).to_be_bytes()),
_ => todo!("pointer size {} is not yet supported", pointer_size),
},
}
vtable_ptr.get_addr(fx)
}
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,10 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
}
}

fn const_data_from_alloc(&self, alloc: &Allocation) -> Self::Value {
const_alloc_to_llvm(self, alloc)
}

fn from_const_alloc(
&self,
layout: TyAndLayout<'tcx>,
Expand Down
43 changes: 4 additions & 39 deletions compiler/rustc_codegen_ssa/src/meth.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::traits::*;

use rustc_middle::ty::{self, Instance, Ty, VtblEntry, COMMON_VTABLE_ENTRIES};
use rustc_middle::ty::{self, Ty};
use rustc_target::abi::call::FnAbi;

#[derive(Copy, Clone, Debug)]
Expand Down Expand Up @@ -70,48 +70,13 @@ pub fn get_vtable<'tcx, Cx: CodegenMethods<'tcx>>(
return val;
}

// Not in the cache; build it.
let nullptr = cx.const_null(cx.type_i8p_ext(cx.data_layout().instruction_address_space));

let vtable_entries = if let Some(trait_ref) = trait_ref {
tcx.vtable_entries(trait_ref.with_self_ty(tcx, ty))
} else {
COMMON_VTABLE_ENTRIES
};

let layout = cx.layout_of(ty);
// /////////////////////////////////////////////////////////////////////////////////////////////
// If you touch this code, be sure to also make the corresponding changes to
// `get_vtable` in `rust_mir/interpret/traits.rs`.
// /////////////////////////////////////////////////////////////////////////////////////////////
let components: Vec<_> = vtable_entries
.iter()
.map(|entry| match entry {
VtblEntry::MetadataDropInPlace => {
cx.get_fn_addr(Instance::resolve_drop_in_place(cx.tcx(), ty))
}
VtblEntry::MetadataSize => cx.const_usize(layout.size.bytes()),
VtblEntry::MetadataAlign => cx.const_usize(layout.align.abi.bytes()),
VtblEntry::Vacant => nullptr,
VtblEntry::Method(def_id, substs) => cx.get_fn_addr(
ty::Instance::resolve_for_vtable(
cx.tcx(),
ty::ParamEnv::reveal_all(),
*def_id,
substs,
)
.unwrap()
.polymorphize(cx.tcx()),
),
})
.collect();

let vtable_const = cx.const_struct(&components, false);
let vtable_alloc_id = tcx.vtable_allocation(ty, trait_ref);
let vtable_allocation = tcx.global_alloc(vtable_alloc_id).unwrap_memory();
let vtable_const = cx.const_data_from_alloc(vtable_allocation);
let align = cx.data_layout().pointer_align.abi;
let vtable = cx.static_addr_of(vtable_const, align, Some("vtable"));

cx.create_vtable_metadata(ty, vtable);

cx.vtables().borrow_mut().insert((ty, trait_ref), vtable);
vtable
}
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_ssa/src/traits/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ pub trait ConstMethods<'tcx>: BackendTypes {
fn const_to_opt_uint(&self, v: Self::Value) -> Option<u64>;
fn const_to_opt_u128(&self, v: Self::Value, sign_ext: bool) -> Option<u128>;

fn const_data_from_alloc(&self, alloc: &Allocation) -> Self::Value;
Copy link
Member

Choose a reason for hiding this comment

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

This is just from_const_alloc, isn't it? Just with a hardcoded offset of 0 and no type.

Copy link
Member

Choose a reason for hiding this comment

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

from_const_alloc also returns a PlaceRef while this returns a raw pointer value.

Copy link
Member

Choose a reason for hiding this comment

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

Right, but the PlaceRef is made from that pointer value and the type/layout passed in.


fn scalar_to_backend(&self, cv: Scalar, layout: &abi::Scalar, llty: Self::Type) -> Self::Value;
fn from_const_alloc(
&self,
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::middle;
use crate::middle::cstore::{CrateStoreDyn, EncodedMetadata};
use crate::middle::resolve_lifetime::{self, LifetimeScopeForPath, ObjectLifetimeDefault};
use crate::middle::stability;
use crate::mir::interpret::{self, Allocation, ConstValue, Scalar};
use crate::mir::interpret::{self, AllocId, Allocation, ConstValue, Scalar};
use crate::mir::{Body, Field, Local, Place, PlaceElem, ProjectionKind, Promoted};
use crate::thir::Thir;
use crate::traits;
Expand Down Expand Up @@ -1045,6 +1045,9 @@ pub struct GlobalCtxt<'tcx> {
output_filenames: Arc<OutputFilenames>,

pub main_def: Option<MainDefinition>,

pub(super) vtables_cache:
Lock<FxHashMap<(Ty<'tcx>, Option<ty::PolyExistentialTraitRef<'tcx>>), AllocId>>,
Comment on lines +1048 to +1050
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, any reason this is a mutable cache in the global context, instead of a query? It looks like the kind of thing we replaced with queries long ago.

(Maybe we should find a way to ban !Freeze types in GlobalCtxt, outside of incremental/query state?)

cc @oli-obk @wesleywiser

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually gave it a try as a query locally, but without succeed. The compilation complained about keys becoming bigger, something like that, i can't remember the exact error content.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, that's because that key type is a bit too "spread out". Sadly you can't just use TraitRef<'tcx>, without passing some dummy trait for the DefId.

You could "just" update the expected size, but someone would need to check off on that (and I don't know who) - the problem there is it can affect performance even if the new query is completely unused.

Copy link
Member

Choose a reason for hiding this comment

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

Having a cache without any dependency tracking looks like a bug to me.

}

impl<'tcx> TyCtxt<'tcx> {
Expand Down Expand Up @@ -1202,6 +1205,7 @@ impl<'tcx> TyCtxt<'tcx> {
alloc_map: Lock::new(interpret::AllocMap::new()),
output_filenames: Arc::new(output_filenames),
main_def: resolutions.main_def,
vtables_cache: Default::default(),
}
}

Expand Down
18 changes: 2 additions & 16 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub use adt::*;
pub use assoc::*;
pub use closure::*;
pub use generics::*;
pub use vtable::*;

use crate::hir::exports::ExportMap;
use crate::ich::StableHashingContext;
Expand Down Expand Up @@ -94,6 +95,7 @@ pub mod relate;
pub mod subst;
pub mod trait_def;
pub mod util;
pub mod vtable;
pub mod walk;

mod adt;
Expand Down Expand Up @@ -2009,19 +2011,3 @@ impl<'tcx> fmt::Debug for SymbolName<'tcx> {
fmt::Display::fmt(&self.name, fmt)
}
}

#[derive(Clone, Copy, Debug, PartialEq, HashStable)]
pub enum VtblEntry<'tcx> {
MetadataDropInPlace,
MetadataSize,
MetadataAlign,
Vacant,
Method(DefId, SubstsRef<'tcx>),
}

pub const COMMON_VTABLE_ENTRIES: &[VtblEntry<'_>] =
&[VtblEntry::MetadataDropInPlace, VtblEntry::MetadataSize, VtblEntry::MetadataAlign];

pub const COMMON_VTABLE_ENTRIES_DROPINPLACE: usize = 0;
pub const COMMON_VTABLE_ENTRIES_SIZE: usize = 1;
pub const COMMON_VTABLE_ENTRIES_ALIGN: usize = 2;
Loading