Skip to content

Commit

Permalink
Rollup merge of rust-lang#103287 - saethlin:faster-len-check, r=thomcc
Browse files Browse the repository at this point in the history
Use a faster allocation size check in slice::from_raw_parts

I've been perusing through the codegen changes that result from turning on the standard library debug assertions. The previous check in here uses saturating arithmetic, which in my experience sometimes makes LLVM just fail to optimize things around the saturating operation.

Here is a demo of the codegen difference: https://godbolt.org/z/WMEqrjajW
Before:
```asm
example::len_check_old:
        mov     rax, rdi
        mov     ecx, 3
        mul     rcx
        setno   cl
        test    rax, rax
        setns   al
        and     al, cl
        ret

example::len_check_old:
        mov     rax, rdi
        mov     ecx, 8
        mul     rcx
        setno   cl
        test    rax, rax
        setns   al
        and     al, cl
        ret
```
After:
```asm
example::len_check_new:
        movabs  rax, 3074457345618258603
        cmp     rdi, rax
        setb    al
        ret

example::len_check_new:
        shr     rdi, 60
        sete    al
        ret
```

Running rustc-perf locally, this looks like up to a 4.5% improvement when `debug-assertions-std = true`.

Thanks ``@LegionMammal978`` (I think that's you?) for turning my idea into a much cleaner implementation.

r? ``@thomcc``
  • Loading branch information
Dylan-DPC authored Oct 25, 2022
2 parents c24f914 + cfcb0a2 commit ccd39fd
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 5 deletions.
10 changes: 10 additions & 0 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2229,6 +2229,16 @@ pub(crate) fn is_aligned_and_not_null<T>(ptr: *const T) -> bool {
!ptr.is_null() && ptr.is_aligned()
}

/// Checks whether an allocation of `len` instances of `T` exceeds
/// the maximum allowed allocation size.
pub(crate) fn is_valid_allocation_size<T>(len: usize) -> bool {
let max_len = const {
let size = crate::mem::size_of::<T>();
if size == 0 { usize::MAX } else { isize::MAX as usize / size }
};
len <= max_len
}

/// Checks whether the regions of memory starting at `src` and `dst` of size
/// `count * size_of::<T>()` do *not* overlap.
pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, count: usize) -> bool {
Expand Down
1 change: 1 addition & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@
#![feature(extern_types)]
#![feature(fundamental)]
#![feature(if_let_guard)]
#![feature(inline_const)]
#![feature(intra_doc_pointers)]
#![feature(intrinsics)]
#![feature(lang_items)]
Expand Down
10 changes: 5 additions & 5 deletions library/core/src/slice/raw.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
//! Free functions to create `&[T]` and `&mut [T]`.
use crate::array;
use crate::intrinsics::{assert_unsafe_precondition, is_aligned_and_not_null};
use crate::intrinsics::{
assert_unsafe_precondition, is_aligned_and_not_null, is_valid_allocation_size,
};
use crate::ops::Range;
use crate::ptr;

Expand Down Expand Up @@ -91,8 +93,7 @@ pub const unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T]
// SAFETY: the caller must uphold the safety contract for `from_raw_parts`.
unsafe {
assert_unsafe_precondition!([T](data: *const T, len: usize) =>
is_aligned_and_not_null(data)
&& crate::mem::size_of::<T>().saturating_mul(len) <= isize::MAX as usize
is_aligned_and_not_null(data) && is_valid_allocation_size::<T>(len)
);
&*ptr::slice_from_raw_parts(data, len)
}
Expand Down Expand Up @@ -135,8 +136,7 @@ pub const unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a m
// SAFETY: the caller must uphold the safety contract for `from_raw_parts_mut`.
unsafe {
assert_unsafe_precondition!([T](data: *mut T, len: usize) =>
is_aligned_and_not_null(data)
&& crate::mem::size_of::<T>().saturating_mul(len) <= isize::MAX as usize
is_aligned_and_not_null(data) && is_valid_allocation_size::<T>(len)
);
&mut *ptr::slice_from_raw_parts_mut(data, len)
}
Expand Down

0 comments on commit ccd39fd

Please sign in to comment.