Refactor translation unit partitioning/collection as a query #44529
Conversation
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
|
☔ The latest upstream changes (presumably #44420) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Thanks, Alex! At first I wasn't sure if I liked the My only nit would be that the fields of r=me after conflicts have been resolved. |
|
Oh sure that's fine, I'll add some method accessors in rustc and use those throughout librustc_trans? |
|
Yes, that should work. |
alexcrichton
force-pushed
the
trans-query
branch
from
ba2fc3b
to
f744cc2
Compare
|
@bors: r=michaelwoerister |
|
📌 Commit f744cc2 has been approved by |
|
Ok I've pushed another commit which changes again how exported symbols are dealt with. Everything should now be behind queries and moving us one step closer to 'codegen as a query' now that |
|
re-r? @michaelwoerister |
aidanhs
added
the
S-waiting-on-review
alexcrichton
force-pushed
the
trans-query
branch
2 times, most recently
from
829de6b
to
dd8dbc5
Compare
|
Alright and to round this off some more, the last commits now make codegen 100% query-ified. |
src/librustc_trans/back/write.rs
Outdated
| @@ -666,19 +669,40 @@ fn need_crate_bitcode_for_rlib(sess: &Session) -> bool { | |||
| sess.opts.output_types.contains_key(&OutputType::Exe) | |||
| } | |||
|
|
|||
| pub fn start_async_translation(sess: &Session, | |||
| pub fn start_async_translation(tcx: TyCtxt, | |||
| // other codegen unit is also not exported then like with the foreign | ||
| // case we apply a hidden visibility. If the symbol is exported from | ||
| // the foreign object file, however, then we leave this at the | ||
| // default visibility as we'll just import it naturally. |
|
@bors r+ Thanks! Excellent |
|
📌 Commit dd8dbc5 has been approved by |
|
@bors: r=michaelwoerister |
|
💡 This pull request was already approved, no need to approve it again.
|
|
📌 Commit dd8dbc5 has been approved by |
|
@bors: r=michaelwoerister |
|
💡 This pull request was already approved, no need to approve it again.
|
|
📌 Commit dd8dbc5 has been approved by |
|
@bors: r=michaelwoerister |
|
📌 Commit 4f8b10b has been approved by |
|
☔ The latest upstream changes (presumably #44502) made this pull request unmergeable. Please resolve the merge conflicts. |
alexcrichton
force-pushed
the
trans-query
branch
from
4f8b10b
to
d1b4773
Compare
|
@bors: r=michaelwoerister |
|
📌 Commit d1b4773 has been approved by |
|
☔ The latest upstream changes (presumably #44634) made this pull request unmergeable. Please resolve the merge conflicts. |
This commit refactors the `collect_crate_translation_items` function to only require the `TyCtxt` instead of a `SharedCrateContext` in preparation for query-ifying this portion of trans.
This commit refactors the the `partitioning::partition` function to operate with a `TyCtxt` instead of a `SharedCrateContext` in preparation for making it a query.
Turns out this was already set up as a query, just wasn't using it yet!
This commit moves the definition of the `ExportedSymbols` structure to the `rustc` crate and then creates a query that'll be used to construct the `ExportedSymbols` set. This in turn uses the reachablity query exposed in the previous commit.
This commit moves the `collect_and_partition_translation_items` function into a query on `TyCtxt` instead of a free function in trans, allowing us to track dependencies and such of the function.
Otherwise we may emit double errors related to the `#[export_name]` attribute, for example, and using a query should ensure that it's only emitted at most once.
This is a big map that ends up inside of a `CrateContext` during translation for all codegen units. This means that any change to the map may end up causing an incremental recompilation of a codegen unit! In order to reduce the amount of dependencies here between codegen units and the actual input crate this commit refactors dealing with exported symbols and such into various queries. The new queries are largely based on existing queries with filled out implementations for the local crate in addition to external crates, but the main idea is that while translating codegen untis no unit needs the entire set of exported symbols, instead they only need queries about particulare `DefId` instances every now and then. The linking stage, however, still generates a full list of all exported symbols from all crates, but that's going to always happen unconditionally anyway, so no news there!
I believe this comment here is mostly talking about the `ptrcast` function call below, so move the comment down to that block.
This commit removes the `crate_trans_items` field from the `CrateContext` of trans. This field, a big map, was calculated during partioning and was a set of all translation items. This isn't quite incremental-friendly because the map may change a lot but not have much effect on downstream consumers. Instead a new query was added for the one location this map was needed, along with a new comment explaining what the location is doing!
This commit attaches a channel to the LLVM workers to the `TyCtxt` which will later be used during the codegen query to actually send work to LLVM workers. Otherwise this commit is just plumbing this channel throughout the compiler to ensure it reaches the right consumers.
This commit moves the actual code generation in the compiler behind a query keyed by a codegen unit's name. This ended up entailing quite a few internal refactorings to enable this, along with a few cut corners: * The `OutputFilenames` structure is now tracked in the `TyCtxt` as it affects a whole bunch of trans and such. This is now behind a query and threaded into the construction of the `TyCtxt`. * The `TyCtxt` now has a channel "out the back" intended to send data to worker threads in rustc_trans. This is used as a sort of side effect of the codegen query but morally what's happening here is the return value of the query (currently unit but morally a path) is only valid once the background threads have all finished. * Dispatching work items to the codegen threads was refactored to only rely on data in `TyCtxt`, which mostly just involved refactoring where data was stored, moving it from the translation thread to the controller thread's `CodegenContext` or the like. * A new thread locals was introduced in trans to work around the query system. This is used in the implementation of `assert_module_sources` which looks like an artifact of the old query system and will presumably go away once red/green is up and running.
alexcrichton
force-pushed
the
trans-query
branch
from
d1b4773
to
6d614dd
Compare
|
@bors: r=michaelwoerister |
|
📌 Commit 6d614dd has been approved by |
Refactor translation unit partitioning/collection as a query This commit is targeted at #44486 with the ultimate goal of making the `collect_and_partition_translation_items` function a query. This mostly just involved query-ifying a few other systems along with plumbing the tcx instead of `SharedCrateContext` in a few locations. Currently this only tackles the first bullet of #44486 and doesn't add a dedicated query for a particular codegen unit. I wasn't quite sure how to do that yet but figured this was good to put up. Closes #44486
|
☀️ Test successful - status-appveyor, status-travis |
This commit is targeted at #44486 with the ultimate goal of making the
collect_and_partition_translation_itemsfunction a query. This mostly just involved query-ifying a few other systems along with plumbing the tcx instead ofSharedCrateContextin a few locations.Currently this only tackles the first bullet of #44486 and doesn't add a dedicated query for a particular codegen unit. I wasn't quite sure how to do that yet but figured this was good to put up.
Closes #44486