Skip to content

rebase "Add some safety comments" #217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions crates/core_simd/src/comparisons.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@ where
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
pub fn lanes_eq(self, other: Self) -> Mask<T::Mask, LANES> {
// Safety: `self` is a vector, and the result of the comparison
// is always a valid mask.
unsafe { Mask::from_int_unchecked(intrinsics::simd_eq(self, other)) }
}

/// Test if each lane is not equal to the corresponding lane in `other`.
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
pub fn lanes_ne(self, other: Self) -> Mask<T::Mask, LANES> {
// Safety: `self` is a vector, and the result of the comparison
// is always a valid mask.
unsafe { Mask::from_int_unchecked(intrinsics::simd_ne(self, other)) }
}
}
Expand All @@ -30,27 +34,35 @@ where
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
pub fn lanes_lt(self, other: Self) -> Mask<T::Mask, LANES> {
// Safety: `self` is a vector, and the result of the comparison
// is always a valid mask.
unsafe { Mask::from_int_unchecked(intrinsics::simd_lt(self, other)) }
}

/// Test if each lane is greater than the corresponding lane in `other`.
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
pub fn lanes_gt(self, other: Self) -> Mask<T::Mask, LANES> {
// Safety: `self` is a vector, and the result of the comparison
// is always a valid mask.
unsafe { Mask::from_int_unchecked(intrinsics::simd_gt(self, other)) }
}

/// Test if each lane is less than or equal to the corresponding lane in `other`.
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
pub fn lanes_le(self, other: Self) -> Mask<T::Mask, LANES> {
// Safety: `self` is a vector, and the result of the comparison
// is always a valid mask.
unsafe { Mask::from_int_unchecked(intrinsics::simd_le(self, other)) }
}

