From 079d3eb22f883edb0ef332a4929a00513112cb6a Mon Sep 17 00:00:00 2001 From: Christopher Durham Date: Sun, 10 Jul 2022 19:23:58 -0400 Subject: [PATCH] Take advantage of known-valid-align in layout.rs --- library/core/src/alloc/layout.rs | 41 +++++++++++++++++++---------- library/core/src/mem/valid_align.rs | 11 +++++++- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/library/core/src/alloc/layout.rs b/library/core/src/alloc/layout.rs index 39bccdb854c30..51e075546fb3c 100644 --- a/library/core/src/alloc/layout.rs +++ b/library/core/src/alloc/layout.rs @@ -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}; @@ -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 { + // 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 @@ -126,10 +143,9 @@ impl Layout { #[inline] pub const fn new() -> Self { let (size, align) = size_align::(); - // 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) } } @@ -141,7 +157,6 @@ impl Layout { #[inline] pub fn for_value(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) } } @@ -176,7 +191,6 @@ impl Layout { pub unsafe fn for_value_raw(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) } } @@ -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()) } } @@ -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 @@ -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)) } @@ -383,7 +396,7 @@ impl Layout { pub fn repeat_packed(&self, n: usize) -> Result { 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 @@ -397,7 +410,7 @@ impl Layout { pub fn extend_packed(&self, next: Self) -> Result { 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]`. @@ -408,7 +421,7 @@ impl Layout { pub fn array(n: usize) -> Result { let array_size = mem::size_of::().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::()) + Layout::from_size_valid_align(array_size, ValidAlign::of::()) } } diff --git a/library/core/src/mem/valid_align.rs b/library/core/src/mem/valid_align.rs index fcfa95120df21..4ce6d13cf9027 100644 --- a/library/core/src/mem/valid_align.rs +++ b/library/core/src/mem/valid_align.rs @@ -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}; @@ -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. @@ -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() -> Self { + // SAFETY: rustc ensures that type alignment is always a power of two. + unsafe { ValidAlign::new_unchecked(mem::align_of::()) } + } } impl fmt::Debug for ValidAlign {