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.

Sign up for GitHub

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

Jump to bottom

LLVM -O2 compile time regression #25393

Closed
seanmonstar opened this issue May 14, 2015 · 6 comments · Fixed by #25423
Closed

LLVM -O2 compile time regression #25393

seanmonstar opened this issue May 14, 2015 · 6 comments · Fixed by #25423
Assignees
@dotdash
Labels
I-compiletime Issue: Problems and improvements with respect to compile times.

Comments

@seanmonstar
Copy link
Contributor

rustc 1.1.0-nightly (9ecc9896d 2015-05-10) (built 2015-05-11)

time: 58.110 LLVM passes

vs

rustc 1.1.0-nightly (c2b30b86d 2015-05-12) (built 2015-05-13)

time: 785.935 LLVM passes

running cargo rustc --release -- -Z time-passes --test on hyper@0.5.0

Full output at https://gist.github.com/seanmonstar/390e04d63ad28f234050
@huonw huonw added the I-compiletime Issue: Problems and improvements with respect to compile times. label May 14, 2015
@huonw
Copy link
Member

huonw commented May 14, 2015

cc @nrc (There's a slight suspicion that this may be caused by #24619, since that's touching trans/codegen.)

@seanmonstar
Copy link
Contributor Author

ran with rustc 1.1.0-nightly (4b88e8f63 2015-05-11) (built 2015-05-12), got time: 58.253 LLVM passes

@nrc
Copy link
Member

nrc commented May 14, 2015

cc @eddyb

@eddyb
Copy link
Member

eddyb commented May 14, 2015

@seanmonstar Could you provide .ll files for before and after? Hopefully cargo rustc --emit=llvm-ir should work now.

@seanmonstar
Copy link
Contributor Author

@eddyb https://gist.github.com/seanmonstar/b302a74d1e0aea3a59e2

@dotdash
Copy link
Contributor

dotdash commented May 14, 2015

Looks like this is caused by lilyball@e1e34e9 -- the assume intrinsics no longer get simplified into nonnull metadata on load instructions and cause massive slowdowns.

@dotdash dotdash self-assigned this May 15, 2015
dotdash added a commit to dotdash/rust that referenced this issue May 15, 2015
@dotdash
Fix major compile time regression
0260333 
The assume intrinsic has a strong, negative impact on compile times, so
we're currently only using it in places where LLVM can simplify it to
nonnull metadata on a load intruction. Unfortunately a recent change
that fixed invalid assume calls introduce new assume calls for which
this simplification can not happen, leading to a massive regression in
compile times in certain cases.

Moving the assumptions from the middle of the function to the beginning
allows the simplification to happen again, bringing compile times back
to their old levels.

Fixes rust-lang#25393
@dotdash dotdash mentioned this issue May 15, 2015
Merged
bors added a commit that referenced this issue May 15, 2015
@bors
Auto merge of #25423 - dotdash:assume, r=huonw
7ebaf1c 
The assume intrinsic has a strong, negative impact on compile times, so
we're currently only using it in places where LLVM can simplify it to
nonnull metadata on a load intruction. Unfortunately a recent change
that fixed invalid assume calls introduce new assume calls for which
this simplification can not happen, leading to a massive regression in
compile times in certain cases.

Moving the assumptions from the middle of the function to the beginning
allows the simplification to happen again, bringing compile times back
to their old levels.

Fixes #25393
@shepmaster shepmaster mentioned this issue May 16, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dotdash dotdash

Labels
I-compiletime Issue: Problems and improvements with respect to compile times.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix major compile time regression dotdash/rust
5 participants
@seanmonstar @eddyb @dotdash @nrc @huonw