Skip to content

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Apr 18, 2020

  • -Zprofile is broken with codegen units because GCOV assumes that each source file corresponds to one object file. This PR makes -Zprofile automatically set codegen units to 1 and gives an error if -Ccodegen-units=X is specified on the command line (with X != 1).
  • The profiler_builtins crate is not suitable for no_std applications since it contains C code that depends on libc. In such cases a custom implementation of the LLVM gcov API (llvm_gcov_init, llvm_gcda_*) is needed. This PR adds -Zno-profiler-runtime flag which inhibits automatic injection of the profiler_builtins crate.

cc @whitequark who implemented the original -Zprofile support

@rust-highfive
Copy link
Contributor

r? @davidtwco

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 18, 2020
Copy link
Contributor

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

LGTM, and good catch!

Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

r=me when CI passes

@Amanieu
Copy link
Member Author

Amanieu commented Apr 18, 2020

@bors r=davidtwco

@bors
Copy link
Collaborator

bors commented Apr 18, 2020

📌 Commit 9f23b2d has been approved by davidtwco

@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 Apr 18, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 18, 2020
Minor improvements to -Zprofile

- `-Zprofile` is broken with codegen units because GCOV assumes that each source file corresponds to one object file. This PR makes `-Zprofile` automatically set codegen units to 1 and gives an error if `-Ccodegen-units=X` is specified on the command line (with `X != 1`).
- The `profiler_builtins` crate is not suitable for `no_std` applications since it contains C code that depends on libc. In such cases a custom implementation of the LLVM gcov API (`llvm_gcov_init`, `llvm_gcda_*`) is needed. This PR adds `-Zno-profiler-runtime` flag which inhibits automatic injection of the `profiler_builtins` crate.

cc @whitequark who implemented the original `-Zprofile` support
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 18, 2020
Minor improvements to -Zprofile

- `-Zprofile` is broken with codegen units because GCOV assumes that each source file corresponds to one object file. This PR makes `-Zprofile` automatically set codegen units to 1 and gives an error if `-Ccodegen-units=X` is specified on the command line (with `X != 1`).
- The `profiler_builtins` crate is not suitable for `no_std` applications since it contains C code that depends on libc. In such cases a custom implementation of the LLVM gcov API (`llvm_gcov_init`, `llvm_gcda_*`) is needed. This PR adds `-Zno-profiler-runtime` flag which inhibits automatic injection of the `profiler_builtins` crate.

cc @whitequark who implemented the original `-Zprofile` support
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#71271 (Move `MapInPlace` to rustc_data_structures)
 - rust-lang#71276 (miri-unleashed: test that we detect heap allocations)
 - rust-lang#71283 (Minor improvements to -Zprofile)
 - rust-lang#71287 (Explain why we shouldn't add inline attr to into_vec)
 - rust-lang#71303 (remove build warnings)

Failed merges:

r? @ghost
@bors bors merged commit cd748ab into rust-lang:master Apr 19, 2020
@Amanieu
Copy link
Member Author

Amanieu commented Apr 20, 2020

Just published minicov which uses this to support code coverage for no_std.

@whitequark
Copy link
Contributor

@Amanieu Excellent work!

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Apr 23, 2020
… r=cramertj

Ignore -Zprofile when building compiler_builtins

rust-lang#70846 made the `compiler_builtins` crate ignore the default codegen-units setting and instead always split each function into a different codegen unit.

This unfortunately breaks `-Zprofile` which requires a single codegen unit per crate (see rust-lang#71283). You can notice this when building with `cargo -Zbuild-std` and `RUSTFLAGS` containing `-Zprofile`.

This PR works around this issue by just ignoring `-Zprofile` for the `compiler-builtins` crate.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 25, 2020
… r=cramertj

Ignore -Zprofile when building compiler_builtins

rust-lang#70846 made the `compiler_builtins` crate ignore the default codegen-units setting and instead always split each function into a different codegen unit.

This unfortunately breaks `-Zprofile` which requires a single codegen unit per crate (see rust-lang#71283). You can notice this when building with `cargo -Zbuild-std` and `RUSTFLAGS` containing `-Zprofile`.

This PR works around this issue by just ignoring `-Zprofile` for the `compiler-builtins` crate.
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.

5 participants