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

Inefficient compiler optimization between stable versions on nightly #123262

Open
ArhanChaudhary opened this issue Mar 31, 2024 · 9 comments
Open
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ArhanChaudhary
Copy link

I have been recently been working on a logic simulator that requires building on the nightly channel. With the release of Rust 1.77.1, I subsequently updated my version from nightly-2024-02-08 to nightly-2024-03-28 and instantly noticed a drastic performance impact. Upon further investigation, starting from nightly-2024-02-12 and onwards, the compiler is no longer able to optimize my multiplexor function.

The godbolt link is unintentionally misleading with its versions. Strangely, this regression does not happen on the equivalent stable channel versions (1.76.0 and 1.77.1), and the reason why this happens only on nightly is beyond my current knowledge.

rustc --version --verbose: (The first bad version, and I think the dates are shifted because of time zones)

rustc 1.78.0-nightly (1a648b397 2024-02-11)
binary: rustc
commit-hash: 1a648b397dedc98ada3dd3360f6d661ec2436c56
commit-date: 2024-02-11
host: aarch64-apple-darwin
release: 1.78.0-nightly
LLVM version: 17.0.6

I'm not sure which regression label to mark this issue with, since it's occurred between stable versions but on the nightly channel, so I'll omit one for now.

@rustbot modify labels: -regression-untriaged

@ArhanChaudhary ArhanChaudhary added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Mar 31, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. and removed regression-untriaged Untriaged performance or correctness regression. labels Mar 31, 2024
@bjorn3 bjorn3 added the I-slow Issue: Problems and improvements with respect to performance of generated code. label Mar 31, 2024
@workingjubilee
Copy link
Member

workingjubilee commented Mar 31, 2024

@ArhanChaudhary

The godbolt link is unintentionally misleading with its versions. Strangely, this regression does not happen on the equivalent stable channel versions (1.76.0 and 1.77.1), and the reason why this happens only on nightly is beyond my current knowledge.

The stable version may be marked as having come out on 2024-03-27 but it dates to the nightly of around ~13 weeks before that, that's probably why. (i.e. there's not actually a corresponding stable release for nightly 2024-02-12 yet).

@Noratrieb Noratrieb added E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 31, 2024
@Noratrieb
Copy link
Member

Thank you for the report! It would be useful to bisect the regression using cargo-bisect-rustc to make it easier to figure out what happened and ping the relevant people. You already have the nightly, but getting the precise PR is even better.

@ArhanChaudhary
Copy link
Author

Thank you for the report! It would be useful to bisect the regression using cargo-bisect-rustc to make it easier to figure out what happened and ping the relevant people. You already have the nightly, but getting the precise PR is even better.

Thanks for the response. I'm short on time at this exact moment, but I'll identify and bisect the regression commit soon.

@ArhanChaudhary
Copy link
Author

ArhanChaudhary commented Apr 1, 2024

searched nightlies: from nightly-2024-02-08 to nightly-2024-02-13
regressed nightly: nightly-2024-02-12
searched commit range: 6cc4843...1a648b3
regressed commit: 42752cb

bisected with cargo-bisect-rustc v0.6.8

Host triple: aarch64-apple-darwin
Reproduce with:

cargo bisect-rustc -v --start=2024-02-08 --end=2024-02-13 --script ./test.sh 

test.sh (using cargo-show-asm)

#!/bin/sh

set -ex

export OUTPUT=`cargo asm --simplify`
test ${#OUTPUT} -lt 500

I can provide more information about my build configuration if necessary.

@DianQK
Copy link
Member

DianQK commented Apr 1, 2024

It seems that jump-threading is making the IR more complex. The optimization information has not been lost: https://alive2.llvm.org/ce/z/cPHXB2.

@rustbot label A-mir-opt A-LLVM

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-mir-opt Area: MIR optimizations labels Apr 1, 2024
@DianQK
Copy link
Member

DianQK commented Apr 1, 2024

Disable jump-threading: https://godbolt.org/z/MfM9Mnb6K.
Disable jump-threading (-Cno-prepopulate-passes): https://godbolt.org/z/Wcj7j7b5s (I can see the IR has become more complex. For jump-threading, this might not be a problem.)
Alive2: https://alive2.llvm.org/ce/z/a_9jWL.

@apiraino
Copy link
Contributor

apiraino commented Apr 2, 2024

paging @cjgillot @tmiasko for #117206

@ArhanChaudhary thanks for bisecting your issue report. Can you quantify a bit the performance impact reported? thanks

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Apr 2, 2024
@ArhanChaudhary
Copy link
Author

paging @cjgillot @tmiasko for #117206

@ArhanChaudhary thanks for bisecting your issue report. Can you quantify a bit the performance impact reported? thanks

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

For my logic simulator specifically, the simulated clock speed drops from around 21.5 MHz to 1.75 MHz following this regression on a wasm32-unknown-unknown target, leaving me unable to use later nightly versions with a performance impact of around 90%. And although not a big deal for my use case, the binary size also increases from 552kb to 659kb. These happen primarily because the multiplexor function is an important part of my logic simulator and I have aggressive performance inlining rules, so this single inefficiency starts to add up.

@ArhanChaudhary
Copy link
Author

@rustbot label -E-needs-bisection

@rustbot rustbot removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Apr 5, 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. A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code. P-medium Medium priority 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

7 participants