Skip to content

Commit

Permalink
Auto merge of rust-lang#111634 - marc0246:arc-new-uninit-bloat, r=thomcc
Browse files Browse the repository at this point in the history
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 rust-lang#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.
  • Loading branch information
bors committed May 22, 2023
2 parents 03761a5 + 2a46646 commit 2fe47b9
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 6 deletions.
25 changes: 19 additions & 6 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>> {
Expand Down Expand Up @@ -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>> {
Expand Down Expand Up @@ -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>]> {
Expand Down Expand Up @@ -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>]> {
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand All @@ -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.
Expand Down
28 changes: 28 additions & 0 deletions tests/codegen/issues/issue-111603.rs
Original file line number Diff line number Diff line change
@@ -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() }
}

0 comments on commit 2fe47b9

Please sign in to comment.