From 88e39ee3149b813af3966388a0d49a92f947e322 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 6 Feb 2023 15:54:35 +0100 Subject: [PATCH 1/5] make first component of dyn* use pointer layout+type, and adjust DynStar comment --- compiler/rustc_codegen_ssa/src/base.rs | 9 +-------- compiler/rustc_middle/src/ty/layout.rs | 2 +- compiler/rustc_ty_utils/src/layout.rs | 2 +- compiler/rustc_type_ir/src/sty.rs | 8 +++----- 4 files changed, 6 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index 023d38e931284..229e3d9dc5f8e 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -39,7 +39,7 @@ use rustc_session::Session; use rustc_span::symbol::sym; use rustc_span::Symbol; use rustc_span::{DebuggerVisualizerFile, DebuggerVisualizerType}; -use rustc_target::abi::{Align, Size, VariantIdx}; +use rustc_target::abi::{Align, VariantIdx}; use std::collections::BTreeSet; use std::time::{Duration, Instant}; @@ -273,13 +273,6 @@ pub fn cast_to_dyn_star<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( matches!(dst_ty.kind(), ty::Dynamic(_, _, ty::DynStar)), "destination type must be a dyn*" ); - // FIXME(dyn-star): this is probably not the best way to check if this is - // a pointer, and really we should ensure that the value is a suitable - // pointer earlier in the compilation process. - let src = match src_ty_and_layout.pointee_info_at(bx.cx(), Size::ZERO) { - Some(_) => bx.ptrtoint(src, bx.cx().type_isize()), - None => bx.bitcast(src, bx.type_isize()), - }; (src, unsized_info(bx, src_ty_and_layout.ty, dst_ty, old_info)) } diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 3d0f9a5053cb3..993191ee96a44 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -770,7 +770,7 @@ where ty::Dynamic(_, _, ty::DynStar) => { if i == 0 { - TyMaybeWithLayout::Ty(tcx.types.usize) + TyMaybeWithLayout::Ty(tcx.mk_mut_ptr(tcx.types.unit)) } else if i == 1 { // FIXME(dyn-star) same FIXME as above applies here too TyMaybeWithLayout::Ty( diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index b860fb6c9186b..1a62794b0b441 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -193,7 +193,7 @@ fn layout_of_uncached<'tcx>( } ty::Dynamic(_, _, ty::DynStar) => { - let mut data = scalar_unit(Int(dl.ptr_sized_integer(), false)); + let mut data = scalar_unit(Pointer(AddressSpace::DATA)); data.valid_range_mut().start = 0; let mut vtable = scalar_unit(Pointer(AddressSpace::DATA)); vtable.valid_range_mut().start = 1; diff --git a/compiler/rustc_type_ir/src/sty.rs b/compiler/rustc_type_ir/src/sty.rs index e080726d91d58..ebe2b76aef335 100644 --- a/compiler/rustc_type_ir/src/sty.rs +++ b/compiler/rustc_type_ir/src/sty.rs @@ -26,11 +26,9 @@ pub enum DynKind { Dyn, /// A sized `dyn* Trait` object /// - /// These objects are represented as a `(data, vtable)` pair where `data` is a ptr-sized value - /// (often a pointer to the real object, but not necessarily) and `vtable` is a pointer to - /// the vtable for `dyn* Trait`. The representation is essentially the same as `&dyn Trait` - /// or similar, but the drop function included in the vtable is responsible for freeing the - /// underlying storage if needed. This allows a `dyn*` object to be treated agnostically with + /// These objects are represented as a `(data, vtable)` pair where `data` is a value of some + /// ptr-sized and ptr-aligned dynamically determined type `T` and `vtable` is a pointer to the + /// vtable of `impl T for Trait`. This allows a `dyn*` object to be treated agnostically with /// respect to whether it points to a `Box`, `Rc`, etc. DynStar, } From df52e2037ab73828a29c46addf07df5522b305e5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 7 Feb 2023 19:17:04 +0000 Subject: [PATCH 2/5] Use inttoptr to support usize as dyn* value, use pointercast to make sure pointers are compatible --- compiler/rustc_codegen_ssa/src/base.rs | 8 ++++++++ tests/ui/dyn-star/llvm-old-style-ptrs.rs | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 tests/ui/dyn-star/llvm-old-style-ptrs.rs diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index 229e3d9dc5f8e..4e13d4dbcb7a4 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -273,6 +273,14 @@ pub fn cast_to_dyn_star<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( matches!(dst_ty.kind(), ty::Dynamic(_, _, ty::DynStar)), "destination type must be a dyn*" ); + // FIXME(dyn-star): We can remove this when all supported LLVMs use opaque ptrs only. + let unit_ptr = bx.cx().type_ptr_to(bx.cx().type_struct(&[], false)); + let src = match bx.cx().type_kind(bx.cx().backend_type(src_ty_and_layout)) { + TypeKind::Pointer => bx.pointercast(src, unit_ptr), + TypeKind::Integer => bx.inttoptr(src, unit_ptr), + // FIXME(dyn-star): We probably have to do a bitcast first, then inttoptr. + kind => bug!("unexpected TypeKind for left-hand side of `dyn*` cast: {kind:?}"), + }; (src, unsized_info(bx, src_ty_and_layout.ty, dst_ty, old_info)) } diff --git a/tests/ui/dyn-star/llvm-old-style-ptrs.rs b/tests/ui/dyn-star/llvm-old-style-ptrs.rs new file mode 100644 index 0000000000000..d35519632becf --- /dev/null +++ b/tests/ui/dyn-star/llvm-old-style-ptrs.rs @@ -0,0 +1,23 @@ +// run-pass +// compile-flags: -Copt-level=0 -Cllvm-args=-opaque-pointers=0 + +// (opaque-pointers flag is called force-opaque-pointers in LLVM 13...) +// min-llvm-version: 14.0 + +// This test can be removed once non-opaque pointers are gone from LLVM, maybe. + +#![feature(dyn_star, pointer_like_trait)] +#![allow(incomplete_features)] + +use std::fmt::Debug; +use std::marker::PointerLike; + +fn make_dyn_star<'a>(t: impl PointerLike + Debug + 'a) -> dyn* Debug + 'a { + t as _ +} + +fn main() { + println!("{:?}", make_dyn_star(Box::new(1i32))); + println!("{:?}", make_dyn_star(2usize)); + println!("{:?}", make_dyn_star((3usize,))); +} From 1f11d841b5748133c2d51da0001fc88bc6b51e78 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 10 Feb 2023 23:54:05 +0000 Subject: [PATCH 3/5] Add codegen test --- tests/codegen/function-arguments.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/codegen/function-arguments.rs b/tests/codegen/function-arguments.rs index 96dfde18683e3..63f0511903108 100644 --- a/tests/codegen/function-arguments.rs +++ b/tests/codegen/function-arguments.rs @@ -1,6 +1,7 @@ // compile-flags: -O -C no-prepopulate-passes #![crate_type = "lib"] +#![feature(dyn_star)] use std::mem::MaybeUninit; use std::num::NonZeroU64; @@ -279,3 +280,9 @@ pub fn enum_id_1(x: Option>) -> Option> { pub fn enum_id_2(x: Option) -> Option { x } + +// CHECK: { {{i8\*|ptr}}, {{i.*\*|ptr}} } @dyn_star({{i8\*|ptr}} noundef %x.0, {{i.*\*|ptr}} noalias noundef readonly align {{.*}} dereferenceable({{.*}}) %x.1) +#[no_mangle] +pub fn dyn_star(x: dyn* Drop) -> dyn* Drop { + x +} From e82cc656c822af7f1905b757a27cac5823fbc301 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 15 Feb 2023 03:42:45 +0000 Subject: [PATCH 4/5] Make dyn* have the same scalar pair ABI as corresponding fat pointer --- compiler/rustc_codegen_llvm/src/type_of.rs | 7 ++++++- tests/codegen/function-arguments.rs | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index cc8ff947fc31f..9cda24bab87d3 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -329,7 +329,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { ) -> &'a Type { // HACK(eddyb) special-case fat pointers until LLVM removes // pointee types, to avoid bitcasting every `OperandRef::deref`. - match self.ty.kind() { + match *self.ty.kind() { ty::Ref(..) | ty::RawPtr(_) => { return self.field(cx, index).llvm_type(cx); } @@ -339,6 +339,11 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { let ptr_ty = cx.tcx.mk_mut_ptr(self.ty.boxed_ty()); return cx.layout_of(ptr_ty).scalar_pair_element_llvm_type(cx, index, immediate); } + // `dyn* Trait` has the same ABI as `*mut dyn Trait` + ty::Dynamic(bounds, region, ty::DynStar) => { + let ptr_ty = cx.tcx.mk_mut_ptr(cx.tcx.mk_dynamic(bounds, region, ty::Dyn)); + return cx.layout_of(ptr_ty).scalar_pair_element_llvm_type(cx, index, immediate); + } _ => {} } diff --git a/tests/codegen/function-arguments.rs b/tests/codegen/function-arguments.rs index 63f0511903108..d6f019016a5a7 100644 --- a/tests/codegen/function-arguments.rs +++ b/tests/codegen/function-arguments.rs @@ -281,7 +281,9 @@ pub fn enum_id_2(x: Option) -> Option { x } -// CHECK: { {{i8\*|ptr}}, {{i.*\*|ptr}} } @dyn_star({{i8\*|ptr}} noundef %x.0, {{i.*\*|ptr}} noalias noundef readonly align {{.*}} dereferenceable({{.*}}) %x.1) +// CHECK: { {{\{\}\*|ptr}}, {{.+}} } @dyn_star({{\{\}\*|ptr}} noundef %x.0, {{.+}} noalias noundef readonly align {{.*}} dereferenceable({{.*}}) %x.1) +// Expect an ABI something like `{ {}*, [3 x i64]* }`, but that's hard to match on generically, +// so do like the `trait_box` test and just match on `{{.+}}` for the vtable. #[no_mangle] pub fn dyn_star(x: dyn* Drop) -> dyn* Drop { x From 7f798c2b216db0bb7ebeb9dd863fbdf7668094c5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 15 Feb 2023 05:11:27 +0000 Subject: [PATCH 5/5] Emit the right types for vtable pointers when dropping dyn* --- compiler/rustc_codegen_ssa/src/mir/block.rs | 158 ++++++++++---------- 1 file changed, 78 insertions(+), 80 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 9af408646ae03..daaa4de1a7c8e 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -452,86 +452,84 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { args1 = [place.llval]; &args1[..] }; - let (drop_fn, fn_abi) = match ty.kind() { - // FIXME(eddyb) perhaps move some of this logic into - // `Instance::resolve_drop_in_place`? - ty::Dynamic(_, _, ty::Dyn) => { - // IN THIS ARM, WE HAVE: - // ty = *mut (dyn Trait) - // which is: exists ( *mut T, Vtable ) - // args[0] args[1] - // - // args = ( Data, Vtable ) - // | - // v - // /-------\ - // | ... | - // \-------/ - // - let virtual_drop = Instance { - def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0), - substs: drop_fn.substs, - }; - debug!("ty = {:?}", ty); - debug!("drop_fn = {:?}", drop_fn); - debug!("args = {:?}", args); - let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty()); - let vtable = args[1]; - // Truncate vtable off of args list - args = &args[..1]; - ( - meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE) - .get_fn(bx, vtable, ty, &fn_abi), - fn_abi, - ) - } - ty::Dynamic(_, _, ty::DynStar) => { - // IN THIS ARM, WE HAVE: - // ty = *mut (dyn* Trait) - // which is: *mut exists (T, Vtable) - // - // args = [ * ] - // | - // v - // ( Data, Vtable ) - // | - // v - // /-------\ - // | ... | - // \-------/ - // - // - // WE CAN CONVERT THIS INTO THE ABOVE LOGIC BY DOING - // - // data = &(*args[0]).0 // gives a pointer to Data above (really the same pointer) - // vtable = (*args[0]).1 // loads the vtable out - // (data, vtable) // an equivalent Rust `*mut dyn Trait` - // - // SO THEN WE CAN USE THE ABOVE CODE. - let virtual_drop = Instance { - def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0), - substs: drop_fn.substs, - }; - debug!("ty = {:?}", ty); - debug!("drop_fn = {:?}", drop_fn); - debug!("args = {:?}", args); - let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty()); - let data = args[0]; - let data_ty = bx.cx().backend_type(place.layout); - let vtable_ptr = - bx.gep(data_ty, data, &[bx.cx().const_i32(0), bx.cx().const_i32(1)]); - let vtable = bx.load(bx.type_i8p(), vtable_ptr, abi::Align::ONE); - // Truncate vtable off of args list - args = &args[..1]; - debug!("args' = {:?}", args); - ( - meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE) - .get_fn(bx, vtable, ty, &fn_abi), - fn_abi, - ) - } - _ => (bx.get_fn_addr(drop_fn), bx.fn_abi_of_instance(drop_fn, ty::List::empty())), - }; + let (drop_fn, fn_abi) = + match ty.kind() { + // FIXME(eddyb) perhaps move some of this logic into + // `Instance::resolve_drop_in_place`? + ty::Dynamic(_, _, ty::Dyn) => { + // IN THIS ARM, WE HAVE: + // ty = *mut (dyn Trait) + // which is: exists ( *mut T, Vtable ) + // args[0] args[1] + // + // args = ( Data, Vtable ) + // | + // v + // /-------\ + // | ... | + // \-------/ + // + let virtual_drop = Instance { + def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0), + substs: drop_fn.substs, + }; + debug!("ty = {:?}", ty); + debug!("drop_fn = {:?}", drop_fn); + debug!("args = {:?}", args); + let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty()); + let vtable = args[1]; + // Truncate vtable off of args list + args = &args[..1]; + ( + meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE) + .get_fn(bx, vtable, ty, &fn_abi), + fn_abi, + ) + } + ty::Dynamic(_, _, ty::DynStar) => { + // IN THIS ARM, WE HAVE: + // ty = *mut (dyn* Trait) + // which is: *mut exists (T, Vtable) + // + // args = [ * ] + // | + // v + // ( Data, Vtable ) + // | + // v + // /-------\ + // | ... | + // \-------/ + // + // + // WE CAN CONVERT THIS INTO THE ABOVE LOGIC BY DOING + // + // data = &(*args[0]).0 // gives a pointer to Data above (really the same pointer) + // vtable = (*args[0]).1 // loads the vtable out + // (data, vtable) // an equivalent Rust `*mut dyn Trait` + // + // SO THEN WE CAN USE THE ABOVE CODE. + let virtual_drop = Instance { + def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0), + substs: drop_fn.substs, + }; + debug!("ty = {:?}", ty); + debug!("drop_fn = {:?}", drop_fn); + debug!("args = {:?}", args); + let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty()); + let meta_ptr = place.project_field(bx, 1); + let meta = bx.load_operand(meta_ptr); + // Truncate vtable off of args list + args = &args[..1]; + debug!("args' = {:?}", args); + ( + meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE) + .get_fn(bx, meta.immediate(), ty, &fn_abi), + fn_abi, + ) + } + _ => (bx.get_fn_addr(drop_fn), bx.fn_abi_of_instance(drop_fn, ty::List::empty())), + }; helper.do_call( self, bx,