From 80feed380d478a14e5acd99fc8721ed0e6de19ba Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 12 Oct 2018 16:10:16 +0200 Subject: [PATCH 1/5] Deduplicate vtables within a single evaluation --- src/librustc_mir/interpret/eval_context.rs | 5 +++++ src/librustc_mir/interpret/traits.rs | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 85a8376134aa4..be9f2b8f658dd 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -27,6 +27,7 @@ use rustc::mir::interpret::{ EvalResult, EvalErrorKind, truncate, sign_extend, }; +use rustc_data_structures::fx::FxHashMap; use syntax::source_map::{self, Span}; @@ -50,6 +51,9 @@ pub struct EvalContext<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'a, 'mir, 'tcx>> { /// The virtual call stack. pub(crate) stack: Vec>, + + /// A cache for deduplicating vtables + pub(super) vtables: FxHashMap<(Ty<'tcx>, ty::PolyTraitRef<'tcx>), AllocId>, } /// A stack frame. @@ -209,6 +213,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc param_env, memory: Memory::new(tcx, memory_data), stack: Vec::new(), + vtables: FxHashMap(), } } diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 227c85772d228..b4c73ad02c313 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -28,7 +28,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> ) -> EvalResult<'tcx, Pointer> { debug!("get_vtable(trait_ref={:?})", trait_ref); - // FIXME: Cache this! + if let Some(&vtable) = self.vtables.get(&(ty, trait_ref)) { + return Ok(Pointer::from(vtable).with_default_tag()); + } let layout = self.layout_of(trait_ref.self_ty())?; assert!(!layout.is_unsized(), "can't create a vtable for an unsized type"); @@ -64,6 +66,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } self.memory.mark_immutable(vtable.alloc_id)?; + assert!(self.vtables.insert((ty, trait_ref), vtable.alloc_id).is_none()); Ok(vtable) } From 8ac7fa414bf6fce856c10bafbe0bf068d48a02be Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 12 Oct 2018 16:45:17 +0200 Subject: [PATCH 2/5] Synchronize get_vtable with the `codegen_llvm` one --- src/librustc_mir/interpret/cast.rs | 7 +------ src/librustc_mir/interpret/eval_context.rs | 2 +- src/librustc_mir/interpret/traits.rs | 18 ++++++++++++------ 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index f4ddfa5293e1e..f5e824b762888 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -327,12 +327,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } (_, &ty::Dynamic(ref data, _)) => { // Initial cast from sized to dyn trait - let trait_ref = data.principal().with_self_ty( - *self.tcx, - src_pointee_ty, - ); - let trait_ref = self.tcx.erase_regions(&trait_ref); - let vtable = self.get_vtable(src_pointee_ty, trait_ref)?; + let vtable = self.get_vtable(src_pointee_ty, data.principal())?; let ptr = self.read_value(src)?.to_scalar_ptr()?; let val = Value::new_dyn_trait(ptr, vtable); self.write_value(val, dest) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index be9f2b8f658dd..13b6fa44de203 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -53,7 +53,7 @@ pub struct EvalContext<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'a, 'mir, 'tcx>> { pub(crate) stack: Vec>, /// A cache for deduplicating vtables - pub(super) vtables: FxHashMap<(Ty<'tcx>, ty::PolyTraitRef<'tcx>), AllocId>, + pub(super) vtables: FxHashMap<(Ty<'tcx>, ty::PolyExistentialTraitRef<'tcx>), AllocId>, } /// A stack frame. diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index b4c73ad02c313..30591a4ff5a5e 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -24,22 +24,28 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> pub fn get_vtable( &mut self, ty: Ty<'tcx>, - trait_ref: ty::PolyTraitRef<'tcx>, + poly_trait_ref: ty::PolyExistentialTraitRef<'tcx>, ) -> EvalResult<'tcx, Pointer> { - debug!("get_vtable(trait_ref={:?})", trait_ref); + debug!("get_vtable(trait_ref={:?})", poly_trait_ref); - if let Some(&vtable) = self.vtables.get(&(ty, trait_ref)) { + let (ty, poly_trait_ref) = self.tcx.erase_regions(&(ty, poly_trait_ref)); + + if let Some(&vtable) = self.vtables.get(&(ty, poly_trait_ref)) { return Ok(Pointer::from(vtable).with_default_tag()); } - let layout = self.layout_of(trait_ref.self_ty())?; + let trait_ref = poly_trait_ref.with_self_ty(*self.tcx, ty); + let trait_ref = self.tcx.erase_regions(&trait_ref); + + let methods = self.tcx.vtable_methods(trait_ref); + + let layout = self.layout_of(ty)?; assert!(!layout.is_unsized(), "can't create a vtable for an unsized type"); let size = layout.size.bytes(); let align = layout.align.abi(); let ptr_size = self.pointer_size(); let ptr_align = self.tcx.data_layout.pointer_align; - let methods = self.tcx.vtable_methods(trait_ref); let vtable = self.memory.allocate( ptr_size * (3 + methods.len() as u64), ptr_align, @@ -66,7 +72,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } self.memory.mark_immutable(vtable.alloc_id)?; - assert!(self.vtables.insert((ty, trait_ref), vtable.alloc_id).is_none()); + assert!(self.vtables.insert((ty, poly_trait_ref), vtable.alloc_id).is_none()); Ok(vtable) } From ef3ece74446d964066a21c9b47fb945a9b04f3ef Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 12 Oct 2018 16:59:55 +0200 Subject: [PATCH 3/5] Uplift some comments to doc comments --- src/librustc/ty/query/mod.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 7f5bc35f91f9b..ac5c0352453e7 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -369,16 +369,16 @@ define_queries! { <'tcx> -> Lrc, [] fn is_object_safe: ObjectSafety(DefId) -> bool, - // Get the ParameterEnvironment for a given item; this environment - // will be in "user-facing" mode, meaning that it is suitabe for - // type-checking etc, and it does not normalize specializable - // associated types. This is almost always what you want, - // unless you are doing MIR optimizations, in which case you - // might want to use `reveal_all()` method to change modes. + /// Get the ParameterEnvironment for a given item; this environment + /// will be in "user-facing" mode, meaning that it is suitabe for + /// type-checking etc, and it does not normalize specializable + /// associated types. This is almost always what you want, + /// unless you are doing MIR optimizations, in which case you + /// might want to use `reveal_all()` method to change modes. [] fn param_env: ParamEnv(DefId) -> ty::ParamEnv<'tcx>, - // Trait selection queries. These are best used by invoking `ty.moves_by_default()`, - // `ty.is_copy()`, etc, since that will prune the environment where possible. + /// Trait selection queries. These are best used by invoking `ty.moves_by_default()`, + /// `ty.is_copy()`, etc, since that will prune the environment where possible. [] fn is_copy_raw: is_copy_dep_node(ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool, [] fn is_sized_raw: is_sized_dep_node(ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool, [] fn is_freeze_raw: is_freeze_dep_node(ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool, From c95ee9e91dce4fa56c4636e0b8b9cbc4e676b830 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 12 Oct 2018 17:08:36 +0200 Subject: [PATCH 4/5] Add comments to remind everyone to keep the `get_vtable` impls in sync --- src/librustc_codegen_llvm/meth.rs | 4 ++++ src/librustc_mir/interpret/traits.rs | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/librustc_codegen_llvm/meth.rs b/src/librustc_codegen_llvm/meth.rs index db06b87f44e1e..29c2e71960c2c 100644 --- a/src/librustc_codegen_llvm/meth.rs +++ b/src/librustc_codegen_llvm/meth.rs @@ -94,6 +94,10 @@ pub fn get_vtable( }); let (size, align) = cx.size_and_align_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<_> = [ callee::get_fn(cx, monomorphize::resolve_drop_in_place(cx.tcx, ty)), C_usize(cx, size.bytes()), diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 30591a4ff5a5e..2b0febc1ce717 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -46,6 +46,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let ptr_size = self.pointer_size(); let ptr_align = self.tcx.data_layout.pointer_align; + // ///////////////////////////////////////////////////////////////////////////////////////// + // If you touch this code, be sure to also make the corresponding changes to + // `get_vtable` in rust_codegen_llvm/meth.rs + // ///////////////////////////////////////////////////////////////////////////////////////// let vtable = self.memory.allocate( ptr_size * (3 + methods.len() as u64), ptr_align, From b1d3111ba2f4c7d318632c330b4356fec52b4055 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 16 Oct 2018 10:39:48 +0200 Subject: [PATCH 5/5] Use forward compatible `FxHashMap` initialization --- src/librustc_mir/interpret/eval_context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 13b6fa44de203..cf5358a989672 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -213,7 +213,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc param_env, memory: Memory::new(tcx, memory_data), stack: Vec::new(), - vtables: FxHashMap(), + vtables: FxHashMap::default(), } }