Skip to content

Conversation

nnethercote
Copy link
Contributor

Some preliminary clean-ups that grease the path to #66961.

r? @alexcrichton

It's unused by any existing targets, and soon we'll be embedding full
bitcode by default anyway.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 25, 2020
@nnethercote nnethercote removed the request for review from alexcrichton March 25, 2020 06:17
@nnethercote nnethercote force-pushed the refactor-object-file-handling branch from 7d2aa8c to 72f1c6c Compare March 25, 2020 06:21
@nnethercote
Copy link
Contributor Author

In theory this shouldn't affect performance at all, but let's check that:

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Collaborator

bors commented Mar 25, 2020

⌛ Trying commit 72f1c6cfed177a356c7e05d16ba778cb1cb45078 with merge 8753044243364ce6751612dea9eae4439dcad8ce...

@michaelwoerister
Copy link
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.

@bors
Copy link
Collaborator

bors commented Mar 25, 2020

☀️ Try build successful - checks-azure
Build commit: 8753044243364ce6751612dea9eae4439dcad8ce (8753044243364ce6751612dea9eae4439dcad8ce)

@rust-timer
Copy link
Collaborator

Queued 8753044243364ce6751612dea9eae4439dcad8ce with parent cdb50c6, future comparison URL.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

I'm always a fan of cleanups :)

@nnethercote
Copy link
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 nnethercote force-pushed the refactor-object-file-handling branch from 72f1c6c to a50cca9 Compare March 26, 2020 03:14
@nnethercote
Copy link
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
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Mar 26, 2020

📌 Commit a50cca9 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 Mar 26, 2020
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
@bors bors merged commit b15423e into rust-lang:master Mar 27, 2020
@nnethercote nnethercote deleted the refactor-object-file-handling branch March 27, 2020 00:33
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.

6 participants