Skip to content
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

bootstrap: build crates under libtest with -Z emit-stack-sizes #59401

Merged
merged 2 commits into from
Mar 29, 2019

Conversation

japaric
Copy link
Member

@japaric japaric commented Mar 24, 2019

Please see the comment in the diff for the rationale.

This change adds a .stack_sizes linker section to libcompiler_builtins.rlib
but this section is discarded by the linker by default so it won't affect the
binary size of most programs. It will, however, negatively affect the binary
size of programs that link to a recent release of the cortex-m-rt crate
because of the linker script that crate provides, but I have proposed a PR
(rust-embedded/cortex-m-rt#186) to solve the problem (which I originally
introduced :-)).

This change does increase the size of the libcompiler_builtins.rlib artifact we
distribute but the increase is in the order of (a few) KBs.

r? @alexcrichton

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 24, 2019
@alexcrichton
Copy link
Member

Seems reasonable to me! I wonder though if we should just go ahead and do this for all of the binaries? (libstd/libtest, not rustc). The size costs here are minimal enough that we can just eat them, and this may be useful information for non-LTO situations as well perhaps?

@japaric
Copy link
Member Author

japaric commented Mar 25, 2019

I wonder though if we should just go ahead and do this for all of the binaries? (libstd/libtest, not rustc)

Sounds good to me. Any suggestion for passing the flag to all the crates below libtest? I thought about using !crate_name.contains("rustc") but that would include crates like syntax.

@alexcrichton
Copy link
Member

I think what you'll want to do is set an env var read by the wrapper in the std_cargo and test_cargo functions in this file, and that should apply only to the libstd/libtest builds?

@japaric japaric changed the title bootstrap: build compiler-builtins with -Z emit-stack-sizes bootstrap: build crates under libtest with -Z emit-stack-sizes Mar 25, 2019
@japaric
Copy link
Member Author

japaric commented Mar 25, 2019

@alexcrichton I have updated the PR to build all crates under libtest with -Zemit-stack-sizes

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 25, 2019

📌 Commit 7d365cf 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 25, 2019
Centril added a commit to Centril/rust that referenced this pull request Mar 29, 2019
…es, r=alexcrichton

bootstrap: build crates under libtest with -Z emit-stack-sizes

Please see the comment in the diff for the rationale.

This change adds a `.stack_sizes` linker section to `libcompiler_builtins.rlib`
but this section is discarded by the linker by default so it won't affect the
binary size of most programs. It will, however, negatively affect the binary
size of programs that link to a recent release of the `cortex-m-rt` crate
because of the linker script that crate provides, but I have proposed a PR
(rust-embedded/cortex-m-rt#186) to solve the problem (which I originally
introduced :-)).

This change does increase the size of the `libcompiler_builtins.rlib` artifact we
distribute but the increase is in the order of (a few) KBs.

r? @alexcrichton
bors added a commit that referenced this pull request Mar 29, 2019
Rollup of 11 pull requests

Successful merges:

 - #58019 (Combine all builtin late lints and make lint checking parallel)
 - #59358 (Use `track_errors` instead of hand rolling)
 - #59394 (warn -> deny duplicate match bindings)
 - #59401 (bootstrap: build crates under libtest with -Z emit-stack-sizes)
 - #59423 (Visit path in `walk_mac`)
 - #59468 (musl: build toolchain libs with -fPIC)
 - #59476 (Use `SmallVec` in `TokenStreamBuilder`.)
 - #59496 (Remove unnecessary with_globals calls)
 - #59498 (Use 'write_all' instead of 'write' in example code)
 - #59503 (Stablize {f32,f64}::copysign().)
 - #59511 (Fix missed fn rename in #59284)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Mar 29, 2019
Rollup of 11 pull requests

Successful merges:

 - #58019 (Combine all builtin late lints and make lint checking parallel)
 - #59358 (Use `track_errors` instead of hand rolling)
 - #59394 (warn -> deny duplicate match bindings)
 - #59401 (bootstrap: build crates under libtest with -Z emit-stack-sizes)
 - #59423 (Visit path in `walk_mac`)
 - #59468 (musl: build toolchain libs with -fPIC)
 - #59476 (Use `SmallVec` in `TokenStreamBuilder`.)
 - #59496 (Remove unnecessary with_globals calls)
 - #59498 (Use 'write_all' instead of 'write' in example code)
 - #59503 (Stablize {f32,f64}::copysign().)
 - #59511 (Fix missed fn rename in #59284)

Failed merges:

r? @ghost
@bors bors merged commit 7d365cf into rust-lang:master Mar 29, 2019
pnkfelix added a commit to pnkfelix/rust that referenced this pull request Apr 12, 2019
…eta regression).

This is result of squashing two revert commits:

Revert "compile all crates under test w/ -Zemit-stack-sizes"

This reverts commit 7d365cf.

Revert "bootstrap: build compiler-builtins with -Z emit-stack-sizes"

This reverts commit 8b8488c.
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
…t-stack-sizes-gc-sections-ld-gold-bug, r=alexcrichton

Revert "compile crates under test w/ -Zemit-stack-sizes"

Revert PR rust-lang#59401 to fix issue rust-lang#59652 (a stable-to-beta regression).

This is result of squashing two revert commits:

Revert "compile all crates under test w/ -Zemit-stack-sizes"

This reverts commit 7d365cf.

Revert "bootstrap: build compiler-builtins with -Z emit-stack-sizes"

This reverts commit 8b8488c.

----

(My intention is that someone can re-add this code again later, either after the `ld.gold` issue itself is fixed, or with safe-guards to check whether `ld.gold` is in use and then issuing warnings about the problems here when they arise.)
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
…t-stack-sizes-gc-sections-ld-gold-bug, r=alexcrichton

Revert "compile crates under test w/ -Zemit-stack-sizes"

Revert PR rust-lang#59401 to fix issue rust-lang#59652 (a stable-to-beta regression).

This is result of squashing two revert commits:

Revert "compile all crates under test w/ -Zemit-stack-sizes"

This reverts commit 7d365cf.

Revert "bootstrap: build compiler-builtins with -Z emit-stack-sizes"

This reverts commit 8b8488c.

----

(My intention is that someone can re-add this code again later, either after the `ld.gold` issue itself is fixed, or with safe-guards to check whether `ld.gold` is in use and then issuing warnings about the problems here when they arise.)
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
…t-stack-sizes-gc-sections-ld-gold-bug, r=alexcrichton

Revert "compile crates under test w/ -Zemit-stack-sizes"

Revert PR rust-lang#59401 to fix issue rust-lang#59652 (a stable-to-beta regression).

This is result of squashing two revert commits:

Revert "compile all crates under test w/ -Zemit-stack-sizes"

This reverts commit 7d365cf.

Revert "bootstrap: build compiler-builtins with -Z emit-stack-sizes"

This reverts commit 8b8488c.

----

(My intention is that someone can re-add this code again later, either after the `ld.gold` issue itself is fixed, or with safe-guards to check whether `ld.gold` is in use and then issuing warnings about the problems here when they arise.)
pietroalbini pushed a commit to pietroalbini/rust that referenced this pull request Apr 26, 2019
…eta regression).

This is result of squashing two revert commits:

Revert "compile all crates under test w/ -Zemit-stack-sizes"

This reverts commit 7d365cf.

Revert "bootstrap: build compiler-builtins with -Z emit-stack-sizes"

This reverts commit 8b8488c.
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.

4 participants