[llvm-cov] Simplify and optimize MC/DC computation #79727
Conversation
Created using spr 1.3.4
|
@MaskRay I'm trying to replace this for another purpose, to handle TestVectors also to clangCodeGen. |
|
It is |
| for (unsigned J = 0; J < I; ++J) { | ||
| const MCDCRecord::TestVector &B = ExecVectors[J]; | ||
| // Enumerate two execution vectors whose outcomes are different. | ||
| if (A[NumConditions] == B[NumConditions]) | ||
| continue; | ||
| unsigned Flip = NumConditions, Idx; | ||
| for (Idx = 0; Idx < NumConditions; ++Idx) { | ||
| MCDCRecord::CondState ACond = A[Idx], BCond = B[Idx]; | ||
| if (ACond == BCond || ACond == MCDCRecord::MCDC_DontCare || | ||
| BCond == MCDCRecord::MCDC_DontCare) | ||
| continue; | ||
|
|
||
| // If a matching pair of vectors is found, record them. | ||
| if ((PairFound = matchTestVectors(I, J, C))) | ||
| IndependencePairs[C] = std::make_pair(I + 1, J + 1); | ||
| if (Flip != NumConditions) | ||
| break; | ||
| Flip = Idx; | ||
| } |
There was a problem hiding this comment.
Could we simplify more this if we could introduce TestVector stuff as the set of bitvectors?
- Pair of bitvector
- "Visited" (Do care) and "True/False map"
- "True" map and "False" map (D/C should be zero both)
- Array of two bits to represent three states.
There was a problem hiding this comment.
Due to the presence of MCDC_DontCare, optimizing this further requires some dirty bit twiddling, which may make the code less readable.
Given that NumConditions is capped by 6, and |TestVectors| is capped by 6+1, the current time complexity should be acceptable.
There was a problem hiding this comment.
FYI I am planning to relax 6 to thousands.
There was a problem hiding this comment.
Thanks for the help. I could understand perhaps extending beyond six, but why thousands? Achieving full MC/DC is not easy even with six, and most embedded cases would have no more than maybe 3 or 4 conditions. I'm concerned about overengineering.
There was a problem hiding this comment.
I have studied MC/DC more and made some notes on https://maskray.me/blog/2024-01-28-mc-dc-and-compiler-implementations
GCC has a pending patch implementing MC/DC as well and they apply an algorithm described by Efficient Test Coverage Measurement for MC/DC, which is linear in terms of the number of conditions.
You mean construct the list of possible test vectors in clang? That would be useful. |
Planning let
"Thousands" is not my goal but my expected capacity. Not measured yet. I am doing it since it is very effective to apply it to the large codebase, LLVM itself, since we could keep it stable and preventing bitrot. I like such a dogfooding. :) Thank you. |
There was a problem hiding this comment.
Looks great from my side.
@evodius96 Could you take a look, too?
| /// Find all possible Independence Pairs for a boolean expression given its | ||
| /// executed Test Vectors. This process involves looking at each condition | ||
| /// and attempting to find two Test Vectors that "match", giving us a pair. | ||
| // Find an independence pair for each condition. |
There was a problem hiding this comment.
Can you add a little more commentary on what constitutes an independence pair, i.e. what the algorithm is looking for -- I had a comment around matchTestVectors() about this. I think it improves readability a bit more.
There was a problem hiding this comment.
The two comments in the function body complement this one, but I can place the three conditions together...
| @@ -458,7 +395,7 @@ class MCDCRecordProcessor { | |||
| MCDCRecord::TestVector TV(NumConditions, MCDCRecord::MCDC_DontCare); | |||
|
|
|||
| // Use the base test vector to build the list of all possible test vectors. | |||
| buildTestVector(TV); | |||
| buildTestVector(TV, 1, 0); | |||
There was a problem hiding this comment.
Minor nit: I wonder if it would be of some value to include a very brief comment that explains what the initial values mean? Up to you.
There was a problem hiding this comment.
Thanks for the suggestion. Added
… Measurement for MC/DC) Created using spr 1.3.4
|
Added a couple of comments, but otherwise LGTM. Thank you for optimizing this. |
MaskRay
deleted the
users/MaskRay/spr/llvm-cov-simplify-and-optimize-mcdc-computation
branch
Update code from https://reviews.llvm.org/D138847
buildTestVectoris a standard DFS (walking a reduced ordered binarydecision diagram). Avoid shouldCopyOffTestVectorFor{True,False}Path
complexity and redundant
Map[ID]lookups.findIndependencePairsunnecessarily uses four nested loops (n<=6) tofind independence pairs. Instead, enumerate the two execution vectors
and find the number of mismatches. This algorithm can be optimized using
the marking function technique described in Efficient Test Coverage
Measurement for MC/DC, 2013, but this may be overkill.