Save metadata even with -Z no-trans (e.g. for multi-crate cargo check). by eddyb · Pull Request #33602 · rust-lang/rust

eddyb · 2016-05-12T20:56:13Z

Removes the item symbol map in metadata, as we can now generate them in a deterministic manner.
The -Z no-trans change lets the LLVM passes and linking run, but with just metadata and no code.
It fails while trying to link a binary because there's no main function, which is correct but not good UX.

There's also no way to easily throw away all of the artifacts to rebuild with actual code generation.
We might want cargo check to do that using cargo-internal information and then it would just work.

cc @alexcrichton @nikomatsakis @Aatch @michaelwoerister

rust-highfive · 2016-05-12T20:56:18Z

r? @jroesch

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

michaelwoerister · 2016-05-12T21:52:50Z

I'm going to review this right now..

michaelwoerister · 2016-05-12T22:43:33Z

@bors: r+

Perfect, @eddyb!

bors · 2016-05-12T22:43:34Z

📌 Commit adee551 has been approved by michaelwoerister

eddyb · 2016-05-13T05:31:57Z

@bors r- Potentially relevant failure

eddyb · 2016-05-13T05:44:27Z

@bors r=michaelwoerister d'oh this PR wasn't even in the rollup 😫

bors · 2016-05-13T05:44:29Z

📌 Commit adee551 has been approved by michaelwoerister

@alexcrichton

…rister Save metadata even with -Z no-trans (e.g. for multi-crate cargo check). Removes the item symbol map in metadata, as we can now generate them in a deterministic manner. The `-Z no-trans` change lets the LLVM passes and linking run, but with just metadata and no code. It fails while trying to link a binary because there's no `main` function, which is correct but not good UX. There's also no way to easily throw away all of the artifacts to rebuild with actual code generation. We might want `cargo check` to do that using cargo-internal information and then it would just work. cc @alexcrichton @nikomatsakis @Aatch @michaelwoerister

eddyb · 2016-05-13T13:30:06Z

@bors rollup-

bors · 2016-05-14T13:56:54Z

