From 7af3406a49eb27f54d043cc45e99b986c7f05d7c Mon Sep 17 00:00:00 2001 From: James Miller Date: Tue, 21 Feb 2017 21:08:06 +1300 Subject: [PATCH 1/2] 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 #39992 --- src/librustc_trans/abi.rs | 4 ++++ src/librustc_trans/base.rs | 11 +++++++++-- src/librustc_trans/builder.rs | 7 +++++++ src/librustc_trans/glue.rs | 10 +++++++++- src/librustc_trans/meth.rs | 8 +++++--- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/librustc_trans/abi.rs b/src/librustc_trans/abi.rs index a476b1d29e5fb..f742cca5b980a 100644 --- a/src/librustc_trans/abi.rs +++ b/src/librustc_trans/abi.rs @@ -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); diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 41c0eaa52a77d..0f7a510c36385 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -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) } diff --git a/src/librustc_trans/builder.rs b/src/librustc_trans/builder.rs index f64e581c1773e..99738dd6872e8 100644 --- a/src/librustc_trans/builder.rs +++ b/src/librustc_trans/builder.rs @@ -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, diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index d66ea4d650f7b..9963514acd736 100644 --- a/src/librustc_trans/glue.rs +++ b/src/librustc_trans/glue.rs @@ -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()); diff --git a/src/librustc_trans/meth.rs b/src/librustc_trans/meth.rs index 3033ae61d20c8..a3f4168e96f2a 100644 --- a/src/librustc_trans/meth.rs +++ b/src/librustc_trans/meth.rs @@ -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 From d80cf80b16660289ebc9765940d02b36ef1032b6 Mon Sep 17 00:00:00 2001 From: James Miller Date: Wed, 22 Feb 2017 09:49:12 +1300 Subject: [PATCH 2/2] Update codegen test with new attributes --- src/test/codegen/function-arguments.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index c373cdb76c5c7..76313b158ab11 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -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) { }