Skip to content
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

Document internal unsafety #182

Closed
wants to merge 2 commits into from
Closed
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 @@ -9,12 +9,16 @@ where
/// Test if each lane is equal to the corresponding lane in `other`.
#[inline]
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]
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 @@ -27,24 +31,32 @@ where
/// Test if each lane is less than the corresponding lane in `other`.
#[inline]
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]
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]
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]
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 @@ -130,6 +133,7 @@ where
/// All lanes must be either 0 or -1.
#[inline]
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 @@ -141,6 +145,7 @@ where
#[inline]
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 @@ -157,6 +162,7 @@ where
/// `lane` must be less than `LANES`.
#[inline]
pub unsafe fn test_unchecked(&self, lane: usize) -> bool {
// Safety: the caller must confirm this invariant
unsafe { self.0.test_unchecked(lane) }
}

Expand All @@ -167,6 +173,7 @@ where
#[inline]
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 @@ -176,6 +183,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 @@ -188,6 +196,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
18 changes: 14 additions & 4 deletions crates/core_simd/src/masks/bitmask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,14 @@ where

#[inline]
pub fn to_int(self) -> Simd<T, LANES> {
// TODO remove the transmute when rustc is more flexible
// Safety: IntBitMask is the integer equivalent to `self`, and matches the output vector
// lane count.
unsafe {
assert_eq!(
core::mem::size_of::<<LaneCount::<LANES> as SupportedLaneCount>::BitMask>(),
core::mem::size_of::<<LaneCount::<LANES> as SupportedLaneCount>::IntBitMask>(),
);
let mask: <LaneCount<LANES> as SupportedLaneCount>::IntBitMask =
core::mem::transmute_copy(&self);
intrinsics::simd_select_bitmask(mask, Simd::splat(T::TRUE), Simd::splat(T::FALSE))
Expand All @@ -110,11 +117,13 @@ where
#[inline]
pub unsafe fn from_int_unchecked(value: Simd<T, LANES>) -> Self {
// TODO remove the transmute when rustc is more flexible
assert_eq!(
core::mem::size_of::<<LaneCount::<LANES> as SupportedLaneCount>::BitMask>(),
core::mem::size_of::<<LaneCount::<LANES> as SupportedLaneCount>::IntBitMask>(),
);
// Safety: IntBitMask is the integer equivalent to `self`, and matches the output vector
// lane count.
unsafe {
assert_eq!(
core::mem::size_of::<<LaneCount::<LANES> as SupportedLaneCount>::BitMask>(),
core::mem::size_of::<<LaneCount::<LANES> as SupportedLaneCount>::IntBitMask>(),
);
let mask: <LaneCount<LANES> as SupportedLaneCount>::IntBitMask =
intrinsics::simd_bitmask(value);
Self(core::mem::transmute_copy(&mask), PhantomData)
Expand All @@ -140,6 +149,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 @@ -99,12 +99,14 @@ 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]
pub fn to_bitmask(self) -> [u8; LaneCount::<LANES>::BITMASK_LEN] {
// Safety: IntBitMask is the integer equivalent of the return type.
unsafe {
// TODO remove the transmute when rustc can use arrays of u8 as bitmasks
assert_eq!(
Expand Down Expand Up @@ -132,6 +134,7 @@ where
#[cfg(feature = "generic_const_exprs")]
#[inline]
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 Down Expand Up @@ -160,11 +163,13 @@ where

#[inline]
pub fn any(self) -> bool {
// Safety: use `self` as an integer vector
unsafe { intrinsics::simd_reduce_any(self.to_int()) }
}

#[inline]
pub fn all(self) -> bool {
// Safety: use `self` as an integer vector
unsafe { intrinsics::simd_reduce_all(self.to_int()) }
}
}
Expand All @@ -187,6 +192,7 @@ where
type Output = Self;
#[inline]
fn bitand(self, rhs: Self) -> Self {
// Safety: `self` is an integer vector
unsafe { Self(intrinsics::simd_and(self.0, rhs.0)) }
}
}
Expand All @@ -199,6 +205,7 @@ where
type Output = Self;
#[inline]
fn bitor(self, rhs: Self) -> Self {
// Safety: `self` is an integer vector
unsafe { Self(intrinsics::simd_or(self.0, rhs.0)) }
}
}
Expand All @@ -211,6 +218,7 @@ where
type Output = Self;
#[inline]
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
19 changes: 19 additions & 0 deletions crates/core_simd/src/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ macro_rules! impl_op {
{
type Output = Self;
fn neg(self) -> Self::Output {
// Safety: `self` is a vector
unsafe { intrinsics::simd_neg(self) }
}
}
Expand All @@ -226,6 +227,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 @@ -268,6 +270,7 @@ macro_rules! impl_op {
{
#[inline]
fn $assign_trait_fn(&mut self, rhs: Self) {
// Safety: `self` is a vector
unsafe {
*self = intrinsics::$intrinsic(*self, rhs);
}
Expand Down Expand Up @@ -339,6 +342,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 All @@ -362,6 +367,8 @@ macro_rules! impl_unsigned_int_ops {
panic!("attempt to divide with overflow");
}
let rhs = Self::splat(rhs);
// 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 @@ -429,6 +436,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 @@ -452,6 +461,8 @@ macro_rules! impl_unsigned_int_ops {
panic!("attempt to calculate the remainder with overflow");
}
let rhs = Self::splat(rhs);
// Safety: `self` is a vector, and the lanes have been checked for
// division by zero and overflow
unsafe { intrinsics::simd_rem(self, rhs) }
}
}
Expand Down Expand Up @@ -513,6 +524,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 @@ -531,6 +544,8 @@ macro_rules! impl_unsigned_int_ops {
panic!("attempt to shift left with overflow");
}
let rhs = Self::splat(rhs);
// Safety: `self` is a vector, and the shift argument has been checked for
// overflow
unsafe { intrinsics::simd_shl(self, rhs) }
}
}
Expand Down Expand Up @@ -578,6 +593,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 All @@ -596,6 +613,8 @@ macro_rules! impl_unsigned_int_ops {
panic!("attempt to shift with overflow");
}
let rhs = Self::splat(rhs);
// Safety: `self` is a vector, and the shift argument has been checked for
// overflow
unsafe { intrinsics::simd_shr(self, rhs) }
}
}
Expand Down
Loading