Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bounds check not omitted #46863

Closed
jrmuizel opened this issue Dec 20, 2017 · 4 comments
Closed

Bounds check not omitted #46863

jrmuizel opened this issue Dec 20, 2017 · 4 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.

Comments

@jrmuizel
Copy link
Contributor

pub fn f(l: &Vec<u64>) -> u64 {
  let mut sum = 0;
  for i in 0..l.len() {
    sum += l[i]
  }
  sum
}

Compiles with -C opt-level=3 -C panic=abort to:

example::f:
        mov     rdx, qword ptr [rdi + 16]
        test    rdx, rdx
        je      .LBB0_1
        mov     rcx, qword ptr [rdi]
        xor     eax, eax
        xor     esi, esi
.LBB0_4:
        cmp     rdx, rsi
        jbe     .LBB0_5
        add     rax, qword ptr [rcx + 8*rsi]
        inc     rsi
        cmp     rsi, rdx
        jb      .LBB0_4
        jmp     .LBB0_2
.LBB0_1:
        xor     eax, eax
.LBB0_2:
        ret
.LBB0_5:
        push    rbp
        mov     rbp, rsp
        lea     rdi, [rip + panic_bounds_check_loc.1]
        call    core::panicking::panic_bounds_check@PLT
        ud2

It should be possible for the bounds check to be dropped.

@arielb1 arielb1 added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Dec 21, 2017
@arielb1
Copy link
Contributor

arielb1 commented Dec 21, 2017

This looks like a pass ordering issue - GVN is needed to simplify the double load, while IndVarSimplify is needed to eliminate the bounds check, while GVN occurs only after IndVarSimplify.

see e.g. http://lists.llvm.org/pipermail/llvm-dev/2013-October/067045.html

@arielb1
Copy link
Contributor

arielb1 commented Dec 21, 2017

Minimal example - I suppose the identical C code has the same problem.

Nothing but GVN can optimize loads across basic blocks, and nothing but indvars (I think) can fix up bounds checks.

Maybe there's another bounds check removal pass we can run later in the pipeline.

#![crate_type="rlib"]
pub fn f(l: &usize) -> u64 {
  let mut sum = 0;
  let len_1 = *l;
  let mut i = 0;
  while i < len_1 {
    let len_2 = *l;
    assert!(i < len_2);
    i += 1;
  }
  sum
}

@AndrewGaspar
Copy link
Contributor

Assuming this is nightly, I believe I also ran into this regression. https://users.rust-lang.org/t/surprising-auto-vectorization-optimization-behavior/14537

@alexcrichton
Copy link
Member

I believe this was fixed by #47828

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
Projects
None yet
Development

No branches or pull requests

4 participants