Skip to content

Fix major compile time regression #25423

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

Merged
merged 1 commit into from
May 15, 2015
Merged

Fix major compile time regression #25423

merged 1 commit into from
May 15, 2015

Conversation

dotdash
Copy link
Contributor

@dotdash dotdash commented May 15, 2015

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

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
@rust-highfive
Copy link
Contributor

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@huonw
Copy link
Member

huonw commented May 15, 2015

@bors r+

Nice investigation!

@bors
Copy link
Collaborator

bors commented May 15, 2015

📌 Commit 0260333 has been approved by huonw

@bors
Copy link
Collaborator

bors commented May 15, 2015

⌛ Testing commit 0260333 with merge daa46b4...

@bors
Copy link
Collaborator

bors commented May 15, 2015

💔 Test failed - auto-linux-32-nopt-t

@pnkfelix
Copy link
Member

I'm going to assume that this is a transient build test failure:

failures:
    net::tcp::tests::fast_rebind

test result: FAILED. 646 passed; 1 failed; 0 ignored; 0 measured

@pnkfelix
Copy link
Member

@bors retry

bors added a commit that referenced this pull request May 15, 2015
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
@bors
Copy link
Collaborator

bors commented May 15, 2015

⌛ Testing commit 0260333 with merge 7ebaf1c...

@bors bors merged commit 0260333 into rust-lang:master May 15, 2015
@dotdash dotdash deleted the assume branch July 27, 2015 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LLVM -O2 compile time regression
6 participants