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

Rust unable to optimize away assert vs. clang. #93036

Closed
jrmuizel opened this issue Jan 18, 2022 · 7 comments · Fixed by #125347
Closed

Rust unable to optimize away assert vs. clang. #93036

jrmuizel opened this issue Jan 18, 2022 · 7 comments · Fixed by #125347
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. 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

@jrmuizel
Copy link
Contributor

pub fn f(arr: &mut [u32]) {
    for i in 0..arr.len() {
        for j in 0..i {
            assert!(j < arr.len());
        }
    }
}

Rustc does not optimize away the the assert. Adding -C passes=constraint-elimination doesn't help.

However, clang does optimize away this assert when compiled with -O2 -mllvm -enable-constraint-elimination

void f(int *ptr, size_t len) {
    for (size_t i = 0; i < len; i++) {
            for (size_t j = 0; j < i; j++) {
                assert(j < len);
            }
    }
}

GCC is also able to remove the assert with GCC trunk.

@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 Jan 18, 2022
@dalcde
Copy link
Contributor

dalcde commented Jan 18, 2022

More generally, rust/llvm does not understand that inequality is transitive. The following code generates three inequality asserts in the assembly:

pub fn cmp(a: u32, b: u32, c: u32) {
    assert!(a < b);
    assert!(b < c);
    assert!(a < c);
}

Rust playground: https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=8cfdb982df6b7e781b398b9408b3cd10

@jrmuizel
Copy link
Contributor Author

More generally, rust/llvm does not understand that inequality is transitive. The following code generates three inequality asserts in the assembly:

Adding -C passes=constraint-elimination fixes that.

@jrmuizel
Copy link
Contributor Author

but not if the types are changed to i32 for which I filed: llvm/llvm-project#53273

@the8472
Copy link
Member

the8472 commented Jan 18, 2022

Assuming you need this for slicing then creating a temporary works.

pub fn f(arr: &mut [u32]) {
    for i in 0..arr.len() {
        let subslice = &arr[0..i];
        for j in 0..subslice.len() {
            assert!(j < subslice.len());
        }
    }
}

https://rust.godbolt.org/z/n6sa6Gf34

@thvdveld
Copy link
Contributor

Since llvm/llvm-project#53273 is now fixed, when is this going to land into nightly Rust (I have no idea when Rust updates LLVM)? And should it fix the origin issue?

@nikic
Copy link
Contributor

nikic commented Apr 25, 2022

This still requires ConstraintElimination to be enabled on the LLVM side. If it is, then this will land with the LLVM 15 upgrade in ~August.

@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
@andjo403
Copy link
Contributor

andjo403 commented Oct 7, 2023

this is fixed in latest stable rustc 1.73 see https://rust.godbolt.org/z/G1E35e375

@nikic nikic added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Oct 7, 2023
tesuji added a commit to tesuji/rustc that referenced this issue May 20, 2024
tesuji added a commit to tesuji/rustc that referenced this issue Jun 8, 2024
tesuji added a commit to tesuji/rustc that referenced this issue Jun 9, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 9, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 10, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 11, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 13, 2024
@bors bors closed this as completed in 7ac6c2f Jun 14, 2024
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. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. 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.

7 participants