diff --git a/crates/core_simd/src/intrinsics.rs b/crates/core_simd/src/intrinsics.rs index 233657202f7..b5d0df7548f 100644 --- a/crates/core_simd/src/intrinsics.rs +++ b/crates/core_simd/src/intrinsics.rs @@ -17,9 +17,15 @@ pub(crate) fn simd_mul(x: T, y: T) -> T; /// udiv/sdiv/fdiv + /// ints and uints: {s,u}div incur UB if division by zero occurs. + /// ints: sdiv is UB for int::MIN / -1. + /// floats: fdiv is never UB, but may create NaNs or infinities. pub(crate) fn simd_div(x: T, y: T) -> T; /// urem/srem/frem + /// ints and uints: {s,u}rem incur UB if division by zero occurs. + /// ints: srem is UB for int::MIN / -1. + /// floats: frem is equivalent to libm::fmod in the "default" floating point environment, sans errno. pub(crate) fn simd_rem(x: T, y: T) -> T; /// shl @@ -45,6 +51,9 @@ pub(crate) fn simd_as(x: T) -> U; /// neg/fneg + /// ints: ultimately becomes a call to cg_ssa's BuilderMethods::neg. cg_llvm equates this to `simd_sub(Simd::splat(0), x)`. + /// floats: LLVM's fneg, which changes the floating point sign bit. Some arches have instructions for it. + /// Rust panics for Neg::neg(int::MIN) due to overflow, but it is not UB in LLVM without `nsw`. pub(crate) fn simd_neg(x: T) -> T; /// fabs diff --git a/crates/core_simd/src/ops.rs b/crates/core_simd/src/ops.rs index b65038933bf..1b35b3e717a 100644 --- a/crates/core_simd/src/ops.rs +++ b/crates/core_simd/src/ops.rs @@ -57,29 +57,40 @@ macro_rules! wrap_bitshift { }; } -// Division by zero is poison, according to LLVM. -// So is dividing the MIN value of a signed integer by -1, -// since that would return MAX + 1. -// FIXME: Rust allows ::MIN / -1, -// so we should probably figure out how to make that safe. +/// SAFETY: This macro must only be used to impl Div or Rem and given the matching intrinsic. +/// It guards against LLVM's UB conditions for integer div or rem using masks and selects, +/// thus guaranteeing a Rust value returns instead. +/// +/// | | LLVM | Rust +/// | :--------------: | :--- | :---------- +/// | N {/,%} 0 | UB | panic!() +/// | <$int>::MIN / -1 | UB | <$int>::MIN +/// | <$int>::MIN % -1 | UB | 0 +/// macro_rules! int_divrem_guard { ( $lhs:ident, $rhs:ident, { const PANIC_ZERO: &'static str = $zero:literal; - const PANIC_OVERFLOW: &'static str = $overflow:literal; $simd_call:ident }, $int:ident ) => { if $rhs.lanes_eq(Simd::splat(0)).any() { panic!($zero); - } else if <$int>::MIN != 0 - && ($lhs.lanes_eq(Simd::splat(<$int>::MIN)) - // type inference can break here, so cut an SInt to size - & $rhs.lanes_eq(Simd::splat(-1i64 as _))).any() - { - panic!($overflow); } else { - unsafe { $crate::simd::intrinsics::$simd_call($lhs, $rhs) } + // Prevent otherwise-UB overflow on the MIN / -1 case. + let rhs = if <$int>::MIN != 0 { + // This should, at worst, optimize to a few branchless logical ops + // Ideally, this entire conditional should evaporate + // Fire LLVM and implement those manually if it doesn't get the hint + ($lhs.lanes_eq(Simd::splat(<$int>::MIN)) + // type inference can break here, so cut an SInt to size + & $rhs.lanes_eq(Simd::splat(-1i64 as _))) + .select(Simd::splat(1), $rhs) + } else { + // Nice base case to make it easy to const-fold away the other branch. + $rhs + }; + unsafe { $crate::simd::intrinsics::$simd_call($lhs, rhs) } } }; } @@ -183,7 +194,6 @@ impl BitXor::bitxor { impl Div::div { int_divrem_guard { const PANIC_ZERO: &'static str = "attempt to divide by zero"; - const PANIC_OVERFLOW: &'static str = "attempt to divide with overflow"; simd_div } } @@ -191,7 +201,6 @@ impl Div::div { impl Rem::rem { int_divrem_guard { const PANIC_ZERO: &'static str = "attempt to calculate the remainder with a divisor of zero"; - const PANIC_OVERFLOW: &'static str = "attempt to calculate the remainder with overflow"; simd_rem } } diff --git a/crates/core_simd/src/vector.rs b/crates/core_simd/src/vector.rs index b7ef7a56c73..5bd8ed69535 100644 --- a/crates/core_simd/src/vector.rs +++ b/crates/core_simd/src/vector.rs @@ -12,7 +12,38 @@ use crate::simd::intrinsics; use crate::simd::{LaneCount, Mask, MaskElement, SupportedLaneCount}; -/// A SIMD vector of `LANES` elements of type `T`. +/// A SIMD vector of `LANES` elements of type `T`. `Simd` has the same shape as [`[T; N]`](array), but operates like `T`. +/// +/// Two vectors of the same type and length will, by convention, support the operators (+, *, etc.) that `T` does. +/// These take the lanes at each index on the left-hand side and right-hand side, perform the operation, +/// and return the result in the same lane in a vector of equal size. For a given operator, this is equivalent to zipping +/// the two arrays together and mapping the operator over each lane. +/// +/// ```rust +/// # #![feature(array_zip, portable_simd)] +/// # use core::simd::{Simd}; +/// let a0: [i32; 4] = [-2, 0, 2, 4]; +/// let a1 = [10, 9, 8, 7]; +/// let zm_add = a0.zip(a1).map(|(lhs, rhs)| lhs + rhs); +/// let zm_mul = a0.zip(a1).map(|(lhs, rhs)| lhs * rhs); +/// +/// // `Simd` implements `From<[T; N]> +/// let (v0, v1) = (Simd::from(a0), Simd::from(a1)); +/// // Which means arrays implement `Into>`. +/// assert_eq!(v0 + v1, zm_add.into()); +/// assert_eq!(v0 * v1, zm_mul.into()); +/// ``` +/// +/// `Simd` with integers has the quirk that these operations are also inherently wrapping, as if `T` was [`Wrapping`]. +/// Thus, `Simd` does not implement `wrapping_add`, because that is the default behavior. +/// This means there is no warning on overflows, even in "debug" builds. +/// For most applications where `Simd` is appropriate, it is "not a bug" to wrap, +/// and even "debug builds" are unlikely to tolerate the loss of performance. +/// You may want to consider using explicitly checked arithmetic if such is required. +/// Division by zero still causes a panic, so you may want to consider using floating point numbers if that is unacceptable. +/// +/// [`Wrapping`]: core::num::Wrapping +/// #[repr(simd)] pub struct Simd([T; LANES]) where diff --git a/crates/core_simd/tests/ops_macros.rs b/crates/core_simd/tests/ops_macros.rs index 4fb9de198ee..50f7a4ca170 100644 --- a/crates/core_simd/tests/ops_macros.rs +++ b/crates/core_simd/tests/ops_macros.rs @@ -210,15 +210,21 @@ fn signum() { ) } + fn div_min_may_overflow() { + let a = Vector::::splat(Scalar::MIN); + let b = Vector::::splat(-1); + assert_eq!(a / b, a); + } + + fn rem_min_may_overflow() { + let a = Vector::::splat(Scalar::MIN); + let b = Vector::::splat(-1); + assert_eq!(a % b, Vector::::splat(0)); + } + } test_helpers::test_lanes_panic! { - fn div_min_overflow_panics() { - let a = Vector::::splat(Scalar::MIN); - let b = Vector::::splat(-1); - let _ = a / b; - } - fn div_by_all_zeros_panics() { let a = Vector::::splat(42); let b = Vector::::splat(0); @@ -232,12 +238,6 @@ fn div_by_one_zero_panics() { let _ = a / b; } - fn rem_min_overflow_panic() { - let a = Vector::::splat(Scalar::MIN); - let b = Vector::::splat(-1); - let _ = a % b; - } - fn rem_zero_panic() { let a = Vector::::splat(42); let b = Vector::::splat(0);