Skip to content

Commit

Permalink
Auto merge of #77023 - HeroicKatora:len-missed-optimization, r=Mark-S…
Browse files Browse the repository at this point in the history
…imulacrum

Hint the maximum length permitted by invariant of slices

One of the safety invariants of references, and in particular of references to slices, is that they may not cover more than `isize::MAX` bytes. The unsafe `from_raw_parts` constructors of slices explicitly requires the caller to guarantee this fact. Violating it would also be UB with regards to the semantics of generated llvm code.

This effectively bounds the length of a (non-ZST) slice from above by a compile time constant. But when the length is loaded from a function argument it appears llvm is not aware of this requirement. The additional value range assertions allow some further elision of code branches, including overflow checks, especially in the presence of artithmetic on the indices.

This may have a performance impact, adding more code to a common method but allowing more optimization. I'm not quite sure, is the Rust side of const-prop strong enough to elide the irrelevant match branches?

Fixes: #67186
  • Loading branch information
bors committed Oct 4, 2020
2 parents 4ccf5f7 + e44784b commit beb5ae4
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 3 deletions.
3 changes: 2 additions & 1 deletion library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,9 @@
#![feature(constctlz)]
#![feature(const_panic)]
#![feature(const_pin)]
#![feature(const_fn_union)]
#![feature(const_fn)]
#![feature(const_fn_union)]
#![feature(const_assume)]
#![cfg_attr(not(bootstrap), feature(const_fn_floating_point_arithmetic))]
#![cfg_attr(not(bootstrap), feature(const_fn_fn_ptr_basics))]
#![feature(const_generics)]
Expand Down
27 changes: 25 additions & 2 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ pub use index::check_range;
#[lang = "slice"]
#[cfg(not(test))]
impl<T> [T] {
#[cfg(not(bootstrap))] // Unused in bootstrap
/// The maximum, inclusive, length such that the slice is no larger than `isize::MAX` bytes.
/// This constant is used in `len` below.
const MAX_LEN_BOUND: usize = {
if mem::size_of::<T>() == 0 {
usize::MAX
} else {
isize::MAX as usize / mem::size_of::<T>()
}
};

/// Returns the number of elements in the slice.
///
/// # Examples
Expand All @@ -90,11 +101,23 @@ impl<T> [T] {
#[rustc_const_stable(feature = "const_slice_len", since = "1.32.0")]
#[inline]
// SAFETY: const sound because we transmute out the length field as a usize (which it must be)
#[allow_internal_unstable(const_fn_union)]
#[allow_internal_unstable(const_fn_union, const_assume)]
pub const fn len(&self) -> usize {
// SAFETY: this is safe because `&[T]` and `FatPtr<T>` have the same layout.
// Only `std` can make this guarantee.
unsafe { crate::ptr::Repr { rust: self }.raw.len }
let raw_len = unsafe { crate::ptr::Repr { rust: self }.raw.len };

#[cfg(not(bootstrap))] // FIXME: executing assume in const eval not supported in bootstrap
// SAFETY: this assume asserts that `raw_len * size_of::<T>() <= isize::MAX`. All
// references must point to one allocation with size at most isize::MAX. Note that we the
// multiplication could appear to overflow until we have assumed the bound. This overflow
// would make additional values appear 'valid' and then `n > 1` the range of permissible
// length would no longer be the full or even a single range.
unsafe {
crate::intrinsics::assume(raw_len <= Self::MAX_LEN_BOUND)
};

raw_len
}

/// Returns `true` if the slice has a length of 0.
Expand Down
24 changes: 24 additions & 0 deletions src/test/codegen/len-is-bounded.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// min-llvm-version: 11.0
// compile-flags: -O -C panic=abort
#![crate_type = "lib"]

#[no_mangle]
pub fn len_range(a: &[u8], b: &[u8]) -> usize {
// CHECK-NOT: panic
a.len().checked_add(b.len()).unwrap()
}

#[no_mangle]
pub fn len_range_on_non_byte(a: &[u16], b: &[u16]) -> usize {
// CHECK-NOT: panic
a.len().checked_add(b.len()).unwrap()
}

pub struct Zst;

#[no_mangle]
pub fn zst_range(a: &[Zst], b: &[Zst]) -> usize {
// Zsts may be arbitrarily large.
// CHECK: panic
a.len().checked_add(b.len()).unwrap()
}

0 comments on commit beb5ae4

Please sign in to comment.