☔ The latest upstream changes (presumably #33632) made this pull request unmergeable. Please resolve the merge conflicts.

eddyb · 2016-05-14T19:15:08Z

@bors r=michaelwoerister

bors · 2016-05-14T19:15:09Z

📌 Commit 5087556 has been approved by michaelwoerister

eddyb · 2016-05-14T19:16:23Z

Oh wait I didn't fix the failure in @alexcrichton's test.

@bors r-

eddyb · 2016-05-14T19:20:30Z

@bors r=michaelwoerister

bors · 2016-05-14T19:20:30Z

📌 Commit d2ded31 has been approved by michaelwoerister

bors · 2016-05-15T07:49:36Z

⌛ Testing commit d2ded31 with merge 33a8175...

bors · 2016-05-25T04:52:01Z

💔 Test failed - auto-linux-64-opt-rustbuild

eddyb · 2016-05-25T05:47:00Z

@bors r=michaelwoerister

bors · 2016-05-25T05:47:00Z

📌 Commit a619901 has been approved by michaelwoerister

bors · 2016-05-25T09:33:30Z

⌛ Testing commit a619901 with merge 5229e0e...

@alexcrichton

Save metadata even with -Z no-trans (e.g. for multi-crate cargo check). Removes the item symbol map in metadata, as we can now generate them in a deterministic manner. The `-Z no-trans` change lets the LLVM passes and linking run, but with just metadata and no code. It fails while trying to link a binary because there's no `main` function, which is correct but not good UX. There's also no way to easily throw away all of the artifacts to rebuild with actual code generation. We might want `cargo check` to do that using cargo-internal information and then it would just work. cc @alexcrichton @nikomatsakis @Aatch @michaelwoerister

bors · 2016-05-25T14:22:58Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-32cross-opt, auto-linux-64-cargotest, auto-linux-64-cross-armhf, auto-linux-64-cross-armsf, auto-linux-64-cross-freebsd, auto-linux-64-cross-netbsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-cross-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild

@Aatch

…haelwoerister Drive trans from the output of the translation item collector This PR changes the way how translation works above the item level. Instead of walking the HIR and calling `trans_item()` on everything encountered (while instantiating monomorphizations on-demand), we now just process the list of translation items generated by the `trans::collector`. Using the collector has the benefit of being able to know the exact set of monomorphizations and symbols before actually running translation, something that is crucial for incremental compilation (but also has [other benefits](#33602)). The collector has existed for quite a while now, but so far it's output was only used for running some auto-tests. With this PR it becomes the only source of truth about what gets translated. One modification we had to make, compared to the initial approach, is that closures are not represented as their own `TransItems`. Doing so, while still supporting non-MIR-based translation, would have been prohibitively complex, and not worth the trouble since legacy-trans will disappear sooner or later. Once there is solely MIR-trans, it would be a good idea to make closures `TransItems` again. This PR removes the most obvious functions and tables that are not needed anymore, but there's definitely still more cleanup possible later on (e.g. `monomorphize::monomorphic_fn()` does very little at this point). Since there are already more than 10 commits in here, doing this in a separate PR seems to be a better idea. These changes definitely warrant a crater run. Thanks @Aatch, for taking on one of the more tedious tasks during the dev-sprint! Thanks @eddyb, for doing some nice refactorings to symbol name generation and making sure these landed so I could use them! cc @rust-lang/compiler cc @rust-lang/tools

@Aatch

Drive trans from the output of the translation item collector This PR changes the way how translation works above the item level. Instead of walking the HIR and calling `trans_item()` on everything encountered (while instantiating monomorphizations on-demand), we now just process the list of translation items generated by the `trans::collector`. Using the collector has the benefit of being able to know the exact set of monomorphizations and symbols before actually running translation, something that is crucial for incremental compilation (but also has [other benefits](#33602)). The collector has existed for quite a while now, but so far it's output was only used for running some auto-tests. With this PR it becomes the only source of truth about what gets translated. One modification we had to make, compared to the initial approach, is that closures are not represented as their own `TransItems`. Doing so, while still supporting non-MIR-based translation, would have been prohibitively complex, and not worth the trouble since legacy-trans will disappear sooner or later. Once there is solely MIR-trans, it would be a good idea to make closures `TransItems` again. This PR removes the most obvious functions and tables that are not needed anymore, but there's definitely still more cleanup possible later on (e.g. `monomorphize::monomorphic_fn()` does very little at this point). Since there are already more than 10 commits in here, doing this in a separate PR seems to be a better idea. These changes definitely warrant a crater run. Thanks @Aatch, for taking on one of the more tedious tasks during the dev-sprint! Thanks @eddyb, for doing some nice refactorings to symbol name generation and making sure these landed so I could use them! cc @rust-lang/compiler cc @rust-lang/tools

@Aatch

Drive trans from the output of the translation item collector This PR changes the way how translation works above the item level. Instead of walking the HIR and calling `trans_item()` on everything encountered (while instantiating monomorphizations on-demand), we now just process the list of translation items generated by the `trans::collector`. Using the collector has the benefit of being able to know the exact set of monomorphizations and symbols before actually running translation, something that is crucial for incremental compilation (but also has [other benefits](#33602)). The collector has existed for quite a while now, but so far it's output was only used for running some auto-tests. With this PR it becomes the only source of truth about what gets translated. One modification we had to make, compared to the initial approach, is that closures are not represented as their own `TransItems`. Doing so, while still supporting non-MIR-based translation, would have been prohibitively complex, and not worth the trouble since legacy-trans will disappear sooner or later. Once there is solely MIR-trans, it would be a good idea to make closures `TransItems` again. This PR removes the most obvious functions and tables that are not needed anymore, but there's definitely still more cleanup possible later on (e.g. `monomorphize::monomorphic_fn()` does very little at this point). Since there are already more than 10 commits in here, doing this in a separate PR seems to be a better idea. These changes definitely warrant a crater run. Thanks @Aatch, for taking on one of the more tedious tasks during the dev-sprint! Thanks @eddyb, for doing some nice refactorings to symbol name generation and making sure these landed so I could use them! cc @rust-lang/compiler cc @rust-lang/tools

rust-highfive assigned jroesch May 12, 2016

michaelwoerister assigned michaelwoerister and unassigned jroesch May 12, 2016

eddyb mentioned this pull request May 13, 2016
Rollup of 29 pull requests #33610
Closed

eddyb mentioned this pull request May 14, 2016
rustc: Add a new crate type, cdylib #33553
Merged

eddyb force-pushed the no-trans--check branch from adee551 to 5087556Compare May 14, 2016 19:14

eddyb mentioned this pull request May 14, 2016
Rollup of 27 pull requests #33643
Merged

trans: save metadata even with -Z no-trans.
a619901

eddyb force-pushed the no-trans--check branch from 1bf0829 to a619901Compare May 25, 2016 05:46

bors mentioned this pull request May 25, 2016
add more comprehensive tests for incr. comp. #33851
Closed

bors merged commit a619901 into rust-lang:masterMay 25, 2016

eddyb deleted the no-trans--check branch May 25, 2016 14:23

bors mentioned this pull request May 25, 2016
Projection cache and better warnings for #32330 #33816
Merged

michaelwoerister mentioned this pull request May 26, 2016
Drive trans from the output of the translation item collector #33890
Merged

eddyb mentioned this pull request Jun 4, 2016
many major compile-time perf regressions in jld-day15-parser #34042
Closed

bluss mentioned this pull request Oct 30, 2016
Use no-trans for every crate rsolomo/cargo-check#16
Open

Uh oh!

Uh oh!

Conversation

eddyb commented May 12, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented May 12, 2016

Uh oh!

michaelwoerister commented May 12, 2016

Uh oh!

michaelwoerister commented May 12, 2016

Uh oh!

bors commented May 12, 2016

Uh oh!

eddyb commented May 13, 2016

Uh oh!

eddyb commented May 13, 2016

Uh oh!

bors commented May 13, 2016

Uh oh!

eddyb commented May 13, 2016

Uh oh!

bors commented May 14, 2016

Uh oh!

eddyb commented May 14, 2016

Uh oh!

bors commented May 14, 2016

Uh oh!

eddyb commented May 14, 2016

Uh oh!

eddyb commented May 14, 2016

Uh oh!

bors commented May 14, 2016

Uh oh!

bors commented May 15, 2016

Uh oh!

bors commented May 25, 2016

Uh oh!

eddyb commented May 25, 2016

Uh oh!

bors commented May 25, 2016

Uh oh!

bors commented May 25, 2016

Uh oh!

bors commented May 25, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eddyb commented May 12, 2016•
edited
Loading