-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Regression in compile times after LLVM was upgraded #31435
Comments
Nominating for triage as well cc @rust-lang/compiler |
before? Most of the time-intensive passes are universally slower. There’s more passes being run in the new LLVM, but all the new ones are pretty fast (timings around 0.000s). |
@nagisa ah yeah oops that was a typo (corrected above) Also I just realized that nightly compilers have LLVM assertions enabled whereas my local compilers did not, that would explain the discrepancy between those timings. |
Looks like something to ping the LLVM guys about, at least to get some insight. Loop-invariant code motion seems like it's taken a significant hit, with the top 3 LICM passes all taking around twice the time now. |
This is at least partially caused by the new AA handling which ends up being quadratic in the number of used passes. I've filed an LLVM bug at https://llvm.org/bugs/show_bug.cgi?id=26564 |
triage: P-medium Not much to do here besides try to keep an eye on how LLVM progresses in fixes this issue, right? (Rolling back LLVM doesn't really seem like a viable plan.) |
At the very least we could apply a temporary local patch if it's a quick hack vs. a longer proper fix. |
We could perhaps disable that pass? I'm not sure if it is a pass that's causing the problem. |
We could remove TBAA which does nothing for us. That saves a few percent.
|
We've got a pretty long period of time before this reaches stable, so I'd be willing to give the LLVM bug report some more time to get some analysis on behalf of the LLVM folks. If this gets close to stable, however, backporting a change to just disable TBAA seems like it should definitely be worth it! In other words, ideally, there'd be an upstream patch to fix this regressions, we'd backport it to our branch, but failing that we can just locally disable the regression temporarily. |
This is perhaps a bit off-topic, but why are we running passes that don't do anything for us? Just to keep us close to the C/C++ pass pipeline? |
Yes, as far as I'm aware that's it. The reason for that is that the default pass setup works reasonablly well so far, and having your own pass setup means more maintenance to keep compatible with different LLVM versions and that you don't automatically profit from improvements made to the default pass setup (obviously). That said, there have been various people who wanted to look into getting a more rust specific pass setup, it's just a ton of work and nothing has been completed (started?) yet. Also, for this specific analysis, the extra time spent on TBAA was negligible, shouldn't have been much more than just checking for TBAA metadata, seeing that there is none and handing things over to the next AA analysis. |
AA fix landed upstream in LLVM r262490. |
Update LLVM to include a backport to restore AA performance cc rust-lang#31435 r? @alexcrichton
I'm not 100% sure what the status is here -- but it seems like we merged the fix from upstream, so going to close this as fixed. Feel free to re-open. |
LLVM was recently upgraded in #30448 which appears to have caused a regression
in compile times. First up, let's talk numbers. The compiler revisions in
question are:
The
nightly-2016-01-30
toolchain conveniently corresponds exactly to303892e. The benchmarks here will all be
compiling libsyntax at these two revision in optimized mode. Note that
libsyntax did not change at all between these two revisions. For local
compilations I configured with:
and the C++ compiler version is:
Nightly compiler (
nightly-2016-01-30
)Summary: 2:11.18s compile time, full output of time-passes
Locally compiled after LLVM upgrade (303892e)
Summary: 1:37.48s compile time, full output of time-passes
Locally compiled before LLVM upgrade (074f49a)
Summary: 1:22.69s compile time, full output of time-passes
Summary
So there are some fascinating data points here:
The nightlies we're shipping are 35% slower than a locally compiled copy-- Edit: nightly has LLVM assertions enabled where my local compilers did notOk, so something looks clearly suspicious with LLVM! Let's try and take a closer
look. First, some data dumps of
-Z time-llvm-passes
nightly-2016-01-30
compilerUnfortunately I don't have much of a takeaway from this. Maybe others might
thought?
Next steps
can include in our local LLVM fork temporarily?
here?
The text was updated successfully, but these errors were encountered: