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) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
