[Coverage][MCDC] Adapt mcdc to llvm 19 #126733

ZhuUx · 2024-06-20T11:28:47Z

Related issue: #126672

Also finish task 4 at #124144

llvm #82448 has introduced some break changes into mcdc, causing incompatibility between llvm 18 and 19. This draft adapts to that change and gives up supporting for llvm-18.

rustbot · 2024-06-20T11:28:55Z

r? @compiler-errors

rustbot has assigned @compiler-errors.
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

ZhuUx · 2024-06-20T11:29:06Z

@rustbot label +A-code-coverage

ZhuUx · 2024-06-20T11:35:53Z

It is still unfinished. I would add some options next days and try to warn about mcdc limits with lints as this comment suggests.

@rustbot author

ZhuUx · 2024-06-21T03:32:53Z

I think we need to discuss more about mcdc options. Thus now I remove the commit and just reset max_conditions_in_decision to i16::MAX as default of clang.

Nevertheless I would wait until rustc is going to update in-tree llvm 19 and see if there are any other changes need to make.

bors · 2024-06-27T04:22:00Z

☔ The latest upstream changes (presumably #123918) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2024-07-05T14:12:52Z

☔ The latest upstream changes (presumably #127360) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2024-07-25T08:13:57Z

Some changes occurred in coverage tests.

cc @Zalathar

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in coverage instrumentation.

cc @Zalathar

Some changes occurred in coverage instrumentation.

cc @Zalathar

Some changes occurred in coverage instrumentation.

cc @Zalathar

Some changes occurred in coverage instrumentation.

cc @Zalathar

ZhuUx · 2024-07-25T08:19:15Z

Now this pr is independent from #126677 . We can focus on this only.
@Zalathar @compiler-errors @nikic
It's prepared for reviewing.
@rustbot review

ZhuUx · 2024-07-25T08:25:06Z

compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs

-    fn take_condition(
+    fn try_finish_decision(
        &mut self,
+        span: Span,
        true_marker: BlockMarkerId,
        false_marker: BlockMarkerId,
-    ) -> (Option<ConditionInfo>, Option<MCDCDecisionSpan>) {


This change looks a bit awkward. It's just a temporary expedient and will be removed in refactored mcdc implementation for pattern matching.

bors · 2024-09-23T04:57:32Z

☔ The latest upstream changes (presumably #130724) made this pull request unmergeable. Please resolve the merge conflicts.

Zalathar · 2024-10-08T02:32:01Z

This needs another rebase because #131325 replaced the bcb_counter method with term_for_bcb.

Also please squish the “fixup” changes into the other patches as appropriate.

@rustbot author

Currently these tests only run against LLVM 18, but the default is now LLVM 19, which makes them very tricky to bless. Since rust-lang#126733 is going to drop support for MC/DC in LLVM 18 anyway, it's easier to just completely disable the tests until that PR fixes them.

Zalathar · 2024-10-08T03:38:40Z

@bors r+

bors · 2024-10-08T03:38:42Z

📌 Commit acd64fa has been approved by Zalathar

It is now in the queue for this repository.

bors · 2024-10-08T07:08:45Z

⌛ Testing commit acd64fa with merge cf24c73...

bors · 2024-10-08T10:06:36Z

☀️ Test successful - checks-actions
Approved by: Zalathar
Pushing cf24c73 to master...

rust-timer · 2024-10-08T11:27:27Z

Finished benchmarking commit (cf24c73): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.0%	[3.0%, 3.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary -2.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.6%	[-2.6%, -2.6%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -3.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.5%	[-7.9%, -1.4%]	12
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 775.416s -> 775.01s (-0.05%)
Artifact size: 329.55 MiB -> 329.57 MiB (0.01%)

coverage: Remove all unstable support for MC/DC instrumentation Preliminary support for a partial implementation of “Modified Condition/Decision Coverage” instrumentation was added behind the unstable flag `-Zcoverage-options=mcdc` in 2024. These are the most substantial PRs involved: - rust-lang#123409 - rust-lang#126733 At the time, I accepted these PRs with relatively modest scrutiny, because I did not want to stand in the way of independent work on MC/DC instrumentation. My hope was that ongoing work by interested contributors would lead to the code becoming clearer and more maintainable over time. --- However, that MC/DC code has proven itself to be a major burden on overall maintenance of coverage instrumentation, and a major obstacle to other planned improvements, such as internal changes needed for proper support of macro expansion regions. I have also become reluctant to accept any further MC/DC-related changes that would increase this burden. That tension has resulted in an unhappy impasse. On one hand, the present MC/DC implementation is not yet complete, and shows little sign of being complete at an acceptable level of code quality in the foreseeable future. On the other hand, the continued existence of this partial MC/DC implementation is imposing serious maintenance burdens on every other aspect of coverage instrumentation, and is preventing some of the very improvements that would make it easier to accept expanded MC/DC support in the future. While I know this will be disappointing to some, I think the healthy way forward is accept that I made the wrong call in accepting the current implementation, and to remove it entirely from the compiler.

Rollup merge of #144999 - Zalathar:remove-mcdc, r=oli-obk coverage: Remove all unstable support for MC/DC instrumentation Preliminary support for a partial implementation of “Modified Condition/Decision Coverage” instrumentation was added behind the unstable flag `-Zcoverage-options=mcdc` in 2024. These are the most substantial PRs involved: - #123409 - #126733 At the time, I accepted these PRs with relatively modest scrutiny, because I did not want to stand in the way of independent work on MC/DC instrumentation. My hope was that ongoing work by interested contributors would lead to the code becoming clearer and more maintainable over time. --- However, that MC/DC code has proven itself to be a major burden on overall maintenance of coverage instrumentation, and a major obstacle to other planned improvements, such as internal changes needed for proper support of macro expansion regions. I have also become reluctant to accept any further MC/DC-related changes that would increase this burden. That tension has resulted in an unhappy impasse. On one hand, the present MC/DC implementation is not yet complete, and shows little sign of being complete at an acceptable level of code quality in the foreseeable future. On the other hand, the continued existence of this partial MC/DC implementation is imposing serious maintenance burdens on every other aspect of coverage instrumentation, and is preventing some of the very improvements that would make it easier to accept expanded MC/DC support in the future. While I know this will be disappointing to some, I think the healthy way forward is accept that I made the wrong call in accepting the current implementation, and to remove it entirely from the compiler.

coverage: Remove all unstable support for MC/DC instrumentation Preliminary support for a partial implementation of “Modified Condition/Decision Coverage” instrumentation was added behind the unstable flag `-Zcoverage-options=mcdc` in 2024. These are the most substantial PRs involved: - rust-lang/rust#123409 - rust-lang/rust#126733 At the time, I accepted these PRs with relatively modest scrutiny, because I did not want to stand in the way of independent work on MC/DC instrumentation. My hope was that ongoing work by interested contributors would lead to the code becoming clearer and more maintainable over time. --- However, that MC/DC code has proven itself to be a major burden on overall maintenance of coverage instrumentation, and a major obstacle to other planned improvements, such as internal changes needed for proper support of macro expansion regions. I have also become reluctant to accept any further MC/DC-related changes that would increase this burden. That tension has resulted in an unhappy impasse. On one hand, the present MC/DC implementation is not yet complete, and shows little sign of being complete at an acceptable level of code quality in the foreseeable future. On the other hand, the continued existence of this partial MC/DC implementation is imposing serious maintenance burdens on every other aspect of coverage instrumentation, and is preventing some of the very improvements that would make it easier to accept expanded MC/DC support in the future. While I know this will be disappointing to some, I think the healthy way forward is accept that I made the wrong call in accepting the current implementation, and to remove it entirely from the compiler.

rustbot assigned compiler-errors Jun 20, 2024

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 Jun 20, 2024

rustbot added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Jun 20, 2024

ZhuUx mentioned this pull request Jun 20, 2024

MC/DC LLVM 19 intrinsics changes #126672

Open

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 20, 2024

ZhuUx mentioned this pull request Jun 20, 2024

Tracking implementation for MC/DC #124144

Open

4 tasks

ZhuUx force-pushed the llvm-19-adapt branch from b268271 to 83e745f Compare June 21, 2024 03:06

ZhuUx force-pushed the llvm-19-adapt branch from 83e745f to 2a68c4d Compare July 3, 2024 08:38

ZhuUx mentioned this pull request Jul 10, 2024

[Coverage][MCDC] Prepare MCDC Builder for pattern matching and llvm 19 #126677

Closed

ZhuUx force-pushed the llvm-19-adapt branch 2 times, most recently from 1a8d158 to b09e96e Compare July 10, 2024 05:40

ZhuUx mentioned this pull request Jul 10, 2024

Support mcdc analysis for pattern matching #124278

Open

ZhuUx force-pushed the llvm-19-adapt branch from b09e96e to 20bdcb2 Compare July 15, 2024 03:51

ZhuUx force-pushed the llvm-19-adapt branch from 20bdcb2 to 664be8c Compare July 25, 2024 08:12

ZhuUx marked this pull request as ready for review July 25, 2024 08:13

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 25, 2024

ZhuUx commented Jul 25, 2024

View reviewed changes

ZhuUx force-pushed the llvm-19-adapt branch from 664be8c to c8d8958 Compare July 25, 2024 08:32

ZhuUx force-pushed the llvm-19-adapt branch from 3bc3f75 to 639d83e Compare September 19, 2024 01:45

This comment has been minimized.

Sign in to view

ZhuUx force-pushed the llvm-19-adapt branch from 639d83e to fb8757d Compare September 19, 2024 08:23

ZhuUx force-pushed the llvm-19-adapt branch from fb8757d to bddf980 Compare September 23, 2024 06:20

jieyouxu mentioned this pull request Sep 26, 2024

Stabilize #[coverage] attribute #130766

Merged

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 8, 2024

Zalathar mentioned this pull request Oct 8, 2024

coverage: Disable the zombie MC/DC tests until they support LLVM 19 #131388

Closed

zhuyunxing added 4 commits October 8, 2024 10:50

coverage. Disable supporting mcdc on llvm-18

911ac56

coverage. MCDC ConditionId start from 0 to keep with llvm 19

99bd601

coverage. Adapt to mcdc mapping formats introduced by llvm 19

6e3e19f

coverage. Warn about too many test vectors

acd64fa

ZhuUx force-pushed the llvm-19-adapt branch from bddf980 to acd64fa Compare October 8, 2024 03:22

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 8, 2024

bors merged commit cf24c73 into rust-lang:master Oct 8, 2024
7 checks passed

rustbot added this to the 1.83.0 milestone Oct 8, 2024

Zalathar mentioned this pull request Aug 6, 2025

coverage: Remove all unstable support for MC/DC instrumentation #144999

Merged

[Coverage][MCDC] Adapt mcdc to llvm 19 #126733

[Coverage][MCDC] Adapt mcdc to llvm 19 #126733

Uh oh!

Conversation

ZhuUx commented Jun 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 20, 2024

Uh oh!

ZhuUx commented Jun 20, 2024

Uh oh!

This comment has been minimized.

ZhuUx commented Jun 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZhuUx commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Jun 27, 2024

Uh oh!

bors commented Jul 5, 2024

Uh oh!

rustbot commented Jul 25, 2024

Uh oh!

ZhuUx commented Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZhuUx Jul 25, 2024

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

bors commented Sep 23, 2024

Uh oh!

Zalathar commented Oct 8, 2024

Uh oh!

Zalathar commented Oct 8, 2024

Uh oh!

bors commented Oct 8, 2024

Uh oh!

bors commented Oct 8, 2024

Uh oh!

bors commented Oct 8, 2024

Uh oh!

Uh oh!

rust-timer commented Oct 8, 2024

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Uh oh!

ZhuUx commented Jun 20, 2024 •

edited

Loading

ZhuUx commented Jun 20, 2024 •

edited

Loading

ZhuUx commented Jun 21, 2024 •

edited

Loading

ZhuUx commented Jul 25, 2024 •

edited

Loading