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

coverage: Eliminate more counters by giving them to unreachable nodes #136957

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

Zalathar
Copy link
Contributor

When preparing a function's coverage counters and metadata during codegen, any part of the original coverage graph that was removed by MIR optimizations can be treated as having an execution count of zero.

Somewhat counter-intuitively, if we give those unreachable nodes a higher priority for receiving physical counters (instead of counter expressions), that ends up reducing the total number of physical counters needed.

This works because if a node is unreachable, we don't actually create a physical counter for it. Instead that node gets a fixed zero counter, and any other node that would have relied on that physical counter in its counter expression can just ignore that term completely.

When preparing a function's coverage counters and metadata during codegen, any
part of the original coverage graph that was removed by MIR optimizations can
be treated as having an execution count of zero.

Somewhat counter-intuitively, if we give those unreachable nodes a _higher_
priority for receiving physical counters (instead of counter expressions), that
ends up reducing the total number of physical counters needed.

This works because if a node is unreachable, we don't actually create a
physical counter for it. Instead that node gets a fixed zero counter, and any
other node that would have relied on that physical counter in its counter
expression can just ignore that term completely.
@Zalathar Zalathar added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Feb 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 13, 2025

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 13, 2025

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@Zalathar
Copy link
Contributor Author

I was originally planning to combine this with some larger changes to transcribe_counters, but I wasn't yet happy with those other changes, so I decided to proceed with this smaller change as-is.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 13, 2025

r? @oli-obk

@bors r+

@bors
Copy link
Contributor

bors commented Feb 13, 2025

📌 Commit ab786d3 has been approved by oli-obk

It is now in the queue for this repository.

@rustbot rustbot assigned oli-obk and unassigned lcnr Feb 13, 2025
@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 Feb 13, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Feb 14, 2025
coverage: Eliminate more counters by giving them to unreachable nodes

When preparing a function's coverage counters and metadata during codegen, any part of the original coverage graph that was removed by MIR optimizations can be treated as having an execution count of zero.

Somewhat counter-intuitively, if we give those unreachable nodes a _higher_ priority for receiving physical counters (instead of counter expressions), that ends up reducing the total number of physical counters needed.

This works because if a node is unreachable, we don't actually create a physical counter for it. Instead that node gets a fixed zero counter, and any other node that would have relied on that physical counter in its counter expression can just ignore that term completely.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2025
…kingjubilee

Rollup of 11 pull requests

Successful merges:

 - rust-lang#136863 (rework rigid alias handling )
 - rust-lang#136869 (Fix diagnostic when using = instead of : in let binding)
 - rust-lang#136895 (debuginfo: Set bitwidth appropriately in enum variant tags)
 - rust-lang#136928 (eagerly prove WF when resolving fully qualified paths)
 - rust-lang#136941 (Move `llvm.ccache` to `build.ccache`)
 - rust-lang#136950 (rustdoc: use better, consistent SVG icons for scraped examples)
 - rust-lang#136957 (coverage: Eliminate more counters by giving them to unreachable nodes)
 - rust-lang#136960 (Compiletest should not inherit all host RUSTFLAGS)
 - rust-lang#136962 (unify LLVM version finding logic)
 - rust-lang#136970 (ci: move `x86_64-gnu-debug` job to the free runner)
 - rust-lang#136973 (Fix `x test --stage 1 ui-fulldeps` on macOS (until the next beta bump))

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d82ec95 into rust-lang:master Feb 14, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 14, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2025
Rollup merge of rust-lang#136957 - Zalathar:counters, r=oli-obk

coverage: Eliminate more counters by giving them to unreachable nodes

When preparing a function's coverage counters and metadata during codegen, any part of the original coverage graph that was removed by MIR optimizations can be treated as having an execution count of zero.

Somewhat counter-intuitively, if we give those unreachable nodes a _higher_ priority for receiving physical counters (instead of counter expressions), that ends up reducing the total number of physical counters needed.

This works because if a node is unreachable, we don't actually create a physical counter for it. Instead that node gets a fixed zero counter, and any other node that would have relied on that physical counter in its counter expression can just ignore that term completely.
@Zalathar Zalathar deleted the counters branch February 14, 2025 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants