Refactor object file handling#70384
Merged
bors merged 3 commits intorust-lang:masterfrom Mar 27, 2020
Merged
Conversation
It's unused by any existing targets, and soon we'll be embedding full bitcode by default anyway.
rust-highfive
added
the
S-waiting-on-review
nnethercote
force-pushed
the
refactor-object-file-handling
branch
from
7d2aa8c to
72f1c6c
Compare
Contributor
Author
|
In theory this shouldn't affect performance at all, but let's check that: @bors try @rust-timer queue |
Collaborator
|
Awaiting bors try build completion |
Collaborator
|
⌛ Trying commit 72f1c6cfed177a356c7e05d16ba778cb1cb45078 with merge 8753044243364ce6751612dea9eae4439dcad8ce... |
Member
|
If I remember correctly "marker" bitcode embedding had something to do with iOS compatibility? #35968 has some discussion on it; but @alexcrichton would know more about that anyway. |
Collaborator
|
☀️ Try build successful - checks-azure |
Collaborator
|
Queued 8753044243364ce6751612dea9eae4439dcad8ce with parent cdb50c6, future comparison URL. |
Member
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
I'm always a fan of cleanups :)
Contributor
Author
|
The perf impact is negligible, as expected. |
Currently, there are three fields in `ModuleConfig` that dictate how object files are emitted: `emit_obj`, `obj_is_bitcode`, and `embed_bitcode`. Some of the combinations of these fields are nonsensical, in particular having both `obj_is_bitcode` and `embed_bitcode` true at the same time. Also, currently: - we needlessly emit and then delete a bytecode file if `obj_is_bitcode` is true but `emit_obj` is false; - we needlessly embed bitcode in the LLVM module if `embed_bitcode` is true and `emit_obj` is false. This commit combines the three fields into one, with a new type `EmitObj` (and the auxiliary `BitcodeSection`) which can encode five different possibilities. In the old code, `set_flags` would set `obj_is_bitcode` and `embed_bitcode` on all three of the configs (`modules`, `allocator`, `metadata`) if the relevant other conditions were met, even if no object code needed to be emitted for one or more of them. Whereas `start_async_codegen` would set `emit_obj`, but only for those configs that need it. In the new code, `start_async_codegen` does all the work of setting `emit_obj`, and it only does that for the configs that need it. `set_flags` no longer sets anything related to object file emission.
It makes things a little clearer.
nnethercote
force-pushed
the
refactor-object-file-handling
branch
from
72f1c6c to
a50cca9
Compare
Contributor
Author
|
@alexcrichton New code is up. I reinstated marker bitcode sections (in a nicer fashion than we had before) and did other minor changes you suggested where possible. |
alexcrichton
approved these changes
Mar 26, 2020
Member
|
@bors: r+ |
Collaborator
|
📌 Commit a50cca9 has been approved by |
bors
added
S-waiting-on-bors
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Mar 26, 2020
…ndling, r=alexcrichton Refactor object file handling Some preliminary clean-ups that grease the path to rust-lang#66961. r? @alexcrichton
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 26, 2020
Rollup of 6 pull requests Successful merges: - rust-lang#70384 (Refactor object file handling) - rust-lang#70397 (add 'fn write_u16s' to Memory) - rust-lang#70413 (Fix incorrect pattern warning "unreachable pattern") - rust-lang#70428 (`error_bad_item_kind`: add help text) - rust-lang#70429 (Clean up E0459 explanation) - rust-lang#70437 (Miri float->int casts: be explicit that this is saturating) Failed merges: r? @ghost
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some preliminary clean-ups that grease the path to #66961.
r? @alexcrichton