Skip to content

Commit

Permalink
Remove overflow panic from divrem
Browse files Browse the repository at this point in the history
  • Loading branch information
workingjubilee committed Feb 8, 2022
1 parent 4910274 commit 8df0494
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 25 deletions.
39 changes: 24 additions & 15 deletions crates/core_simd/src/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <SInt>::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) }
}
};
}
Expand Down Expand Up @@ -183,15 +194,13 @@ for_base_ops! {
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
}
}

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
}
}
Expand Down
21 changes: 11 additions & 10 deletions crates/core_simd/tests/ops_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,22 @@ macro_rules! impl_signed_tests {
)
}

}
fn div_min_may_overflow<const LANES: usize>() {
let a = Vector::<LANES>::splat(Scalar::MIN);
let b = Vector::<LANES>::splat(-1);
assert_eq!(a / b, a / (b * b));
}

test_helpers::test_lanes_panic! {
fn div_min_overflow_panics<const LANES: usize>() {
fn rem_min_may_overflow<const LANES: usize>() {
let a = Vector::<LANES>::splat(Scalar::MIN);
let b = Vector::<LANES>::splat(-1);
let _ = a / b;
assert_eq!(a % b, a % (b * b));
}

}

test_helpers::test_lanes_panic! {

fn div_by_all_zeros_panics<const LANES: usize>() {
let a = Vector::<LANES>::splat(42);
let b = Vector::<LANES>::splat(0);
Expand All @@ -232,12 +239,6 @@ macro_rules! impl_signed_tests {
let _ = a / b;
}

fn rem_min_overflow_panic<const LANES: usize>() {
let a = Vector::<LANES>::splat(Scalar::MIN);
let b = Vector::<LANES>::splat(-1);
let _ = a % b;
}

fn rem_zero_panic<const LANES: usize>() {
let a = Vector::<LANES>::splat(42);
let b = Vector::<LANES>::splat(0);
Expand Down

0 comments on commit 8df0494

Please sign in to comment.