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

Set metadata for vtable-related loads #39995

Merged
merged 2 commits into from
Feb 25, 2017
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
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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't readonly a function attribute? What does this mean when applied to arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When applied to a pointer argument, it means that the function will only read from it. That allows LLVM to assume that the value behind the pointer won't change across a call to that function.

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)
Copy link
Member

Choose a reason for hiding this comment

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

Missing alignment?

Copy link
Contributor

Choose a reason for hiding this comment

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

None = ABI alignment. Vtables are ABI aligned.

};

(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