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

Weird codegen / constant folding with iterators of length > 101 #112169

Open
SadiinsoSnowfall opened this issue Jun 1, 2023 · 8 comments
Open
Labels
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.

Comments

@SadiinsoSnowfall
Copy link

Hi,
When playing with some of my code on compiler explorer, I found a case where rust would not compute the result of some functions at compile time. I managed to reduce the code to the following:

pub fn test() -> i32 {
    let mut s = 0;

    for i in 0..102 {
        if i == 0 {
            s = i;
        }

        s += 1;
    }

    s
}

pub fn test2() -> i32 {
    let mut s = 0;

    for i in 0..101 {
        if i == 0 {
            s = i;
        }

        s += 1;
    }

    s
}

pub fn test3() -> i32 {
    let mut s = 0;

    for _ in 0..102 {
        s += 1;
    }

    s
}

compiler explorer link

Both test2 and test3 are completely optimized and are compiled down to a singled "mov constant + ret" combo. But the test function generates something quite convoluted: the compiler loads the result (102) and removes 6 from it until it reaches 0, at which point it returns the value of another register to which 6s have been added at each step.

I tried a few versions of rustc but couldn't find any which completely optimized away the first function. Using mir-opt-level didn't help.

@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 Jun 1, 2023
@nikic
Copy link
Contributor

nikic commented Jun 1, 2023

I expect that select peeling in LLVM 17 will fix this.

@erikdesjardins
Copy link
Contributor

Fixed in 1.73, presumably due to the LLVM 17 upgrade (#114048): https://godbolt.org/z/eWYs5YvKK

@rustbot label +E-needs-test

@rustbot rustbot added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Mar 5, 2024
@nikic
Copy link
Contributor

nikic commented Apr 17, 2024

(Keeping this open until a codegen test has been added for it -- these kinds of optimizations have an annoying habit of regressing without that.)

@nikic nikic reopened this Apr 17, 2024
@SadiinsoSnowfall
Copy link
Author

My bad, I just looked at the number of opened issues and thought I'd better close mine to reduce the "noise" since it seemed to be fixed.
Let me know if I can help in any way.

@nikic
Copy link
Contributor

nikic commented Apr 17, 2024

If you're interested, what we need it basically a test like this: https://github.com/rust-lang/rust/blob/master/tests/codegen/issues/issue-101082.rs That just takes the test function checks it compiles down to a single ret.

@SadiinsoSnowfall
Copy link
Author

Ok, I'm looking at the filecheck documentation, I will inform you if I manage to implement the codegen test ^^

bors added a commit to rust-lang-ci/rust that referenced this issue Apr 21, 2024
…lacrum

Add codegen test for 112169

Add codegen test for rust-lang#112169.
The test passes but it's my first time using FileCheck, don't hesitate to tell me if this can be improved ^^
@SadiinsoSnowfall
Copy link
Author

As @nikic said in the linked PR, this still isn't completely fixed, using a length of 801 and above still results in the loop not being optimized away (see https://godbolt.org/z/48594oGn5)

@SadiinsoSnowfall
Copy link
Author

fun note: adding -C target-cpu=+avx makes the compiler optimize the loop away.

@nikic nikic removed the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Jun 17, 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. I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants