Unsound Div, Rem

[calebzulawski]

The following should panic for integers:

• neg(MIN)
• div(MIN, -1)
• rem(MIN, -1)
• div(x, 0)
• rem(x, 0)

[calebzulawski]

This should probably be fixed after or with #49

[workingjubilee]

NB: these are all excepting neg(MIN), these are considered UB by LLVM and currently these scalar Rust ops panic (or are even prevented with a "this will panic at runtime" warning).

[calebzulawski]

looking again, neg in llvm is implemented with sub and appears to be allowed to wrap, so maybe that needs to be checked in the compiler only in debug mode?

[thomcc]

Is there any chance of having a wrapping_simd_foo version that we could use instead? I suspect in practice the branching here will be very bad for performance.

[calebzulawski]

For neg I think we want to extend the compiler intrinsics eventually. For div and rem the branching is unavoidable (but we can try to optimize it in the future)

[workingjubilee]

looking again, neg in llvm is implemented with sub and appears to be allowed to wrap, so maybe that needs to be checked in the compiler only in debug mode?

Playground

fn main() {
    println!("{}", -i32::MIN);
}

Compiling playground v0.0.1 (/playground)
Finished dev [unoptimized + debuginfo] target(s) in 1.69s
Running target/debug/playground
thread 'main' panicked at 'attempt to negate with overflow', src/main.rs:2:20
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace

However...

Running target/release/playground

-2147483648

So yeah! That one only panics in debug, actually.

[thomcc]

For neg I think we want to extend the compiler intrinsics eventually. For div and rem the branching is unavoidable (but we can try to optimize it in the future)

div/rem are heavyweight operations that this seems fine for, so long as the branch is correctly predicted against and sufficiently outlined the way normal int divs and such are.

[workingjubilee]

I think we have a separate issue on our hands which is re: whether or not add/sub/neg should overflow in Debug mode, and as such have removed that from this issue and into #56.

[workingjubilee]

Resolved by #55.