about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-05-22 15:06:32 +0000
committerbors <bors@rust-lang.org>2023-05-22 15:06:32 +0000
commit2fe47b966a8ee689d697583be4182262e7b4fd08 (patch)
tree2933eebcfa9f68a6bff70dab8ad086c7352bc06b
parent03761a50a3b26daded09e6da79252692c9bbad5f (diff)
parent2a466466c768dc56382550a3130674b2eebe227c (diff)
downloadrust-2fe47b966a8ee689d697583be4182262e7b4fd08.tar.gz
rust-2fe47b966a8ee689d697583be4182262e7b4fd08.zip
Auto merge of #111634 - marc0246:arc-new-uninit-bloat, r=thomcc
Fix duplicate `arcinner_layout_for_value_layout` calls when using the uninit `Arc` constructors

What this fixes is the duplicate calls to `arcinner_layout_for_value_layout` seen here: https://godbolt.org/z/jr5Gxozhj

The issue was discovered alongside #111603 but is otherwise unrelated to the duplicate `alloca`s, which remain unsolved. Everything I tried to solve said main issue has failed.

As for the duplicate layout calculations, I also tried slapping `#[inline]` and `#[inline(always)]` on everything in sight but the only thing that worked in the end is to dedup the calls by hand.
-rw-r--r--library/alloc/src/sync.rs25
-rw-r--r--tests/codegen/issues/issue-111603.rs28
2 files changed, 47 insertions, 6 deletions
diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs
index 7347980abbc..bfdb7a92bef 100644
--- a/library/alloc/src/sync.rs
+++ b/library/alloc/src/sync.rs
@@ -502,6 +502,7 @@ impl<T> Arc<T> {
     /// assert_eq!(*five, 5)
     /// ```
     #[cfg(not(no_global_oom_handling))]
+    #[inline]
     #[unstable(feature = "new_uninit", issue = "63291")]
     #[must_use]
     pub fn new_uninit() -> Arc<mem::MaybeUninit<T>> {
@@ -535,6 +536,7 @@ impl<T> Arc<T> {
     ///
     /// [zeroed]: mem::MaybeUninit::zeroed
     #[cfg(not(no_global_oom_handling))]
+    #[inline]
     #[unstable(feature = "new_uninit", issue = "63291")]
     #[must_use]
     pub fn new_zeroed() -> Arc<mem::MaybeUninit<T>> {
@@ -844,6 +846,7 @@ impl<T> Arc<[T]> {
     /// assert_eq!(*values, [1, 2, 3])
     /// ```
     #[cfg(not(no_global_oom_handling))]
+    #[inline]
     #[unstable(feature = "new_uninit", issue = "63291")]
     #[must_use]
     pub fn new_uninit_slice(len: usize) -> Arc<[mem::MaybeUninit<T>]> {
@@ -871,6 +874,7 @@ impl<T> Arc<[T]> {
     ///
     /// [zeroed]: mem::MaybeUninit::zeroed
     #[cfg(not(no_global_oom_handling))]
+    #[inline]
     #[unstable(feature = "new_uninit", issue = "63291")]
     #[must_use]
     pub fn new_zeroed_slice(len: usize) -> Arc<[mem::MaybeUninit<T>]> {
@@ -1300,10 +1304,10 @@ impl<T: ?Sized> Arc<T> {
         mem_to_arcinner: impl FnOnce(*mut u8) -> *mut ArcInner<T>,
     ) -> *mut ArcInner<T> {
         let layout = arcinner_layout_for_value_layout(value_layout);
-        unsafe {
-            Arc::try_allocate_for_layout(value_layout, allocate, mem_to_arcinner)
-                .unwrap_or_else(|_| handle_alloc_error(layout))
-        }
+
+        let ptr = allocate(layout).unwrap_or_else(|_| handle_alloc_error(layout));
+
+        unsafe { Self::initialize_arcinner(ptr, layout, mem_to_arcinner) }
     }
 
     /// Allocates an `ArcInner<T>` with sufficient space for
@@ -1321,7 +1325,16 @@ impl<T: ?Sized> Arc<T> {
 
         let ptr = allocate(layout)?;
 
-        // Initialize the ArcInner
+        let inner = unsafe { Self::initialize_arcinner(ptr, layout, mem_to_arcinner) };
+
+        Ok(inner)
+    }
+
+    unsafe fn initialize_arcinner(
+        ptr: NonNull<[u8]>,
+        layout: Layout,
+        mem_to_arcinner: impl FnOnce(*mut u8) -> *mut ArcInner<T>,
+    ) -> *mut ArcInner<T> {
         let inner = mem_to_arcinner(ptr.as_non_null_ptr().as_ptr());
         debug_assert_eq!(unsafe { Layout::for_value(&*inner) }, layout);
 
@@ -1330,7 +1343,7 @@ impl<T: ?Sized> Arc<T> {
             ptr::write(&mut (*inner).weak, atomic::AtomicUsize::new(1));
         }
 
-        Ok(inner)
+        inner
     }
 
     /// Allocates an `ArcInner<T>` with sufficient space for an unsized inner value.
diff --git a/tests/codegen/issues/issue-111603.rs b/tests/codegen/issues/issue-111603.rs
new file mode 100644
index 00000000000..90b3c314d2f
--- /dev/null
+++ b/tests/codegen/issues/issue-111603.rs
@@ -0,0 +1,28 @@
+// compile-flags: -O
+
+#![crate_type = "lib"]
+#![feature(get_mut_unchecked, new_uninit)]
+
+use std::sync::Arc;
+
+// CHECK-LABEL: @new_uninit
+#[no_mangle]
+pub fn new_uninit(x: u64) -> Arc<[u64; 1000]> {
+    // CHECK: call alloc::sync::arcinner_layout_for_value_layout
+    // CHECK-NOT: call alloc::sync::arcinner_layout_for_value_layout
+    let mut arc = Arc::new_uninit();
+    unsafe { Arc::get_mut_unchecked(&mut arc) }.write([x; 1000]);
+    unsafe { arc.assume_init() }
+}
+
+// CHECK-LABEL: @new_uninit_slice
+#[no_mangle]
+pub fn new_uninit_slice(x: u64) -> Arc<[u64]> {
+    // CHECK: call alloc::sync::arcinner_layout_for_value_layout
+    // CHECK-NOT: call alloc::sync::arcinner_layout_for_value_layout
+    let mut arc = Arc::new_uninit_slice(1000);
+    for elem in unsafe { Arc::get_mut_unchecked(&mut arc) } {
+        elem.write(x);
+    }
+    unsafe { arc.assume_init() }
+}