https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/112702
>From fc697f04fd6c9f3c217ce04e3f1dd082c1f1a705 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Wed, 16 Oct 2024 23:16:53 +0900 Subject: [PATCH 1/2] [Coverage] Introduce `getBranchCounterPair()`. NFC. 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. - `getBranchCounterPair()` is not called in `llvm::EnableSingleByteCoverage`. I will implement the new behavior of SingleByteCoverage in it. - `IsCounterEqual(Out, Par)` is introduced instead of `Counter::operator==`. Tweaks would be required for the comparison for additional counters. - Braces around `assert()` is intentional. I will add a statement there. https://discourse.llvm.org/t/rfc-integrating-singlebytecoverage-with-branch-coverage/82492 --- clang/lib/CodeGen/CoverageMappingGen.cpp | 177 +++++++++++++---------- 1 file changed, 102 insertions(+), 75 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 07015834bc84f3..0bfad9cbcbe12b 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -941,6 +941,19 @@ struct CounterCoverageMappingBuilder return Counter::getCounter(CounterMap[S]); } + std::pair<Counter, Counter> getBranchCounterPair(const Stmt *S, + Counter ParentCnt) { + Counter ExecCnt = getRegionCounter(S); + 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 @@ -1592,6 +1605,13 @@ struct CounterCoverageMappingBuilder llvm::EnableSingleByteCoverage ? getRegionCounter(S->getCond()) : addCounters(ParentCount, BackedgeCount, BC.ContinueCount); + auto [ExecCount, ExitCount] = + (llvm::EnableSingleByteCoverage + ? std::make_pair(getRegionCounter(S), Counter::getZero()) + : getBranchCounterPair(S, CondCount)); + if (!llvm::EnableSingleByteCoverage) { + assert(ExecCount.isZero() || ExecCount == BodyCount); + } propagateCounts(CondCount, S->getCond()); adjustForOutOfOrderTraversal(getEnd(S)); @@ -1600,13 +1620,11 @@ struct CounterCoverageMappingBuilder if (Gap) fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount); - Counter OutCount = - llvm::EnableSingleByteCoverage - ? getRegionCounter(S) - : addCounters(BC.BreakCount, - subtractCounters(CondCount, BodyCount)); + Counter OutCount = llvm::EnableSingleByteCoverage + ? getRegionCounter(S) + : addCounters(BC.BreakCount, ExitCount); - if (OutCount != ParentCount) { + if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; if (BodyHasTerminateStmt) @@ -1615,8 +1633,7 @@ struct CounterCoverageMappingBuilder // Create Branch Region around condition. if (!llvm::EnableSingleByteCoverage) - createBranchRegion(S->getCond(), BodyCount, - subtractCounters(CondCount, BodyCount)); + createBranchRegion(S->getCond(), BodyCount, ExitCount); } void VisitDoStmt(const DoStmt *S) { @@ -1645,22 +1662,26 @@ struct CounterCoverageMappingBuilder Counter CondCount = llvm::EnableSingleByteCoverage ? getRegionCounter(S->getCond()) : addCounters(BackedgeCount, BC.ContinueCount); + auto [ExecCount, ExitCount] = + (llvm::EnableSingleByteCoverage + ? std::make_pair(getRegionCounter(S), Counter::getZero()) + : getBranchCounterPair(S, CondCount)); + if (!llvm::EnableSingleByteCoverage) { + assert(ExecCount.isZero() || ExecCount == BodyCount); + } propagateCounts(CondCount, S->getCond()); - Counter OutCount = - llvm::EnableSingleByteCoverage - ? getRegionCounter(S) - : addCounters(BC.BreakCount, - subtractCounters(CondCount, BodyCount)); - if (OutCount != ParentCount) { + Counter OutCount = llvm::EnableSingleByteCoverage + ? getRegionCounter(S) + : addCounters(BC.BreakCount, ExitCount); + 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, ExitCount); if (BodyHasTerminateStmt) HasTerminateStmt = true; @@ -1709,6 +1730,13 @@ struct CounterCoverageMappingBuilder : addCounters( addCounters(ParentCount, BackedgeCount, BodyBC.ContinueCount), IncrementBC.ContinueCount); + auto [ExecCount, ExitCount] = + (llvm::EnableSingleByteCoverage + ? std::make_pair(getRegionCounter(S), Counter::getZero()) + : getBranchCounterPair(S, CondCount)); + if (!llvm::EnableSingleByteCoverage) { + assert(ExecCount.isZero() || ExecCount == BodyCount); + } if (const Expr *Cond = S->getCond()) { propagateCounts(CondCount, Cond); @@ -1723,9 +1751,8 @@ struct CounterCoverageMappingBuilder Counter OutCount = llvm::EnableSingleByteCoverage ? getRegionCounter(S) - : addCounters(BodyBC.BreakCount, IncrementBC.BreakCount, - subtractCounters(CondCount, BodyCount)); - if (OutCount != ParentCount) { + : addCounters(BodyBC.BreakCount, IncrementBC.BreakCount, ExitCount); + if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; if (BodyHasTerminateStmt) @@ -1734,8 +1761,7 @@ struct CounterCoverageMappingBuilder // Create Branch Region around condition. if (!llvm::EnableSingleByteCoverage) - createBranchRegion(S->getCond(), BodyCount, - subtractCounters(CondCount, BodyCount)); + createBranchRegion(S->getCond(), BodyCount, ExitCount); } void VisitCXXForRangeStmt(const CXXForRangeStmt *S) { @@ -1764,15 +1790,21 @@ struct CounterCoverageMappingBuilder fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount); Counter OutCount; + Counter ExitCount; Counter LoopCount; if (llvm::EnableSingleByteCoverage) OutCount = getRegionCounter(S); else { - LoopCount = addCounters(ParentCount, BackedgeCount, BC.ContinueCount); - OutCount = - addCounters(BC.BreakCount, subtractCounters(LoopCount, BodyCount)); + LoopCount = + (ParentCount.isZero() + ? ParentCount + : addCounters(ParentCount, BackedgeCount, BC.ContinueCount)); + auto [ExecCount, SkipCount] = getBranchCounterPair(S, LoopCount); + ExitCount = SkipCount; + assert(ExecCount.isZero() || ExecCount == BodyCount); + OutCount = addCounters(BC.BreakCount, ExitCount); } - if (OutCount != ParentCount) { + if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; if (BodyHasTerminateStmt) @@ -1781,8 +1813,7 @@ struct CounterCoverageMappingBuilder // Create Branch Region around condition. if (!llvm::EnableSingleByteCoverage) - createBranchRegion(S->getCond(), BodyCount, - subtractCounters(LoopCount, BodyCount)); + createBranchRegion(S->getCond(), BodyCount, ExitCount); } void VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S) { @@ -1803,10 +1834,13 @@ struct CounterCoverageMappingBuilder fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount); Counter LoopCount = - addCounters(ParentCount, BackedgeCount, BC.ContinueCount); - Counter OutCount = - addCounters(BC.BreakCount, subtractCounters(LoopCount, BodyCount)); - if (OutCount != ParentCount) { + (ParentCount.isZero() + ? ParentCount + : addCounters(ParentCount, BackedgeCount, BC.ContinueCount)); + auto [ExecCount, ExitCount] = getBranchCounterPair(S, LoopCount); + assert(ExecCount.isZero() || ExecCount == BodyCount); + Counter OutCount = addCounters(BC.BreakCount, ExitCount); + if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; } @@ -2016,9 +2050,12 @@ struct CounterCoverageMappingBuilder extendRegion(S->getCond()); Counter ParentCount = getRegion().getCounter(); - Counter ThenCount = llvm::EnableSingleByteCoverage - ? getRegionCounter(S->getThen()) - : getRegionCounter(S); + auto [ThenCount, ElseCount] = + (llvm::EnableSingleByteCoverage + ? std::make_pair(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. @@ -2033,12 +2070,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; @@ -2061,15 +2092,14 @@ struct CounterCoverageMappingBuilder if (llvm::EnableSingleByteCoverage) OutCount = getRegionCounter(S); - if (OutCount != ParentCount) { + if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; } if (!S->isConsteval() && !llvm::EnableSingleByteCoverage) // Create Branch Region around condition. - createBranchRegion(S->getCond(), ThenCount, - subtractCounters(ParentCount, ThenCount)); + createBranchRegion(S->getCond(), ThenCount, ElseCount); } void VisitCXXTryStmt(const CXXTryStmt *S) { @@ -2095,9 +2125,11 @@ struct CounterCoverageMappingBuilder extendRegion(E); Counter ParentCount = getRegion().getCounter(); - Counter TrueCount = llvm::EnableSingleByteCoverage - ? getRegionCounter(E->getTrueExpr()) - : getRegionCounter(E); + auto [TrueCount, FalseCount] = + (llvm::EnableSingleByteCoverage + ? std::make_pair(getRegionCounter(E->getTrueExpr()), + getRegionCounter(E->getFalseExpr())) + : getBranchCounterPair(E, ParentCount)); Counter OutCount; if (const auto *BCO = dyn_cast<BinaryConditionalOperator>(E)) { @@ -2116,25 +2148,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) { @@ -2233,27 +2260,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) @@ -2294,31 +2321,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) >From ad136910aad1c8e53a8c6091999ad2f90d180761 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Fri, 18 Oct 2024 09:37:18 +0900 Subject: [PATCH 2/2] Rewind changes for folding --- clang/lib/CodeGen/CoverageMappingGen.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 0bfad9cbcbe12b..8bd9ab402f4e59 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -1795,10 +1795,7 @@ struct CounterCoverageMappingBuilder if (llvm::EnableSingleByteCoverage) OutCount = getRegionCounter(S); else { - LoopCount = - (ParentCount.isZero() - ? ParentCount - : addCounters(ParentCount, BackedgeCount, BC.ContinueCount)); + LoopCount = addCounters(ParentCount, BackedgeCount, BC.ContinueCount); auto [ExecCount, SkipCount] = getBranchCounterPair(S, LoopCount); ExitCount = SkipCount; assert(ExecCount.isZero() || ExecCount == BodyCount); @@ -1834,9 +1831,7 @@ struct CounterCoverageMappingBuilder fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount); Counter LoopCount = - (ParentCount.isZero() - ? ParentCount - : addCounters(ParentCount, BackedgeCount, BC.ContinueCount)); + addCounters(ParentCount, BackedgeCount, BC.ContinueCount); auto [ExecCount, ExitCount] = getBranchCounterPair(S, LoopCount); assert(ExecCount.isZero() || ExecCount == BodyCount); Counter OutCount = addCounters(BC.BreakCount, ExitCount); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
