rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.84k stars 12.51k forks source link

Excessive overflow checks that can be proven unnecessary #114386

Open ProgramCrafter opened 1 year ago

ProgramCrafter commented 1 year ago

I tried this code:

#[inline(never)]
fn overflow_check(triple_lim: usize) -> bool {
    let lim: usize = triple_lim / 3;
    lim * 3 >= lim
}

fn main() {
    let n = std::hint::black_box(30);
    println!("{}", overflow_check(n));
}

The check lim * 3 >= lim is trivially true, since lim is obtained by dividing usize by 3 so there can be no overflow. I expected to see it optimized out in ASM, like here (obtained via std::hint::black_box-ing true):

playground::overflow_check:
    movb    $1, -1(%rsp)
    leaq    -1(%rsp), %rax
    #APP
    #NO_APP
    movzbl  -1(%rsp), %eax
    retq

Instead, this happened:

playground::overflow_check:
    movq    %rdi, %rax
    movabsq $-6148914691236517205, %rcx
    mulq    %rcx
    shrq    %rdx
    leaq    (%rdx,%rdx,2), %rax
    cmpq    %rdx, %rax
    setae   %al
    retq

Meta

https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=17cc61a317c18d7ea86d7d98b6a43510

triple_lim >= triple_lim / 3 in a similar test is actually optimized away.

Source

Optimized down from https://rust.godbolt.org/z/jsh6zhPYa (utility function for converting BGR to RGBA arrays).

AaronKutch commented 1 year ago

It's worrying about wrap around overflow inlim * 3 >= lim by itself in case lim is something like usize::MAX, I'm guessing the problem is that the numerical bounds of what leads to lim could be are not being tracked with respect to division by a constant.

erikdesjardins commented 1 year ago

nuw already gets inferred on the mul: https://rust.godbolt.org/z/99r9no3MP

So LLVM just needs this transform: https://alive2.llvm.org/ce/z/4a6RLR

declare void @llvm.assume(i1)

define i1 @src(i16 %x, i16 %c) {
  %c_nonzero = icmp ne i16 %c, 0
  call void @llvm.assume(i1 %c_nonzero)

  %x3 = mul nuw i16 %x, %c
  %cmp = icmp uge i16 %x3, %x
  ret i1 %cmp
}

define i1 @tgt(i16 %x, i16 %c) {
  ret i1 true
}