Skip to content

Commit

Permalink
Take advantage of known-valid-align in layout.rs
Browse files Browse the repository at this point in the history
  • Loading branch information
CAD97 committed Jul 11, 2022
1 parent 29554c0 commit 079d3eb
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 15 deletions.
41 changes: 27 additions & 14 deletions library/core/src/alloc/layout.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
// Seemingly inconsequential code changes to this file can lead to measurable
// performance impact on compilation times, due at least in part to the fact
// that the layout code gets called from many instantiations of the various
// collections, resulting in having to optimize down excess IR multiple times.
// Your performance intuition is useless. Run perf.

use crate::cmp;
use crate::fmt;
use crate::mem::{self, ValidAlign};
Expand Down Expand Up @@ -85,6 +91,17 @@ impl Layout {
unsafe { Ok(Layout::from_size_align_unchecked(size, align)) }
}

/// Internal helper constructor to skip revalidating alignment validity.
#[inline]
const fn from_size_valid_align(size: usize, align: ValidAlign) -> Result<Self, LayoutError> {
// See above for the correctness of this check.
if size > isize::MAX as usize - (align.as_nonzero().get() - 1) {
return Err(LayoutError);
}
// SAFTEY: as above, this check is sufficient.
Ok(Layout { size, align })
}

/// Creates a layout, bypassing all checks.
///
/// # Safety
Expand Down Expand Up @@ -126,10 +143,9 @@ impl Layout {
#[inline]
pub const fn new<T>() -> Self {
let (size, align) = size_align::<T>();
// SAFETY: the align is guaranteed by Rust to be a power of two and
// the size+align combo is guaranteed to fit in our address space. As a
// result use the unchecked constructor here to avoid inserting code
// that panics if it isn't optimized well enough.
// SAFETY: if the type is instantiated, rustc already ensures that its
// layout is valid. Use the unchecked constructor to avoid inserting a
// panicking codepath that needs to be optimized out.
unsafe { Layout::from_size_align_unchecked(size, align) }
}

Expand All @@ -141,7 +157,6 @@ impl Layout {
#[inline]
pub fn for_value<T: ?Sized>(t: &T) -> Self {
let (size, align) = (mem::size_of_val(t), mem::align_of_val(t));
debug_assert!(Layout::from_size_align(size, align).is_ok());
// SAFETY: see rationale in `new` for why this is using the unsafe variant
unsafe { Layout::from_size_align_unchecked(size, align) }
}
Expand Down Expand Up @@ -176,7 +191,6 @@ impl Layout {
pub unsafe fn for_value_raw<T: ?Sized>(t: *const T) -> Self {
// SAFETY: we pass along the prerequisites of these functions to the caller
let (size, align) = unsafe { (mem::size_of_val_raw(t), mem::align_of_val_raw(t)) };
debug_assert!(Layout::from_size_align(size, align).is_ok());
// SAFETY: see rationale in `new` for why this is using the unsafe variant
unsafe { Layout::from_size_align_unchecked(size, align) }
}
Expand Down Expand Up @@ -280,8 +294,7 @@ impl Layout {
// > less than or equal to `isize::MAX`)
let new_size = self.size() + pad;

// SAFETY: self.align is already known to be valid and new_size has been
// padded already.
// SAFETY: padded size is guaranteed to not exceed `isize::MAX`.
unsafe { Layout::from_size_align_unchecked(new_size, self.align()) }
}

Expand All @@ -304,7 +317,7 @@ impl Layout {
let alloc_size = padded_size.checked_mul(n).ok_or(LayoutError)?;

// The safe constructor is called here to enforce the isize size limit.
Layout::from_size_align(alloc_size, self.align()).map(|layout| (layout, padded_size))
Layout::from_size_valid_align(alloc_size, self.align).map(|layout| (layout, padded_size))
}

/// Creates a layout describing the record for `self` followed by
Expand Down Expand Up @@ -355,14 +368,14 @@ impl Layout {
#[stable(feature = "alloc_layout_manipulation", since = "1.44.0")]
#[inline]
pub fn extend(&self, next: Self) -> Result<(Self, usize), LayoutError> {
let new_align = cmp::max(self.align(), next.align());
let new_align = cmp::max(self.align, next.align);
let pad = self.padding_needed_for(next.align());

let offset = self.size().checked_add(pad).ok_or(LayoutError)?;
let new_size = offset.checked_add(next.size()).ok_or(LayoutError)?;

// The safe constructor is called here to enforce the isize size limit.
let layout = Layout::from_size_align(new_size, new_align)?;
let layout = Layout::from_size_valid_align(new_size, new_align)?;
Ok((layout, offset))
}

Expand All @@ -383,7 +396,7 @@ impl Layout {
pub fn repeat_packed(&self, n: usize) -> Result<Self, LayoutError> {
let size = self.size().checked_mul(n).ok_or(LayoutError)?;
// The safe constructor is called here to enforce the isize size limit.
Layout::from_size_align(size, self.align())
Layout::from_size_valid_align(size, self.align)
}

/// Creates a layout describing the record for `self` followed by
Expand All @@ -397,7 +410,7 @@ impl Layout {
pub fn extend_packed(&self, next: Self) -> Result<Self, LayoutError> {
let new_size = self.size().checked_add(next.size()).ok_or(LayoutError)?;
// The safe constructor is called here to enforce the isize size limit.
Layout::from_size_align(new_size, self.align())
Layout::from_size_valid_align(new_size, self.align)
}

/// Creates a layout describing the record for a `[T; n]`.
Expand All @@ -408,7 +421,7 @@ impl Layout {
pub fn array<T>(n: usize) -> Result<Self, LayoutError> {
let array_size = mem::size_of::<T>().checked_mul(n).ok_or(LayoutError)?;
// The safe constructor is called here to enforce the isize size limit.
Layout::from_size_align(array_size, mem::align_of::<T>())
Layout::from_size_valid_align(array_size, ValidAlign::of::<T>())
}
}

Expand Down
11 changes: 10 additions & 1 deletion library/core/src/mem/valid_align.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::convert::TryFrom;
use crate::intrinsics::assert_unsafe_precondition;
use crate::num::NonZeroUsize;
use crate::{cmp, fmt, hash, mem, num};

Expand Down Expand Up @@ -26,7 +27,8 @@ impl ValidAlign {
/// It must *not* be zero.
#[inline]
pub(crate) const unsafe fn new_unchecked(align: usize) -> Self {
debug_assert!(align.is_power_of_two());
// SAFETY: Precondition passed to the caller.
unsafe { assert_unsafe_precondition!(align.is_power_of_two()) };

// SAFETY: By precondition, this must be a power of two, and
// our variants encompass all possible powers of two.
Expand All @@ -46,6 +48,13 @@ impl ValidAlign {
pub(crate) fn log2(self) -> u32 {
self.as_nonzero().trailing_zeros()
}

/// Returns the alignment for a type.
#[inline]
pub(crate) fn of<T>() -> Self {
// SAFETY: rustc ensures that type alignment is always a power of two.
unsafe { ValidAlign::new_unchecked(mem::align_of::<T>()) }
}
}

impl fmt::Debug for ValidAlign {
Expand Down

0 comments on commit 079d3eb

Please sign in to comment.