/// Test if each lane is greater than or equal to the corresponding lane in `other`.
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
pub fn lanes_ge(self, other: Self) -> Mask<T::Mask, LANES> {
// Safety: `self` is a vector, and the result of the comparison
// is always a valid mask.
unsafe { Mask::from_int_unchecked(intrinsics::simd_ge(self, other)) }
}
}
9 changes: 9 additions & 0 deletions crates/core_simd/src/masks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ mod sealed {
use sealed::Sealed;

/// Marker trait for types that may be used as SIMD mask elements.
///
/// # Safety
/// Type must be a signed integer.
pub unsafe trait MaskElement: SimdElement + Sealed {}

macro_rules! impl_element {
Expand Down Expand Up @@ -149,6 +152,7 @@ where
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
pub unsafe fn from_int_unchecked(value: Simd<T, LANES>) -> Self {
// Safety: the caller must confirm this invariant
unsafe { Self(mask_impl::Mask::from_int_unchecked(value)) }
}

Expand All @@ -161,6 +165,7 @@ where
#[must_use = "method returns a new mask and does not mutate the original value"]
pub fn from_int(value: Simd<T, LANES>) -> Self {
assert!(T::valid(value), "all values must be either 0 or -1",);
// Safety: the validity has been checked
unsafe { Self::from_int_unchecked(value) }
}

Expand All @@ -179,6 +184,7 @@ where
#[inline]
#[must_use = "method returns a new bool and does not mutate the original value"]
pub unsafe fn test_unchecked(&self, lane: usize) -> bool {
// Safety: the caller must confirm this invariant
unsafe { self.0.test_unchecked(lane) }
}

Expand All @@ -190,6 +196,7 @@ where
#[must_use = "method returns a new bool and does not mutate the original value"]
pub fn test(&self, lane: usize) -> bool {
assert!(lane < LANES, "lane index out of range");
// Safety: the lane index has been checked
unsafe { self.test_unchecked(lane) }
}

Expand All @@ -199,6 +206,7 @@ where
/// `lane` must be less than `LANES`.
#[inline]
pub unsafe fn set_unchecked(&mut self, lane: usize, value: bool) {
// Safety: the caller must confirm this invariant
unsafe {
self.0.set_unchecked(lane, value);
}
Expand All @@ -211,6 +219,7 @@ where
#[inline]
pub fn set(&mut self, lane: usize, value: bool) {
assert!(lane < LANES, "lane index out of range");
// Safety: the lane index has been checked
unsafe {
self.set_unchecked(lane, value);
}
Expand Down
1 change: 1 addition & 0 deletions crates/core_simd/src/masks/bitmask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ where
where
U: MaskElement,
{
// Safety: bitmask layout does not depend on the element width
unsafe { core::mem::transmute_copy(&self) }
}

Expand Down
6 changes: 6 additions & 0 deletions crates/core_simd/src/masks/full_masks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ where
where
U: MaskElement,
{
// Safety: masks are simply integer vectors of 0 and -1, and we can cast the element type.
unsafe { Mask(intrinsics::simd_cast(self.0)) }
}

Expand Down Expand Up @@ -155,12 +156,14 @@ where
#[inline]
#[must_use = "method returns a new bool and does not mutate the original value"]
pub fn any(self) -> bool {
// Safety: use `self` as an integer vector
unsafe { intrinsics::simd_reduce_any(self.to_int()) }
}

#[inline]
#[must_use = "method returns a new vector and does not mutate the original value"]
pub fn all(self) -> bool {
// Safety: use `self` as an integer vector
unsafe { intrinsics::simd_reduce_all(self.to_int()) }
}
}
Expand All @@ -184,6 +187,7 @@ where
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
fn bitand(self, rhs: Self) -> Self {
// Safety: `self` is an integer vector
unsafe { Self(intrinsics::simd_and(self.0, rhs.0)) }
}
}
Expand All @@ -197,6 +201,7 @@ where
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
fn bitor(self, rhs: Self) -> Self {
// Safety: `self` is an integer vector
unsafe { Self(intrinsics::simd_or(self.0, rhs.0)) }
}
}
Expand All @@ -210,6 +215,7 @@ where
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
fn bitxor(self, rhs: Self) -> Self {
// Safety: `self` is an integer vector
unsafe { Self(intrinsics::simd_xor(self.0, rhs.0)) }
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/core_simd/src/math.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ macro_rules! impl_uint_arith {
/// ```
#[inline]
pub fn saturating_add(self, second: Self) -> Self {
// Safety: `self` is a vector
unsafe { simd_saturating_add(self, second) }
}

Expand All @@ -41,6 +42,7 @@ macro_rules! impl_uint_arith {
/// assert_eq!(sat, Simd::splat(0));
#[inline]
pub fn saturating_sub(self, second: Self) -> Self {
// Safety: `self` is a vector
unsafe { simd_saturating_sub(self, second) }
}
})+
Expand Down Expand Up @@ -68,6 +70,7 @@ macro_rules! impl_int_arith {
/// ```
#[inline]
pub fn saturating_add(self, second: Self) -> Self {
// Safety: `self` is a vector
unsafe { simd_saturating_add(self, second) }
}

Expand All @@ -87,6 +90,7 @@ macro_rules! impl_int_arith {
/// assert_eq!(sat, Simd::from_array([MIN, MIN, MIN, 0]));
#[inline]
pub fn saturating_sub(self, second: Self) -> Self {
// Safety: `self` is a vector
unsafe { simd_saturating_sub(self, second) }
}

Expand Down
8 changes: 8 additions & 0 deletions crates/core_simd/src/reduction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,28 @@ macro_rules! impl_integer_reductions {
/// Horizontal wrapping add. Returns the sum of the lanes of the vector, with wrapping addition.
#[inline]
pub fn horizontal_sum(self) -> $scalar {
// Safety: `self` is an integer vector
unsafe { simd_reduce_add_ordered(self, 0) }
}

/// Horizontal wrapping multiply. Returns the product of the lanes of the vector, with wrapping multiplication.
#[inline]
pub fn horizontal_product(self) -> $scalar {
// Safety: `self` is an integer vector
unsafe { simd_reduce_mul_ordered(self, 1) }
}

/// Horizontal maximum. Returns the maximum lane in the vector.
#[inline]
pub fn horizontal_max(self) -> $scalar {
// Safety: `self` is an integer vector
unsafe { simd_reduce_max(self) }
}

/// Horizontal minimum. Returns the minimum lane in the vector.
#[inline]
pub fn horizontal_min(self) -> $scalar {
// Safety: `self` is an integer vector
unsafe { simd_reduce_min(self) }
}
}
Expand Down Expand Up @@ -63,6 +67,7 @@ macro_rules! impl_float_reductions {
if cfg!(all(target_arch = "x86", not(target_feature = "sse2"))) {
self.as_array().iter().sum()
} else {
// Safety: `self` is a float vector
unsafe { simd_reduce_add_ordered(self, 0.) }
}
}
Expand All @@ -74,6 +79,7 @@ macro_rules! impl_float_reductions {
if cfg!(all(target_arch = "x86", not(target_feature = "sse2"))) {
self.as_array().iter().product()
} else {
// Safety: `self` is a float vector
unsafe { simd_reduce_mul_ordered(self, 1.) }
}
}
Expand All @@ -84,6 +90,7 @@ macro_rules! impl_float_reductions {
/// return either. This function will not return `NaN` unless all lanes are `NaN`.
#[inline]
pub fn horizontal_max(self) -> $scalar {
// Safety: `self` is a float vector
unsafe { simd_reduce_max(self) }
}

Expand All @@ -93,6 +100,7 @@ macro_rules! impl_float_reductions {
/// return either. This function will not return `NaN` unless all lanes are `NaN`.
#[inline]
pub fn horizontal_min(self) -> $scalar {
// Safety: `self` is a float vector
unsafe { simd_reduce_min(self) }
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/core_simd/src/swizzle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ pub trait Swizzle<const INPUT_LANES: usize, const OUTPUT_LANES: usize> {
LaneCount<INPUT_LANES>: SupportedLaneCount,
LaneCount<OUTPUT_LANES>: SupportedLaneCount,
{
// Safety: `vector` is a vector, and `INDEX_IMPL` is a const array of u32.
unsafe { intrinsics::simd_shuffle(vector, vector, Self::INDEX_IMPL) }
}
}
Expand All @@ -119,6 +120,7 @@ pub trait Swizzle2<const INPUT_LANES: usize, const OUTPUT_LANES: usize> {
LaneCount<INPUT_LANES>: SupportedLaneCount,
LaneCount<OUTPUT_LANES>: SupportedLaneCount,
{
// Safety: `first` and `second` are vectors, and `INDEX_IMPL` is a const array of u32.
unsafe { intrinsics::simd_shuffle(first, second, Self::INDEX_IMPL) }
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/core_simd/src/to_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ macro_rules! impl_to_bytes {
/// Return the memory representation of this integer as a byte array in native byte
/// order.
pub fn to_ne_bytes(self) -> crate::simd::Simd<u8, {{ $size * LANES }}> {
// Safety: transmuting between vectors is safe
unsafe { core::mem::transmute_copy(&self) }
}

/// Create a native endian integer value from its memory representation as a byte array
/// in native endianness.
pub fn from_ne_bytes(bytes: crate::simd::Simd<u8, {{ $size * LANES }}>) -> Self {
// Safety: transmuting between vectors is safe
unsafe { core::mem::transmute_copy(&bytes) }
}
}
Expand Down
12 changes: 7 additions & 5 deletions crates/core_simd/src/vector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ where
or: Self,
) -> Self {
let enable: Mask<isize, LANES> = enable & idxs.lanes_lt(Simd::splat(slice.len()));
// SAFETY: We have masked-off out-of-bounds lanes.
// Safety: We have masked-off out-of-bounds lanes.
unsafe { Self::gather_select_unchecked(slice, enable, idxs, or) }
}

Expand Down Expand Up @@ -247,7 +247,7 @@ where
let base_ptr = crate::simd::ptr::SimdConstPtr::splat(slice.as_ptr());
// Ferris forgive me, I have done pointer arithmetic here.
let ptrs = base_ptr.wrapping_add(idxs);
// SAFETY: The ptrs have been bounds-masked to prevent memory-unsafe reads insha'allah
// Safety: The ptrs have been bounds-masked to prevent memory-unsafe reads insha'allah
unsafe { intrinsics::simd_gather(or, ptrs, enable.to_int()) }
}

Expand Down Expand Up @@ -299,7 +299,7 @@ where
idxs: Simd<usize, LANES>,
) {
let enable: Mask<isize, LANES> = enable & idxs.lanes_lt(Simd::splat(slice.len()));
// SAFETY: We have masked-off out-of-bounds lanes.
// Safety: We have masked-off out-of-bounds lanes.
unsafe { self.scatter_select_unchecked(slice, enable, idxs) }
}

Expand Down Expand Up @@ -338,7 +338,7 @@ where
enable: Mask<isize, LANES>,
idxs: Simd<usize, LANES>,
) {
// SAFETY: This block works with *mut T derived from &mut 'a [T],
// Safety: This block works with *mut T derived from &mut 'a [T],
// which means it is delicate in Rust's borrowing model, circa 2021:
// &mut 'a [T] asserts uniqueness, so deriving &'a [T] invalidates live *mut Ts!
// Even though this block is largely safe methods, it must be exactly this way
Expand Down Expand Up @@ -518,7 +518,9 @@ mod sealed {
use sealed::Sealed;

/// Marker trait for types that may be used as SIMD vector elements.
/// SAFETY: This trait, when implemented, asserts the compiler can monomorphize
///
/// # Safety
/// This trait, when implemented, asserts the compiler can monomorphize
/// `#[repr(simd)]` structs with the marked type as an element.
/// Strictly, it is valid to impl if the vector will not be miscompiled.
/// Practically, it is user-unfriendly to impl it if the vector won't compile,
Expand Down
4 changes: 4 additions & 0 deletions crates/core_simd/src/vector/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ where
#[inline]
#[must_use]
pub fn wrapping_add(self, addend: Simd<usize, LANES>) -> Self {
// Safety: converting pointers to usize and vice-versa is safe
// (even if using that pointer is not)
unsafe {
let x: Simd<usize, LANES> = mem::transmute_copy(&self);
mem::transmute_copy(&{ x + (addend * Simd::splat(mem::size_of::<T>())) })
Expand All @@ -47,6 +49,8 @@ where
#[inline]
#[must_use]
pub fn wrapping_add(self, addend: Simd<usize, LANES>) -> Self {
// Safety: converting pointers to usize and vice-versa is safe
// (even if using that pointer is not)
unsafe {
let x: Simd<usize, LANES> = mem::transmute_copy(&self);
mem::transmute_copy(&{ x + (addend * Simd::splat(mem::size_of::<T>())) })
Expand Down
2 changes: 2 additions & 0 deletions crates/core_simd/src/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ macro_rules! from_transmute {
impl core::convert::From<$from> for $to {
#[inline]
fn from(value: $from) -> $to {
// Safety: transmuting between vectors is safe, but the caller of this macro
// checks the invariants
unsafe { core::mem::transmute(value) }
}
}
Expand Down