From 9af70c19604ded4f60b3bcf0329b22db8a2d7ca5 Mon Sep 17 00:00:00 2001 From: Joshua Wong Date: Wed, 9 Oct 2024 14:24:05 -0400 Subject: [PATCH 1/3] add initial tests for placement new changes --- tests/codegen/placement-new.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/codegen/placement-new.rs diff --git a/tests/codegen/placement-new.rs b/tests/codegen/placement-new.rs new file mode 100644 index 0000000000000..397dc4796567b --- /dev/null +++ b/tests/codegen/placement-new.rs @@ -0,0 +1,27 @@ +//@ compile-flags: -O +#![crate_type = "lib"] + +// Test to check that types with "complex" destructors, but trivial `Default` impls +// are constructed directly into the allocation in `Box::default` and `Arc::default`. + +use std::sync::Arc; + +// CHECK-LABEL: @box_default_inplace +#[no_mangle] +pub fn box_default_inplace() -> Box<(String, String)> { + // CHECK: [[ALLOCA:%.*]] = alloca + // CHECK: [[BOX:%.*]] = {{.*}}call {{.*}}__rust_alloc( + // CHECK: call void @llvm.memcpy.p0.p0.i64(ptr {{.*}}[[BOX]], ptr {{.*}}[[ALLOCA]] + // CHECK: ret ptr [[BOX]] + Box::default() +} + +// CHECK-LABEL: @arc_default_inplace +#[no_mangle] +pub fn arc_default_inplace() -> Arc<(String, String)> { + // CHECK: [[ALLOCA:%.*]] = alloca + // CHECK: [[ARC:%.*]] = {{.*}}call {{.*}}__rust_alloc( + // CHECK: call void @llvm.memcpy.p0.p0.i64( + // CHECK: ret ptr [[ARC]] + Arc::default() +} From 2f4ef710e9174a1f4884bf251a8375fdc44115a7 Mon Sep 17 00:00:00 2001 From: Joshua Wong Date: Wed, 9 Oct 2024 14:24:05 -0400 Subject: [PATCH 2/3] allocate before calling T::default in >::default() The `Box` impl currently calls `T::default()` before allocating the `Box`. Most `Default` impls are trivial, which should in theory allow LLVM to construct `T: Default` directly in the `Box` allocation when calling `>::default()`. However, the allocation may fail, which necessitates calling `T's` destructor if it has one. If the destructor is non-trivial, then LLVM has a hard time proving that it's sound to elide, which makes it construct `T` on the stack first, and then copy it into the allocation. Create an uninit `Box` first, and then write `T::default` into it, so that LLVM now only needs to prove that the `T::default` can't panic, which should be trivial for most `Default` impls. --- library/alloc/src/boxed.rs | 2 +- tests/codegen/placement-new.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 5f20729568352..3e791416820ef 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -1688,7 +1688,7 @@ impl Default for Box { /// Creates a `Box`, with the `Default` value for T. #[inline] fn default() -> Self { - Box::new(T::default()) + Box::write(Box::new_uninit(), T::default()) } } diff --git a/tests/codegen/placement-new.rs b/tests/codegen/placement-new.rs index 397dc4796567b..1b49d780b5273 100644 --- a/tests/codegen/placement-new.rs +++ b/tests/codegen/placement-new.rs @@ -9,9 +9,9 @@ use std::sync::Arc; // CHECK-LABEL: @box_default_inplace #[no_mangle] pub fn box_default_inplace() -> Box<(String, String)> { - // CHECK: [[ALLOCA:%.*]] = alloca + // CHECK-NOT: alloca // CHECK: [[BOX:%.*]] = {{.*}}call {{.*}}__rust_alloc( - // CHECK: call void @llvm.memcpy.p0.p0.i64(ptr {{.*}}[[BOX]], ptr {{.*}}[[ALLOCA]] + // CHECK-NOT: call void @llvm.memcpy.p0.p0.i64(ptr {{.*}}[[BOX]], // CHECK: ret ptr [[BOX]] Box::default() } From c34b98f1bad5b994d8f4fc89eb88d7334a386379 Mon Sep 17 00:00:00 2001 From: Joshua Wong Date: Wed, 9 Oct 2024 14:24:05 -0400 Subject: [PATCH 3/3] allocate before calling T::default in >::default() Same rationale as in the previous commit. --- library/alloc/src/lib.rs | 1 + library/alloc/src/sync.rs | 8 +++++++- tests/codegen/placement-new.rs | 4 ++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index c60c0743c7e12..dcfe96be7551d 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -104,6 +104,7 @@ #![feature(async_closure)] #![feature(async_fn_traits)] #![feature(async_iterator)] +#![feature(box_uninit_write)] #![feature(clone_to_uninit)] #![feature(coerce_unsized)] #![feature(const_align_of_val)] diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 5d099a49854af..0038750d25dd1 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -3447,7 +3447,13 @@ impl Default for Arc { /// assert_eq!(*x, 0); /// ``` fn default() -> Arc { - Arc::new(Default::default()) + let x = Box::into_raw(Box::write(Box::new_uninit(), ArcInner { + strong: atomic::AtomicUsize::new(1), + weak: atomic::AtomicUsize::new(1), + data: T::default(), + })); + // SAFETY: `Box::into_raw` consumes the `Box` and never returns null + unsafe { Self::from_inner(NonNull::new_unchecked(x)) } } } diff --git a/tests/codegen/placement-new.rs b/tests/codegen/placement-new.rs index 1b49d780b5273..a102dd51db6a7 100644 --- a/tests/codegen/placement-new.rs +++ b/tests/codegen/placement-new.rs @@ -19,9 +19,9 @@ pub fn box_default_inplace() -> Box<(String, String)> { // CHECK-LABEL: @arc_default_inplace #[no_mangle] pub fn arc_default_inplace() -> Arc<(String, String)> { - // CHECK: [[ALLOCA:%.*]] = alloca + // CHECK-NOT: alloca // CHECK: [[ARC:%.*]] = {{.*}}call {{.*}}__rust_alloc( - // CHECK: call void @llvm.memcpy.p0.p0.i64( + // CHECK-NOT: call void @llvm.memcpy.p0.p0.i64( // CHECK: ret ptr [[ARC]] Arc::default() }