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 Dec 19, 2021
1 parent 533f0fc commit 1dea939
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 6 deletions.
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)) }
}
}
2 changes: 1 addition & 1 deletion crates/core_simd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#![cfg_attr(feature = "generic_const_exprs", feature(generic_const_exprs))]
#![cfg_attr(feature = "generic_const_exprs", allow(incomplete_features))]
#![warn(missing_docs)]
#![deny(unsafe_op_in_unsafe_fn)]
#![deny(unsafe_op_in_unsafe_fn, clippy::undocumented_unsafe_blocks)]
#![unstable(feature = "portable_simd", issue = "86656")]
//! Portable SIMD module.
Expand Down
9 changes: 9 additions & 0 deletions crates/core_simd/src/masks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,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 @@ -131,6 +134,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 @@ -143,6 +147,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 @@ -161,6 +166,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 @@ -172,6 +178,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 @@ -181,6 +188,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 @@ -193,6 +201,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
8 changes: 8 additions & 0 deletions crates/core_simd/src/masks/full_masks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,15 @@ 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)) }
}

#[cfg(feature = "generic_const_exprs")]
#[inline]
#[must_use = "method returns a new array and does not mutate the original value"]
pub fn to_bitmask(self) -> [u8; LaneCount::<LANES>::BITMASK_LEN] {
// Safety: IntBitMask is the integer equivalent of the return type.
unsafe {
let mut bitmask: [u8; LaneCount::<LANES>::BITMASK_LEN] =
intrinsics::simd_bitmask(self.0);
Expand All @@ -134,6 +136,7 @@ where
#[inline]
#[must_use = "method returns a new mask and does not mutate the original value"]
pub fn from_bitmask(mut bitmask: [u8; LaneCount::<LANES>::BITMASK_LEN]) -> Self {
// Safety: IntBitMask is the integer equivalent of the input type.
unsafe {
// There is a bug where LLVM appears to implement this operation with the wrong
// bit order.
Expand All @@ -155,12 +158,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 +189,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 +203,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 +217,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
9 changes: 9 additions & 0 deletions crates/core_simd/src/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ macro_rules! impl_op {

#[inline]
fn $trait_fn(self, rhs: Self) -> Self::Output {
// Safety: `self` is a vector
unsafe {
intrinsics::$intrinsic(self, rhs)
}
Expand Down Expand Up @@ -169,6 +170,8 @@ macro_rules! impl_unsigned_int_ops {
.any(|(x,y)| *x == <$scalar>::MIN && *y == -1 as _) {
panic!("attempt to divide with overflow");
}
// Safety: `self` is a vector, and the lanes have been checked for
// division by zero and overflow
unsafe { intrinsics::simd_div(self, rhs) }
}
}
Expand Down Expand Up @@ -198,6 +201,8 @@ macro_rules! impl_unsigned_int_ops {
.any(|(x,y)| *x == <$scalar>::MIN && *y == -1 as _) {
panic!("attempt to calculate the remainder with overflow");
}
// Safety: `self` is a vector, and the lanes have been checked for
// division by zero and overflow
unsafe { intrinsics::simd_rem(self, rhs) }
}
}
Expand All @@ -221,6 +226,8 @@ macro_rules! impl_unsigned_int_ops {
{
panic!("attempt to shift left with overflow");
}
// Safety: `self` is a vector, and the shift argument has been checked for
// overflow
unsafe { intrinsics::simd_shl(self, rhs) }
}
}
Expand All @@ -243,6 +250,8 @@ macro_rules! impl_unsigned_int_ops {
{
panic!("attempt to shift with overflow");
}
// Safety: `self` is a vector, and the shift argument has been checked for
// overflow
unsafe { intrinsics::simd_shr(self, rhs) }
}
}
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
6 changes: 6 additions & 0 deletions crates/core_simd/src/round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,31 @@ macro_rules! implement {
#[must_use = "method returns a new vector and does not mutate the original value"]
#[inline]
pub fn ceil(self) -> Self {
// Safety: `self` is a vector
unsafe { intrinsics::simd_ceil(self) }
}

/// Returns the largest integer value less than or equal to each lane.
#[must_use = "method returns a new vector and does not mutate the original value"]
#[inline]
pub fn floor(self) -> Self {
// Safety: `self` is a vector
unsafe { intrinsics::simd_floor(self) }
}

/// Rounds to the nearest integer value. Ties round toward zero.
#[must_use = "method returns a new vector and does not mutate the original value"]
#[inline]
pub fn round(self) -> Self {
// Safety: `self` is a vector
unsafe { intrinsics::simd_round(self) }
}

/// Returns the floating point's integer value, with its fractional part removed.
#[must_use = "method returns a new vector and does not mutate the original value"]
#[inline]
pub fn trunc(self) -> Self {
// Safety: `self` is a vector
unsafe { intrinsics::simd_trunc(self) }
}

Expand All @@ -61,13 +65,15 @@ macro_rules! implement {
/// * Be representable in the return type, after truncating off its fractional part
#[inline]
pub unsafe fn to_int_unchecked(self) -> Simd<$int_type, LANES> {
// Safety: the caller must check the invariants
unsafe { intrinsics::simd_cast(self) }
}

/// Creates a floating-point vector from an integer vector. Rounds values that are
/// not exactly representable.
#[inline]
pub fn round_from_int(value: Simd<$int_type, LANES>) -> Self {
// Safety: conversion from integer to float vectors is safe
unsafe { intrinsics::simd_cast(value) }
}
}
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
Loading

0 comments on commit 1dea939

Please sign in to comment.