Skip to content

Commit

Permalink
Add some safety comments
Browse files Browse the repository at this point in the history
  • Loading branch information
calebzulawski authored and workingjubilee committed Feb 10, 2022
1 parent 6143bde commit dddfffc
Show file tree
Hide file tree
Showing 11 changed files with 57 additions and 5 deletions.
12 changes: 12 additions & 0 deletions crates/core_simd/src/comparisons.rs
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
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
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
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
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
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
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
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
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
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
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

0 comments on commit dddfffc

Please sign in to comment.