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

Bound checking elided for [index] but not for [..index] #73396

Closed
tesuji opened this issue Jun 16, 2020 · 3 comments · Fixed by #75886
Closed

Bound checking elided for [index] but not for [..index] #73396

tesuji opened this issue Jun 16, 2020 · 3 comments · Fixed by #75886
Assignees
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tesuji
Copy link
Contributor

tesuji commented Jun 16, 2020

I tried this code: https://rust.godbolt.org/z/ihbmwB

pub fn foo(s: &[u8]) -> &[u8] {
    if let Some(idx) = s.iter().position(|b| *b == b'\\') {
        &s[..idx]
    } else {
        s
    }
}

I expected to see this happen: There is no bound check for s[..idx] in line 3.

Instead, this happened: There is bound check.

Note using s[idx] bound check is elided:

pub fn bar(s: &[u8]) -> u8 {
    if let Some(idx) = s.iter().position(|b| *b == b'\\') {
        s[idx]
    } else {
        42
    }
}

cc #30112

Meta

rustc --version --verbose: rustc 1.46.0-nightly (4fb54ed 2020-06-14)

rustc 1.46.0-nightly (4fb54ed48 2020-06-14)
binary: rustc
commit-hash: 4fb54ed484e2239a3e9eff3be17df00d2a162be3
commit-date: 2020-06-14
host: x86_64-unknown-linux-gnu
release: 1.46.0-nightly
LLVM version: 10.0
@leonardo-m
Copy link

Apparently here LLVM removes the bound tests:

pub fn foo2(s: &[u8]) -> &[u8] {
    let mut idx = 0;
    while idx < s.len() {
        if s[idx] == b'\\' {
            return &s[.. idx];
        }
        idx += 1;
    }
    s
}

@jonas-schievink jonas-schievink added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 16, 2020
@nikic
Copy link
Contributor

nikic commented Jun 16, 2020

  %_4.i.i.i = icmp ult i64 %.sroa.3.0.i.i.i, %s.1, !dbg !66
  tail call void @llvm.assume(i1 %_4.i.i.i) #2, !dbg !74, !noalias !75
  %_8.i.i.i = icmp ugt i64 %.sroa.3.0.i.i.i, %s.1, !dbg !80
  br i1 %_8.i.i.i, label %bb5.i.i.i, label %bb6, !dbg !87

This is https://bugs.llvm.org/show_bug.cgi?id=40149.

@tesuji tesuji changed the title Bound checking for [..index] but not for [index] Bound checking elided for [index] but not for [..index] Jun 16, 2020
@nikic
Copy link
Contributor

nikic commented Jun 28, 2020

https://reviews.llvm.org/D82717

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. C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants