Can add_unreachable_coverage() in mapgen.rs be simplified?
#79622
Labels
A-code-coverage
Area: Source-based code coverage (-Cinstrument-coverage)
C-enhancement
Category: An issue proposing an enhancement or a PR with one.
Comments
|
cc: @tmandry |
wesleywiser
added
A-code-coverage
|
It's worth noting that a weakness of the current approach is that crates which only define generic functions won't get zero counters for their unused functions. It would be nice to fix that edge case somehow. |
|
I agree (assuming this is not particularly rare). As we discussed, maybe a way to do that is to forcibly inject a I'm not sure where/how to do that most efficiently, but it's probably not impossible. @wesleywiser - I think I saw you've worked on code related to |
|
(We probably would also have to mark that function "used" in LLVM IR to prevent it being optimized out.) |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Jan 5, 2022
…ioning, r=tmandry [code coverage] Fix missing dead code in modules that are never called The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead). The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols. This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs. Fixes rust-lang#91661 Fixes rust-lang#86177 Fixes rust-lang#85718 Fixes rust-lang#79622 r? `@tmandry` cc `@richkadel` This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Jan 12, 2022
…ioning, r=tmandry [code coverage] Fix missing dead code in modules that are never called The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead). The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols. This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs. Fixes rust-lang#91661 Fixes rust-lang#86177 Fixes rust-lang#85718 Fixes rust-lang#79622 r? `@tmandry` cc `@richkadel` This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Jan 13, 2022
…ioning, r=tmandry [code coverage] Fix missing dead code in modules that are never called The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead). The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols. This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs. Fixes rust-lang#91661 Fixes rust-lang#86177 Fixes rust-lang#85718 Fixes rust-lang#79622 r? ``@tmandry`` cc ``@richkadel`` This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Jan 13, 2022
…ioning, r=tmandry [code coverage] Fix missing dead code in modules that are never called The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead). The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols. This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs. Fixes rust-lang#91661 Fixes rust-lang#86177 Fixes rust-lang#85718 Fixes rust-lang#79622 r? ```@tmandry``` cc ```@richkadel``` This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
See #79109 (comment)
The PR introduced a solution to adding
Zerocoverage for functions that were processed for MIR, but not codegenned, because they were optimized out and never reached (never called).The algorithm for finding and injecting the additional code regions, in the right place, is a bit complex.
Can this solution be simplified and/or improved? Are there other sources of compiler state data that might help (or better sources that could be exposed via
cx,tcx, or something else, but aren't yet)?(Related to Tracking Issue #79121)
The text was updated successfully, but these errors were encountered: