[clang] f5cd181 - [Coverage] Introduce `getBranchCounterPair()`. NFC. (#112702)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 8 23:47:05 PST 2025
Author: NAKAMURA Takumi
Date: 2025-01-09T16:47:01+09:00
New Revision: f5cd181ffbb7cb61d582fe130d46580d5969d47a
URL: https://github.com/llvm/llvm-project/commit/f5cd181ffbb7cb61d582fe130d46580d5969d47a
DIFF: https://github.com/llvm/llvm-project/commit/f5cd181ffbb7cb61d582fe130d46580d5969d47a.diff
LOG: [Coverage] Introduce `getBranchCounterPair()`. NFC. (#112702)
This aggregates the generation of branch counter pair as `ExecCnt` and
`SkipCnt`, to aggregate `CounterExpr::subtract`. At the moment:
- This change preserves the behavior of
`llvm::EnableSingleByteCoverage`. Almost of SingleByteCoverage will be
cleaned up by coming commits.
- `IsCounterEqual(Out, Par)` is introduced instead of
`Counter::operator==`. Tweaks would be required for the comparison for
additional counters.
https://discourse.llvm.org/t/rfc-integrating-singlebytecoverage-with-branch-coverage/82492
Added:
Modified:
clang/lib/CodeGen/CoverageMappingGen.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index cda218eac34af8..dfffa12b639f24 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -919,15 +919,11 @@ struct CounterCoverageMappingBuilder
/// Return a counter for the sum of \c LHS and \c RHS.
Counter addCounters(Counter LHS, Counter RHS, bool Simplify = true) {
- assert(!llvm::EnableSingleByteCoverage &&
- "cannot add counters when single byte coverage mode is enabled");
return Builder.add(LHS, RHS, Simplify);
}
Counter addCounters(Counter C1, Counter C2, Counter C3,
bool Simplify = true) {
- assert(!llvm::EnableSingleByteCoverage &&
- "cannot add counters when single byte coverage mode is enabled");
return addCounters(addCounters(C1, C2, Simplify), C3, Simplify);
}
@@ -938,6 +934,50 @@ struct CounterCoverageMappingBuilder
return Counter::getCounter(CounterMap[S]);
}
+ struct BranchCounterPair {
+ Counter Executed; ///< The Counter previously assigned.
+ Counter Skipped; ///< An expression (Parent-Executed), or equivalent to it.
+ };
+
+ /// Retrieve or assign the pair of Counter(s).
+ ///
+ /// This returns BranchCounterPair {Executed, Skipped}.
+ /// Executed is the Counter associated with S assigned by an earlier
+ /// CounterMapping pass.
+ /// Skipped may be an expression (Executed - ParentCnt) or newly
+ /// assigned Counter in EnableSingleByteCoverage, as subtract
+ /// expressions are not available in this mode.
+ ///
+ /// \param S Key to the CounterMap
+ /// \param ParentCnt The Counter representing how many times S is evaluated.
+ /// \param SkipCntForOld (To be removed later) Optional fake Counter
+ /// to override Skipped for adjustment of
+ /// expressions in the old behavior of
+ /// EnableSingleByteCoverage that is unaware of
+ /// Branch coverage.
+ BranchCounterPair
+ getBranchCounterPair(const Stmt *S, Counter ParentCnt,
+ std::optional<Counter> SkipCntForOld = std::nullopt) {
+ Counter ExecCnt = getRegionCounter(S);
+
+ // The old behavior of SingleByte is unaware of Branches.
+ // Will be pruned after the migration of SingleByte.
+ if (llvm::EnableSingleByteCoverage) {
+ assert(SkipCntForOld &&
+ "SingleByte must provide SkipCntForOld as a fake Skipped count.");
+ return {ExecCnt, *SkipCntForOld};
+ }
+
+ return {ExecCnt, Builder.subtract(ParentCnt, ExecCnt)};
+ }
+
+ bool IsCounterEqual(Counter OutCount, Counter ParentCount) {
+ if (OutCount == ParentCount)
+ return true;
+
+ return false;
+ }
+
/// Push a region onto the stack.
///
/// Returns the index on the stack where the region was pushed. This can be
@@ -1588,6 +1628,10 @@ struct CounterCoverageMappingBuilder
llvm::EnableSingleByteCoverage
? getRegionCounter(S->getCond())
: addCounters(ParentCount, BackedgeCount, BC.ContinueCount);
+ auto BranchCount = getBranchCounterPair(S, CondCount, getRegionCounter(S));
+ assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount ||
+ llvm::EnableSingleByteCoverage);
+
propagateCounts(CondCount, S->getCond());
adjustForOutOfOrderTraversal(getEnd(S));
@@ -1596,13 +1640,11 @@ struct CounterCoverageMappingBuilder
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount);
- Counter OutCount =
- llvm::EnableSingleByteCoverage
- ? getRegionCounter(S)
- : addCounters(BC.BreakCount,
- subtractCounters(CondCount, BodyCount));
-
- if (OutCount != ParentCount) {
+ assert(
+ !llvm::EnableSingleByteCoverage ||
+ (BC.BreakCount.isZero() && BranchCount.Skipped == getRegionCounter(S)));
+ Counter OutCount = addCounters(BC.BreakCount, BranchCount.Skipped);
+ if (!IsCounterEqual(OutCount, ParentCount)) {
pushRegion(OutCount);
GapRegionCounter = OutCount;
if (BodyHasTerminateStmt)
@@ -1611,8 +1653,7 @@ struct CounterCoverageMappingBuilder
// Create Branch Region around condition.
if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(S->getCond(), BodyCount,
- subtractCounters(CondCount, BodyCount));
+ createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped);
}
void VisitDoStmt(const DoStmt *S) {
@@ -1641,22 +1682,24 @@ struct CounterCoverageMappingBuilder
Counter CondCount = llvm::EnableSingleByteCoverage
? getRegionCounter(S->getCond())
: addCounters(BackedgeCount, BC.ContinueCount);
+ auto BranchCount = getBranchCounterPair(S, CondCount, getRegionCounter(S));
+ assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount ||
+ llvm::EnableSingleByteCoverage);
+
propagateCounts(CondCount, S->getCond());
- Counter OutCount =
- llvm::EnableSingleByteCoverage
- ? getRegionCounter(S)
- : addCounters(BC.BreakCount,
- subtractCounters(CondCount, BodyCount));
- if (OutCount != ParentCount) {
+ assert(
+ !llvm::EnableSingleByteCoverage ||
+ (BC.BreakCount.isZero() && BranchCount.Skipped == getRegionCounter(S)));
+ Counter OutCount = addCounters(BC.BreakCount, BranchCount.Skipped);
+ if (!IsCounterEqual(OutCount, ParentCount)) {
pushRegion(OutCount);
GapRegionCounter = OutCount;
}
// Create Branch Region around condition.
if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(S->getCond(), BodyCount,
- subtractCounters(CondCount, BodyCount));
+ createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped);
if (BodyHasTerminateStmt)
HasTerminateStmt = true;
@@ -1705,6 +1748,9 @@ struct CounterCoverageMappingBuilder
: addCounters(
addCounters(ParentCount, BackedgeCount, BodyBC.ContinueCount),
IncrementBC.ContinueCount);
+ auto BranchCount = getBranchCounterPair(S, CondCount, getRegionCounter(S));
+ assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount ||
+ llvm::EnableSingleByteCoverage);
if (const Expr *Cond = S->getCond()) {
propagateCounts(CondCount, Cond);
@@ -1716,12 +1762,11 @@ struct CounterCoverageMappingBuilder
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount);
- Counter OutCount =
- llvm::EnableSingleByteCoverage
- ? getRegionCounter(S)
- : addCounters(BodyBC.BreakCount, IncrementBC.BreakCount,
- subtractCounters(CondCount, BodyCount));
- if (OutCount != ParentCount) {
+ assert(!llvm::EnableSingleByteCoverage ||
+ (BodyBC.BreakCount.isZero() && IncrementBC.BreakCount.isZero()));
+ Counter OutCount = addCounters(BodyBC.BreakCount, IncrementBC.BreakCount,
+ BranchCount.Skipped);
+ if (!IsCounterEqual(OutCount, ParentCount)) {
pushRegion(OutCount);
GapRegionCounter = OutCount;
if (BodyHasTerminateStmt)
@@ -1730,8 +1775,7 @@ struct CounterCoverageMappingBuilder
// Create Branch Region around condition.
if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(S->getCond(), BodyCount,
- subtractCounters(CondCount, BodyCount));
+ createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped);
}
void VisitCXXForRangeStmt(const CXXForRangeStmt *S) {
@@ -1759,16 +1803,17 @@ struct CounterCoverageMappingBuilder
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount);
- Counter OutCount;
- Counter LoopCount;
- if (llvm::EnableSingleByteCoverage)
- OutCount = getRegionCounter(S);
- else {
- LoopCount = addCounters(ParentCount, BackedgeCount, BC.ContinueCount);
- OutCount =
- addCounters(BC.BreakCount, subtractCounters(LoopCount, BodyCount));
- }
- if (OutCount != ParentCount) {
+ Counter LoopCount =
+ addCounters(ParentCount, BackedgeCount, BC.ContinueCount);
+ auto BranchCount = getBranchCounterPair(S, LoopCount, getRegionCounter(S));
+ assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount ||
+ llvm::EnableSingleByteCoverage);
+ assert(
+ !llvm::EnableSingleByteCoverage ||
+ (BC.BreakCount.isZero() && BranchCount.Skipped == getRegionCounter(S)));
+
+ Counter OutCount = addCounters(BC.BreakCount, BranchCount.Skipped);
+ if (!IsCounterEqual(OutCount, ParentCount)) {
pushRegion(OutCount);
GapRegionCounter = OutCount;
if (BodyHasTerminateStmt)
@@ -1777,8 +1822,7 @@ struct CounterCoverageMappingBuilder
// Create Branch Region around condition.
if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(S->getCond(), BodyCount,
- subtractCounters(LoopCount, BodyCount));
+ createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped);
}
void VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S) {
@@ -1800,9 +1844,10 @@ struct CounterCoverageMappingBuilder
Counter LoopCount =
addCounters(ParentCount, BackedgeCount, BC.ContinueCount);
- Counter OutCount =
- addCounters(BC.BreakCount, subtractCounters(LoopCount, BodyCount));
- if (OutCount != ParentCount) {
+ auto BranchCount = getBranchCounterPair(S, LoopCount);
+ assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount);
+ Counter OutCount = addCounters(BC.BreakCount, BranchCount.Skipped);
+ if (!IsCounterEqual(OutCount, ParentCount)) {
pushRegion(OutCount);
GapRegionCounter = OutCount;
}
@@ -2010,9 +2055,12 @@ struct CounterCoverageMappingBuilder
extendRegion(S->getCond());
Counter ParentCount = getRegion().getCounter();
- Counter ThenCount = llvm::EnableSingleByteCoverage
- ? getRegionCounter(S->getThen())
- : getRegionCounter(S);
+ auto [ThenCount, ElseCount] =
+ (llvm::EnableSingleByteCoverage
+ ? BranchCounterPair{getRegionCounter(S->getThen()),
+ (S->getElse() ? getRegionCounter(S->getElse())
+ : Counter::getZero())}
+ : getBranchCounterPair(S, ParentCount));
// Emitting a counter for the condition makes it easier to interpret the
// counter for the body when looking at the coverage.
@@ -2027,12 +2075,6 @@ struct CounterCoverageMappingBuilder
extendRegion(S->getThen());
Counter OutCount = propagateCounts(ThenCount, S->getThen());
- Counter ElseCount;
- if (!llvm::EnableSingleByteCoverage)
- ElseCount = subtractCounters(ParentCount, ThenCount);
- else if (S->getElse())
- ElseCount = getRegionCounter(S->getElse());
-
if (const Stmt *Else = S->getElse()) {
bool ThenHasTerminateStmt = HasTerminateStmt;
HasTerminateStmt = false;
@@ -2055,15 +2097,14 @@ struct CounterCoverageMappingBuilder
if (llvm::EnableSingleByteCoverage)
OutCount = getRegionCounter(S);
- if (OutCount != ParentCount) {
+ if (!IsCounterEqual(OutCount, ParentCount)) {
pushRegion(OutCount);
GapRegionCounter = OutCount;
}
if (!llvm::EnableSingleByteCoverage)
// Create Branch Region around condition.
- createBranchRegion(S->getCond(), ThenCount,
- subtractCounters(ParentCount, ThenCount));
+ createBranchRegion(S->getCond(), ThenCount, ElseCount);
}
void VisitCXXTryStmt(const CXXTryStmt *S) {
@@ -2089,9 +2130,11 @@ struct CounterCoverageMappingBuilder
extendRegion(E);
Counter ParentCount = getRegion().getCounter();
- Counter TrueCount = llvm::EnableSingleByteCoverage
- ? getRegionCounter(E->getTrueExpr())
- : getRegionCounter(E);
+ auto [TrueCount, FalseCount] =
+ (llvm::EnableSingleByteCoverage
+ ? BranchCounterPair{getRegionCounter(E->getTrueExpr()),
+ getRegionCounter(E->getFalseExpr())}
+ : getBranchCounterPair(E, ParentCount));
Counter OutCount;
if (const auto *BCO = dyn_cast<BinaryConditionalOperator>(E)) {
@@ -2110,25 +2153,20 @@ struct CounterCoverageMappingBuilder
}
extendRegion(E->getFalseExpr());
- Counter FalseCount = llvm::EnableSingleByteCoverage
- ? getRegionCounter(E->getFalseExpr())
- : subtractCounters(ParentCount, TrueCount);
-
Counter FalseOutCount = propagateCounts(FalseCount, E->getFalseExpr());
if (llvm::EnableSingleByteCoverage)
OutCount = getRegionCounter(E);
else
OutCount = addCounters(OutCount, FalseOutCount);
- if (OutCount != ParentCount) {
+ if (!IsCounterEqual(OutCount, ParentCount)) {
pushRegion(OutCount);
GapRegionCounter = OutCount;
}
// Create Branch Region around condition.
if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(E->getCond(), TrueCount,
- subtractCounters(ParentCount, TrueCount));
+ createBranchRegion(E->getCond(), TrueCount, FalseCount);
}
void createOrCancelDecision(const BinaryOperator *E, unsigned Since) {
@@ -2227,27 +2265,27 @@ struct CounterCoverageMappingBuilder
extendRegion(E->getRHS());
propagateCounts(getRegionCounter(E), E->getRHS());
+ if (llvm::EnableSingleByteCoverage)
+ return;
+
// Track RHS True/False Decision.
const auto DecisionRHS = MCDCBuilder.back();
+ // Extract the Parent Region Counter.
+ Counter ParentCnt = getRegion().getCounter();
+
// Extract the RHS's Execution Counter.
- Counter RHSExecCnt = getRegionCounter(E);
+ auto [RHSExecCnt, LHSExitCnt] = getBranchCounterPair(E, ParentCnt);
// Extract the RHS's "True" Instance Counter.
- Counter RHSTrueCnt = getRegionCounter(E->getRHS());
-
- // Extract the Parent Region Counter.
- Counter ParentCnt = getRegion().getCounter();
+ auto [RHSTrueCnt, RHSExitCnt] =
+ getBranchCounterPair(E->getRHS(), RHSExecCnt);
// Create Branch Region around LHS condition.
- if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(E->getLHS(), RHSExecCnt,
- subtractCounters(ParentCnt, RHSExecCnt), DecisionLHS);
+ createBranchRegion(E->getLHS(), RHSExecCnt, LHSExitCnt, DecisionLHS);
// Create Branch Region around RHS condition.
- if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(E->getRHS(), RHSTrueCnt,
- subtractCounters(RHSExecCnt, RHSTrueCnt), DecisionRHS);
+ createBranchRegion(E->getRHS(), RHSTrueCnt, RHSExitCnt, DecisionRHS);
// Create MCDC Decision Region if at top-level (root).
if (IsRootNode)
@@ -2288,31 +2326,31 @@ struct CounterCoverageMappingBuilder
extendRegion(E->getRHS());
propagateCounts(getRegionCounter(E), E->getRHS());
+ if (llvm::EnableSingleByteCoverage)
+ return;
+
// Track RHS True/False Decision.
const auto DecisionRHS = MCDCBuilder.back();
+ // Extract the Parent Region Counter.
+ Counter ParentCnt = getRegion().getCounter();
+
// Extract the RHS's Execution Counter.
- Counter RHSExecCnt = getRegionCounter(E);
+ auto [RHSExecCnt, LHSExitCnt] = getBranchCounterPair(E, ParentCnt);
// Extract the RHS's "False" Instance Counter.
- Counter RHSFalseCnt = getRegionCounter(E->getRHS());
+ auto [RHSFalseCnt, RHSExitCnt] =
+ getBranchCounterPair(E->getRHS(), RHSExecCnt);
if (!shouldVisitRHS(E->getLHS())) {
GapRegionCounter = OutCount;
}
- // Extract the Parent Region Counter.
- Counter ParentCnt = getRegion().getCounter();
-
// Create Branch Region around LHS condition.
- if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(E->getLHS(), subtractCounters(ParentCnt, RHSExecCnt),
- RHSExecCnt, DecisionLHS);
+ createBranchRegion(E->getLHS(), LHSExitCnt, RHSExecCnt, DecisionLHS);
// Create Branch Region around RHS condition.
- if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(E->getRHS(), subtractCounters(RHSExecCnt, RHSFalseCnt),
- RHSFalseCnt, DecisionRHS);
+ createBranchRegion(E->getRHS(), RHSExitCnt, RHSFalseCnt, DecisionRHS);
// Create MCDC Decision Region if at top-level (root).
if (IsRootNode)
More information about the cfe-commits
mailing list