Skip to content

rustc: Handle modules in "fat" LTO more robustly #63956

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
Aug 29, 2019

Conversation

alexcrichton
Copy link
Member

When performing a "fat" LTO the compiler has a whole mess of codegen
units that it links together. To do this it needs to select one module
as a "base" module and then link everything else into this module.
Previously LTO passes assume that there's at least one module in-memory
to link into, but nowadays that's not always true! With incremental
compilation modules may actually largely be cached and it may be
possible that there's no in-memory modules to work with.

This commit updates the logic of the LTO backend to handle modules a bit
more uniformly during a fat LTO. This commit immediately splits them
into two lists, one serialized and one in-memory. The in-memory list is
then searched for the largest module and failing that we simply
deserialize the first serialized module and link into that. This
refactoring avoids juggling three lists, two of which are serialized
modules and one of which is half serialized and half in-memory.

Closes #63349

@rust-highfive
Copy link
Contributor

r? @eddyb

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 27, 2019
@rust-highfive

This comment has been minimized.

When performing a "fat" LTO the compiler has a whole mess of codegen
units that it links together. To do this it needs to select one module
as a "base" module and then link everything else into this module.
Previously LTO passes assume that there's at least one module in-memory
to link into, but nowadays that's not always true! With incremental
compilation modules may actually largely be cached and it may be
possible that there's no in-memory modules to work with.

This commit updates the logic of the LTO backend to handle modules a bit
more uniformly during a fat LTO. This commit immediately splits them
into two lists, one serialized and one in-memory. The in-memory list is
then searched for the largest module and failing that we simply
deserialize the first serialized module and link into that. This
refactoring avoids juggling three lists, two of which are serialized
modules and one of which is half serialized and half in-memory.

Closes rust-lang#63349
@michaelwoerister
Copy link
Member

Thanks, @alexcrichton! I suspected that it's a relatively simple problem like that. Great that you came up with a small test case!

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 28, 2019

📌 Commit 1a4330d has been approved by michaelwoerister

@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 Aug 28, 2019
// Sort out all our lists of incoming modules into two lists.
//
// * `serialized_modules` (also and argument to this function) contains all
// modules that are serialized in-memory.
Copy link
Member

@RalfJung RalfJung Aug 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So serialized modules are also in-memory? That makes serialized_modules vs in_memory not mutually exclusive, so maybe serialized_modules vs parsed_modules or so would make more sense?

As someone not at all familiar with this code, this is rather confusing.

Centril added a commit to Centril/rust that referenced this pull request Aug 29, 2019
…michaelwoerister

rustc: Handle modules in "fat" LTO more robustly

When performing a "fat" LTO the compiler has a whole mess of codegen
units that it links together. To do this it needs to select one module
as a "base" module and then link everything else into this module.
Previously LTO passes assume that there's at least one module in-memory
to link into, but nowadays that's not always true! With incremental
compilation modules may actually largely be cached and it may be
possible that there's no in-memory modules to work with.

This commit updates the logic of the LTO backend to handle modules a bit
more uniformly during a fat LTO. This commit immediately splits them
into two lists, one serialized and one in-memory. The in-memory list is
then searched for the largest module and failing that we simply
deserialize the first serialized module and link into that. This
refactoring avoids juggling three lists, two of which are serialized
modules and one of which is half serialized and half in-memory.

Closes rust-lang#63349
bors added a commit that referenced this pull request Aug 29, 2019
Rollup of 4 pull requests

Successful merges:

 - #62734 (Hide trait default methods)
 - #63953 (bootstrap: allow specifying mirror for bootstrap compiler download.)
 - #63956 (rustc: Handle modules in "fat" LTO more robustly)
 - #63979 (std: Remove the `wasm_syscall` feature)

Failed merges:

r? @ghost
@bors bors merged commit 1a4330d into rust-lang:master Aug 29, 2019
@bors
Copy link
Collaborator

bors commented Aug 29, 2019

⌛ Testing commit 1a4330d with merge fbdf1d2...

@pietroalbini
Copy link
Member

@bors r- retry

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 4, 2019
@alexcrichton alexcrichton deleted the fix-lto-all-cached branch September 20, 2019 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recompiling causes panic
7 participants