From 80faf203515bf3be214ad3ffc019b55b026b1220 Mon Sep 17 00:00:00 2001 From: Joshua Wong Date: Sun, 26 Jan 2025 03:23:02 -0500 Subject: [PATCH 1/2] add test for Box::default's stack usage in debug mode --- tests/codegen/box-default-debug-copies.rs | 25 +++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 tests/codegen/box-default-debug-copies.rs diff --git a/tests/codegen/box-default-debug-copies.rs b/tests/codegen/box-default-debug-copies.rs new file mode 100644 index 0000000000000..ed4e0c416b8dc --- /dev/null +++ b/tests/codegen/box-default-debug-copies.rs @@ -0,0 +1,25 @@ +//@ compile-flags: -Copt-level=0 + +// Test to make sure that `>::default` does not create too many copies of `T` on the stack. +// in debug mode. This regressed in dd0620b86721ae8cae86736443acd3f72ba6fc32 to +// four `T` allocas. +// +// See https://github.com/rust-lang/rust/issues/136043 for more context. + +#![crate_type = "lib"] + +#[allow(dead_code)] +pub struct Thing([u8; 1000000]); + +impl Default for Thing { + fn default() -> Self { + Thing([0; 1000000]) + } +} + +// CHECK-COUNT-4: %{{.*}} = alloca {{.*}}1000000 +// CHECK-NOT: %{{.*}} = alloca {{.*}}1000000 +#[no_mangle] +pub fn box_default_single_copy() -> Box { + Box::default() +} From 97005678c38fd391c9b502d011cc3f3d4434a18a Mon Sep 17 00:00:00 2001 From: Joshua Wong Date: Sun, 26 Jan 2025 03:36:50 -0500 Subject: [PATCH 2/2] reduce `Box::default` stack copies in debug mode The `Box::new(T::default())` implementation of `Box::default` only had two stack copies in debug mode, compared to the current version, which has four. By avoiding creating any `MaybeUninit`'s and just writing `T` directly to the `Box` pointer, the stack usage in debug mode remains the same as the old version. --- library/alloc/src/boxed.rs | 15 ++++++++++++++- tests/codegen/box-default-debug-copies.rs | 5 ++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 1b5e44a913467..8f43ebc388889 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -1730,7 +1730,20 @@ impl Default for Box { /// Creates a `Box`, with the `Default` value for T. #[inline] fn default() -> Self { - Box::write(Box::new_uninit(), T::default()) + let mut x: Box> = Box::new_uninit(); + unsafe { + // SAFETY: `x` is valid for writing and has the same layout as `T`. + // If `T::default()` panics, dropping `x` will just deallocate the Box as `MaybeUninit` + // does not have a destructor. + // + // We use `ptr::write` as `MaybeUninit::write` creates + // extra stack copies of `T` in debug mode. + // + // See https://github.com/rust-lang/rust/issues/136043 for more context. + ptr::write(&raw mut *x as *mut T, T::default()); + // SAFETY: `x` was just initialized above. + x.assume_init() + } } } diff --git a/tests/codegen/box-default-debug-copies.rs b/tests/codegen/box-default-debug-copies.rs index ed4e0c416b8dc..06cc41b21c06f 100644 --- a/tests/codegen/box-default-debug-copies.rs +++ b/tests/codegen/box-default-debug-copies.rs @@ -5,6 +5,9 @@ // four `T` allocas. // // See https://github.com/rust-lang/rust/issues/136043 for more context. +// +// FIXME: This test only wants to ensure that there are at most two allocas of `T` created, instead +// of checking for exactly two. #![crate_type = "lib"] @@ -17,7 +20,7 @@ impl Default for Thing { } } -// CHECK-COUNT-4: %{{.*}} = alloca {{.*}}1000000 +// CHECK-COUNT-2: %{{.*}} = alloca {{.*}}1000000 // CHECK-NOT: %{{.*}} = alloca {{.*}}1000000 #[no_mangle] pub fn box_default_single_copy() -> Box {