Skip to content

Rust doesn't optimize bounds checks that Go 1.11 does #51709

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

Closed
KamilaBorowska opened this issue Jun 22, 2018 · 5 comments
Closed

Rust doesn't optimize bounds checks that Go 1.11 does #51709

KamilaBorowska opened this issue Jun 22, 2018 · 5 comments
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. C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such 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

@KamilaBorowska
Copy link
Contributor

KamilaBorowska commented Jun 22, 2018

After reading https://docs.google.com/presentation/d/1tpeJZFObkeick4CF-mx0L3CeCgvT15B96aJeRpxEPcE/preview#slide=id.g3bc6fcf60c_0_200, I noticed that there were three examples of Go optimizing bounds checks. None of those are optimized by Rust nightly in release mode as I see calls in assembly to bounds checking error function.

#![crate_type = "lib"]

/* now fixed
pub fn one(s: &[u8], suffix: &[u8]) -> bool {
    s.len() >= suffix.len() && s[s.len() - suffix.len()..] == *suffix
}

pub fn two(a: &[i32]) -> i32 {
    let mut x = 0;
    for i in (0..a.len()).rev() {
        x += a[i];
    }
    x
}
*/

pub fn three(i: usize, b: &mut [i32]) {
    if i >= b.len() {
        return;
    }
    for j in 0..i {
        b[j] += 1;
    }
}
@stokhos stokhos added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jun 22, 2018
@leonardo-m
Copy link

Rustc will need specific passes for bounds checks elision, optimization of integer operations with overflow, escape analysis, devirtualization, and perhaps more.

@KamilaBorowska
Copy link
Contributor Author

KamilaBorowska commented Dec 13, 2018

Example one is now optimized correctly, as of Rust 1.29.0.

@nikic nikic added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-slow Issue: Problems and improvements with respect to performance of generated code. labels Dec 13, 2018
@jonas-schievink jonas-schievink added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Mar 17, 2020
@KamilaBorowska
Copy link
Contributor Author

With an update to LLVM 12, example two is now optimized correctly.

@leonardo-m
Copy link

This doesn't have array bound tests, so there could be something fishy:

pub fn four(i: usize, b: &mut [i32]) {
    if i >= b.len() {
        return;
    }
    for x in &mut b[.. i].iter_mut() {
        *x += 1;
    }
}

@workingjubilee workingjubilee added the C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such label Oct 8, 2023
@KamilaBorowska
Copy link
Contributor Author

As of Rust 1.73.0, this issue was fixed.

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. C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such 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

No branches or pull requests

6 participants