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

incr.comp.: Don't automatically enable -Zshare-generics for incr. comp. builds. #54557

Merged

Conversation

michaelwoerister
Copy link
Member

So far the compiler would automatically enable sharing of monomorphizations for incremental builds. That was OK because without (Thin)LTO this could have very little impact on the runtime performance of the generated code. However, since #53673, ThinLTO and incr. comp. can be combined, so the trade-off is not as clear anymore.

This PR removes the automatic tie between the two options. Whether monomorphizations are shared between crates or not now only depends on the optimization level.

r? @alexcrichton

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 25, 2018
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Sep 25, 2018

📌 Commit 8fc7b5d has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Sep 25, 2018
…nerics-for-incr-comp, r=alexcrichton

incr.comp.: Don't automatically enable -Zshare-generics for incr. comp. builds.

So far the compiler would automatically enable sharing of monomorphizations for incremental builds. That was OK because without (Thin)LTO this could have very little impact on the runtime performance of the generated code. However, since rust-lang#53673, ThinLTO and incr. comp. can be combined, so the trade-off is not as clear anymore.

This PR removes the automatic tie between the two options. Whether monomorphizations are shared between crates or not now _only_ depends on the optimization level.

r? @alexcrichton
bors added a commit that referenced this pull request Sep 25, 2018
Rollup of 12 pull requests

Successful merges:

 - #53518 (Add doc for impl From in char_convert)
 - #54058 (Introduce the partition_dedup/by/by_key methods for slices)
 - #54281 (Search box)
 - #54368 (Reduce code block sides padding)
 - #54498 (The project moved under the Mozilla umbrella)
 - #54518 (resolve: Do not block derive helper resolutions on single import resolutions)
 - #54522 (Fixed three small typos.)
 - #54529 (aarch64-pc-windows-msvc: Don't link libpanic_unwind to libtest.)
 - #54537 (Rename slice::exact_chunks() to slice::chunks_exact())
 - #54539 (Fix js error)
 - #54557 (incr.comp.: Don't automatically enable -Zshare-generics for incr. comp. builds.)
 - #54558 (Improvements to finding LLVM's FileCheck)

Failed merges:

r? @ghost
@bors bors merged commit 8fc7b5d into rust-lang:master Sep 26, 2018
@michaelwoerister
Copy link
Member Author

Nominating for a beta-backport. This is one of the reasons why bootstrapping with incremental=true is still rather slow.

@michaelwoerister michaelwoerister added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 26, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Oct 4, 2018

Seems like a risk-free patch. Approving for backport ahead of meeting to save time.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Oct 4, 2018
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 4, 2018
bors added a commit that referenced this pull request Oct 4, 2018
[beta] Rollup backports

Merged and approved:

* #54605: resolve: Disambiguate a subset of conflicts "macro_rules" vs "macro name in module"
* #54557: incr.comp.: Don't automatically enable -Zshare-generics for incr. comp. builds.
* #54759: do not normalize non-scalar constants to a ConstValue::ScalarPair

Closes #54759
r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants