Allow MIR pass UnreachableProp when coverage is enabled
#116938
Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
|
cc @cjgillot for awareness because you have been working on this pass, though I don't think this change directly affects your work |
|
@rustbot label +A-code-coverage +A-mir-opt |
rustbot
added
A-code-coverage
Zalathar
changed the title
UnreachableProp when coverage is enabled
| /// instrumented, and it will disappear from coverage mappings and coverage reports. | ||
| /// | ||
| /// This pass detects when that has happened, and re-inserts a dummy counter-increment | ||
| /// statement so that LLVM knows to treat the function as instrumented. |
There was a problem hiding this comment.
I wonder if this should be done by codegen directly. When starting codegen for a mono-item, check if function_coverage_info is set, and add this logic?
There was a problem hiding this comment.
Yeah, that sounds reasonable. I'll have to track down the right place to add a hook, but once I do the result should be more self-contained in codegen.
This test is mainly for detecting problems triggered by MIR optimizations, but the run-coverage tests don't enable optimization by default. (The coverage-map copy has also been updated, to keep the two tests in sync.)
…ters If a function has been instrumented for coverage, but MIR optimizations subsequently remove all of its counter-increment statements, then we won't emit LLVM counter-increment intrinsics. LLVM will think the function is not instrumented, and it will disappear from coverage mappings and coverage reports. This new MIR pass detects when that has happened, and re-inserts a dummy counter-increment statement so that LLVM knows to treat the function as instrumented.
Now that coverage has a dedicated MIR pass to detect and repair instrumented functions that have lost all their counters, it should be OK to permit this pass in coverage builds.
|
Hmm, I was investigating this some more, and found that I couldn't get my tests to fail in the way they were failing before. That means I can't be confident that my fix is working, or even necessary. I'll put this on hold until I figure out what's going on. @rustbot author |
rustbot
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #117484) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@Zalathar any updates on this? Thanks |
|
@Dylan-DPC This has been a low priority for me, so no updates unfortunately. |
|
I found a much nicer way to do this:
|
|
Closing in favour of #122860. |
Follow-up to this pass being disabled for coverage in #116166; fixes #116171.
This optimization pass can potentially remove all counter-increment statements from an instrumented function. Previously that would cause it to disappear from coverage reports, so the pass had to be turned off during coverage builds.
This PR introduces a more robust solution: we add an additional post-optimization MIR pass that detects when an instrumented function has lost all of its counter-increment statements, and simply adds another counter-increment statement. That allows coverage codegen to emit the function's coverage mappings as normal.