From dddfffcfb3e49c752d6a28039735ada2552d0307 Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Fri, 5 Nov 2021 01:42:29 +0000 Subject: [PATCH] Add some safety comments --- crates/core_simd/src/comparisons.rs | 12 ++++++++++++ crates/core_simd/src/masks.rs | 9 +++++++++ crates/core_simd/src/masks/bitmask.rs | 1 + crates/core_simd/src/masks/full_masks.rs | 6 ++++++ crates/core_simd/src/math.rs | 4 ++++ crates/core_simd/src/reduction.rs | 8 ++++++++ crates/core_simd/src/swizzle.rs | 2 ++ crates/core_simd/src/to_bytes.rs | 2 ++ crates/core_simd/src/vector.rs | 12 +++++++----- crates/core_simd/src/vector/ptr.rs | 4 ++++ crates/core_simd/src/vendor.rs | 2 ++ 11 files changed, 57 insertions(+), 5 deletions(-) diff --git a/crates/core_simd/src/comparisons.rs b/crates/core_simd/src/comparisons.rs index edef5af3687a..d024cf4ddbe3 100644 --- a/crates/core_simd/src/comparisons.rs +++ b/crates/core_simd/src/comparisons.rs @@ -10,6 +10,8 @@ where #[inline] #[must_use = "method returns a new mask and does not mutate the original value"] pub fn lanes_eq(self, other: Self) -> Mask { + // 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)) } } @@ -17,6 +19,8 @@ where #[inline] #[must_use = "method returns a new mask and does not mutate the original value"] pub fn lanes_ne(self, other: Self) -> Mask { + // 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)) } } } @@ -30,6 +34,8 @@ where #[inline] #[must_use = "method returns a new mask and does not mutate the original value"] pub fn lanes_lt(self, other: Self) -> Mask { + // 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)) } } @@ -37,6 +43,8 @@ where #[inline] #[must_use = "method returns a new mask and does not mutate the original value"] pub fn lanes_gt(self, other: Self) -> Mask { + // 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)) } } @@ -44,6 +52,8 @@ where #[inline] #[must_use = "method returns a new mask and does not mutate the original value"] pub fn lanes_le(self, other: Self) -> Mask { + // 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)) } } @@ -51,6 +61,8 @@ where #[inline] #[must_use = "method returns a new mask and does not mutate the original value"] pub fn lanes_ge(self, other: Self) -> Mask { + // 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)) } } } diff --git a/crates/core_simd/src/masks.rs b/crates/core_simd/src/masks.rs index ae1fef53da88..b1f98d9eb4e6 100644 --- a/crates/core_simd/src/masks.rs +++ b/crates/core_simd/src/masks.rs @@ -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 { @@ -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) -> Self { + // Safety: the caller must confirm this invariant unsafe { Self(mask_impl::Mask::from_int_unchecked(value)) } } @@ -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) -> 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) } } @@ -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) } } @@ -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) } } @@ -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); } @@ -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); } diff --git a/crates/core_simd/src/masks/bitmask.rs b/crates/core_simd/src/masks/bitmask.rs index b4217dc87ba9..b7f8b2c236ef 100644 --- a/crates/core_simd/src/masks/bitmask.rs +++ b/crates/core_simd/src/masks/bitmask.rs @@ -137,6 +137,7 @@ where where U: MaskElement, { + // Safety: bitmask layout does not depend on the element width unsafe { core::mem::transmute_copy(&self) } } diff --git a/crates/core_simd/src/masks/full_masks.rs b/crates/core_simd/src/masks/full_masks.rs index e5bb784bb910..02b5593c8f46 100644 --- a/crates/core_simd/src/masks/full_masks.rs +++ b/crates/core_simd/src/masks/full_masks.rs @@ -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)) } } @@ -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()) } } } @@ -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)) } } } @@ -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)) } } } @@ -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)) } } } diff --git a/crates/core_simd/src/math.rs b/crates/core_simd/src/math.rs index 7435b6df9186..0b4e40983af5 100644 --- a/crates/core_simd/src/math.rs +++ b/crates/core_simd/src/math.rs @@ -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) } } @@ -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) } } })+ @@ -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) } } @@ -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) } } diff --git a/crates/core_simd/src/reduction.rs b/crates/core_simd/src/reduction.rs index e79a185816bf..e1cd743e4424 100644 --- a/crates/core_simd/src/reduction.rs +++ b/crates/core_simd/src/reduction.rs @@ -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) } } } @@ -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.) } } } @@ -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.) } } } @@ -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) } } @@ -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) } } } diff --git a/crates/core_simd/src/swizzle.rs b/crates/core_simd/src/swizzle.rs index bdc489774a54..08b2add11667 100644 --- a/crates/core_simd/src/swizzle.rs +++ b/crates/core_simd/src/swizzle.rs @@ -95,6 +95,7 @@ pub trait Swizzle { LaneCount: SupportedLaneCount, LaneCount: SupportedLaneCount, { + // Safety: `vector` is a vector, and `INDEX_IMPL` is a const array of u32. unsafe { intrinsics::simd_shuffle(vector, vector, Self::INDEX_IMPL) } } } @@ -119,6 +120,7 @@ pub trait Swizzle2 { LaneCount: SupportedLaneCount, LaneCount: 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) } } } diff --git a/crates/core_simd/src/to_bytes.rs b/crates/core_simd/src/to_bytes.rs index 8d9b3e8ff85e..b36b1a347b22 100644 --- a/crates/core_simd/src/to_bytes.rs +++ b/crates/core_simd/src/to_bytes.rs @@ -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 { + // 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) -> Self { + // Safety: transmuting between vectors is safe unsafe { core::mem::transmute_copy(&bytes) } } } diff --git a/crates/core_simd/src/vector.rs b/crates/core_simd/src/vector.rs index 5bd8ed69535e..e452fa8bfc82 100644 --- a/crates/core_simd/src/vector.rs +++ b/crates/core_simd/src/vector.rs @@ -206,7 +206,7 @@ where or: Self, ) -> Self { let enable: Mask = 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) } } @@ -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()) } } @@ -299,7 +299,7 @@ where idxs: Simd, ) { let enable: Mask = 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) } } @@ -338,7 +338,7 @@ where enable: Mask, idxs: Simd, ) { - // 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 @@ -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, diff --git a/crates/core_simd/src/vector/ptr.rs b/crates/core_simd/src/vector/ptr.rs index c668d9a6eaee..417d255c28d6 100644 --- a/crates/core_simd/src/vector/ptr.rs +++ b/crates/core_simd/src/vector/ptr.rs @@ -21,6 +21,8 @@ where #[inline] #[must_use] pub fn wrapping_add(self, addend: Simd) -> Self { + // Safety: converting pointers to usize and vice-versa is safe + // (even if using that pointer is not) unsafe { let x: Simd = mem::transmute_copy(&self); mem::transmute_copy(&{ x + (addend * Simd::splat(mem::size_of::())) }) @@ -47,6 +49,8 @@ where #[inline] #[must_use] pub fn wrapping_add(self, addend: Simd) -> Self { + // Safety: converting pointers to usize and vice-versa is safe + // (even if using that pointer is not) unsafe { let x: Simd = mem::transmute_copy(&self); mem::transmute_copy(&{ x + (addend * Simd::splat(mem::size_of::())) }) diff --git a/crates/core_simd/src/vendor.rs b/crates/core_simd/src/vendor.rs index e8ce7176b4f2..9fb70218c954 100644 --- a/crates/core_simd/src/vendor.rs +++ b/crates/core_simd/src/vendor.rs @@ -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) } } }