Skip to content

Commit

Permalink
Rollup merge of rust-lang#39995 - Aatch:vtable-ptr-metadata, r=arielb1
Browse files Browse the repository at this point in the history
Set metadata for vtable-related loads

Give LLVM much more information about vtable pointers. Without the extra
information, LLVM has to be rather pessimistic about vtables, preventing
a number of obvious optimisations.

* Makes the vtable pointer argument noalias and readonly.
* Marks loads of the vtable pointer as nonnull.
* Marks load from the vtable with `!invariant.load` metadata.

Fixes rust-lang#39992
  • Loading branch information
eddyb authored Feb 25, 2017
2 parents 0a950bd + d80cf80 commit 5c0b4b3
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 8 deletions.
4 changes: 4 additions & 0 deletions src/librustc_trans/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,11 @@ impl FnType {
if let Some(inner) = rust_ptr_attrs(ty, &mut data) {
data.attrs.set(ArgAttribute::NonNull);
if ccx.tcx().struct_tail(inner).is_trait() {
// vtables can be safely marked non-null, readonly
// and noalias.
info.attrs.set(ArgAttribute::NonNull);
info.attrs.set(ArgAttribute::ReadOnly);
info.attrs.set(ArgAttribute::NoAlias);
}
}
args.push(data);
Expand Down
11 changes: 9 additions & 2 deletions src/librustc_trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,8 +472,15 @@ pub fn load_fat_ptr<'a, 'tcx>(
b.load(ptr, alignment.to_align())
};

// FIXME: emit metadata on `meta`.
let meta = b.load(get_meta(b, src), alignment.to_align());
let meta = get_meta(b, src);
let meta_ty = val_ty(meta);
// If the 'meta' field is a pointer, it's a vtable, so use load_nonnull
// instead
let meta = if meta_ty.element_type().kind() == llvm::TypeKind::Pointer {
b.load_nonnull(meta, None)
} else {
b.load(meta, None)
};

(ptr, meta)
}
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_trans/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

pub fn set_invariant_load(&self, load: ValueRef) {
unsafe {
llvm::LLVMSetMetadata(load, llvm::MD_invariant_load as c_uint,
llvm::LLVMMDNodeInContext(self.ccx.llcx(), ptr::null(), 0));
}
}

/// Returns the ptr value that should be used for storing `val`.
fn check_store<'b>(&self,
val: ValueRef,
Expand Down
10 changes: 9 additions & 1 deletion src/librustc_trans/glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,15 @@ pub fn size_and_align_of_dst<'a, 'tcx>(bcx: &Builder<'a, 'tcx>, t: Ty<'tcx>, inf
let info = bcx.pointercast(info, Type::int(bcx.ccx).ptr_to());
let size_ptr = bcx.gepi(info, &[1]);
let align_ptr = bcx.gepi(info, &[2]);
(bcx.load(size_ptr, None), bcx.load(align_ptr, None))

let size = bcx.load(size_ptr, None);
let align = bcx.load(align_ptr, None);

// Vtable loads are invariant
bcx.set_invariant_load(size);
bcx.set_invariant_load(align);

(size, align)
}
ty::TySlice(_) | ty::TyStr => {
let unit_ty = t.sequence_element_type(bcx.tcx());
Expand Down
8 changes: 5 additions & 3 deletions src/librustc_trans/meth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@ const VTABLE_OFFSET: usize = 3;
/// Extracts a method from a trait object's vtable, at the specified index.
pub fn get_virtual_method<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
llvtable: ValueRef,
vtable_index: usize)
-> ValueRef {
vtable_index: usize) -> ValueRef {
// Load the data pointer from the object.
debug!("get_virtual_method(vtable_index={}, llvtable={:?})",
vtable_index, Value(llvtable));

bcx.load(bcx.gepi(llvtable, &[vtable_index + VTABLE_OFFSET]), None)
let ptr = bcx.load_nonnull(bcx.gepi(llvtable, &[vtable_index + VTABLE_OFFSET]), None);
// Vtable loads are invariant
bcx.set_invariant_load(ptr);
ptr
}

/// Generate a shim function that allows an object type like `SomeTrait` to
Expand Down
4 changes: 2 additions & 2 deletions src/test/codegen/function-arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,13 @@ pub fn unsafe_slice(_: &[UnsafeInner]) {
fn str(_: &[u8]) {
}

// CHECK: @trait_borrow(i8* nonnull, void (i8*)** nonnull)
// CHECK: @trait_borrow(i8* nonnull, void (i8*)** noalias nonnull readonly)
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
fn trait_borrow(_: &Drop) {
}

// CHECK: @trait_box(i8* noalias nonnull, void (i8*)** nonnull)
// CHECK: @trait_box(i8* noalias nonnull, void (i8*)** noalias nonnull readonly)
#[no_mangle]
fn trait_box(_: Box<Drop>) {
}
Expand Down

0 comments on commit 5c0b4b3

Please sign in to comment.