From 618e63946923a460b2684738e636c77b1706322a Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Thu, 17 Oct 2024 01:22:54 +0900 Subject: [PATCH 01/34] Introduce CounterExpressionBuilder::replace(C, Map) This return a counter for each term in the expression replaced by ReplaceMap. At the moment, this doesn't update the Map, so Map is marked as `const`. --- .../ProfileData/Coverage/CoverageMapping.h | 6 ++++ .../ProfileData/Coverage/CoverageMapping.cpp | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h index fa07b3a9e8b14..d6528bd407ef3 100644 --- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h +++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -213,6 +214,11 @@ class CounterExpressionBuilder { /// Return a counter that represents the expression that subtracts RHS from /// LHS. Counter subtract(Counter LHS, Counter RHS, bool Simplify = true); + + using ReplaceMap = std::map; + + /// Return a counter for each term in the expression replaced by ReplaceMap. + Counter replace(Counter C, const ReplaceMap &Map); }; using LineColPair = std::pair; diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index c713371da81e4..b50f025d261e1 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -135,6 +135,38 @@ Counter CounterExpressionBuilder::subtract(Counter LHS, Counter RHS, return Simplify ? simplify(Cnt) : Cnt; } +Counter CounterExpressionBuilder::replace(Counter C, const ReplaceMap &Map) { + auto I = Map.find(C); + + // Replace C with the Map even if C is Expression. + if (I != Map.end()) + return I->second; + + // Traverse only Expression. + if (!C.isExpression()) + return C; + + auto CE = Expressions[C.getExpressionID()]; + auto NewLHS = replace(CE.LHS, Map); + auto NewRHS = replace(CE.RHS, Map); + + // Reconstruct Expression with induced subexpressions. + switch (CE.Kind) { + case CounterExpression::Add: + C = add(NewLHS, NewRHS); + break; + case CounterExpression::Subtract: + C = subtract(NewLHS, NewRHS); + break; + } + + // Reconfirm if the reconstructed expression would hit the Map. + if ((I = Map.find(C)) != Map.end()) + return I->second; + + return C; +} + void CounterMappingContext::dump(const Counter &C, raw_ostream &OS) const { switch (C.getKind()) { case Counter::Zero: From fc697f04fd6c9f3c217ce04e3f1dd082c1f1a705 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 16 Oct 2024 23:16:53 +0900 Subject: [PATCH 02/34] [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 07015834bc84f..0bfad9cbcbe12 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 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(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 e4172ca273a6fdfcbfc4662c9e37276ef34c2df4 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Thu, 17 Oct 2024 00:32:26 +0900 Subject: [PATCH 03/34] Introduce the type `CounterPair` for RegionCounterMap `CounterPair` can hold `` instead of current `unsigned`, to hold also the counter number of SkipPath. For now, this change provides the skeleton and only `CounterPair::first` is used. Each counter number can have `None` to suppress emitting counter increment. `second` is initialized as `None` by default, since most `Stmt*` don't have a pair of counters. This change also provides stubs for the verifyer. I'll provide the impl of verifier for `+Asserts` later. `markStmtAsUsed(bool, Stmt*)` may be used to inform that other side counter may not emitted. `markStmtMaybeUsed(S)` may be used for the `Stmt` and its inner will be excluded for emission in the case of skipping by constant folding. I put it into places where I found. `verifyCounterMap()` will check the coverage map the counter map and can be used to report inconsistency. These verifier methods shall be eliminated in `-Asserts`. https://discourse.llvm.org/t/rfc-integrating-singlebytecoverage-with-branch-coverage/82492 --- clang/lib/CodeGen/CGDecl.cpp | 9 ++++++++- clang/lib/CodeGen/CGExpr.cpp | 1 + clang/lib/CodeGen/CGExprScalar.cpp | 9 +++++++-- clang/lib/CodeGen/CGStmt.cpp | 3 +++ clang/lib/CodeGen/CodeGenFunction.cpp | 3 +++ clang/lib/CodeGen/CodeGenFunction.h | 6 ++++++ clang/lib/CodeGen/CodeGenModule.h | 19 +++++++++++++++++++ clang/lib/CodeGen/CodeGenPGO.cpp | 14 ++++++++++---- clang/lib/CodeGen/CodeGenPGO.h | 17 +++++++++++++++-- clang/lib/CodeGen/CoverageMappingGen.cpp | 6 +++--- clang/lib/CodeGen/CoverageMappingGen.h | 5 +++-- 11 files changed, 78 insertions(+), 14 deletions(-) diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index 563f728e29d78..ed5f41b624b62 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -362,6 +362,8 @@ CodeGenFunction::AddInitializerToStaticVarDecl(const VarDecl &D, return GV; } + PGO.markStmtMaybeUsed(D.getInit()); // FIXME: Too lazy + #ifndef NDEBUG CharUnits VarSize = CGM.getContext().getTypeSizeInChars(D.getType()) + D.getFlexibleArrayInitChars(getContext()); @@ -1869,7 +1871,10 @@ void CodeGenFunction::EmitAutoVarInit(const AutoVarEmission &emission) { // If we are at an unreachable point, we don't need to emit the initializer // unless it contains a label. if (!HaveInsertPoint()) { - if (!Init || !ContainsLabel(Init)) return; + if (!Init || !ContainsLabel(Init)) { + PGO.markStmtMaybeUsed(Init); + return; + } EnsureInsertPoint(); } @@ -1978,6 +1983,8 @@ void CodeGenFunction::EmitAutoVarInit(const AutoVarEmission &emission) { return EmitExprAsInit(Init, &D, lv, capturedByInit); } + PGO.markStmtMaybeUsed(Init); + if (!emission.IsConstantAggregate) { // For simple scalar/complex initialization, store the value directly. LValue lv = MakeAddrLValue(Loc, type); diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 52d2f6d52abf9..2fd6b02a3395e 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -5134,6 +5134,7 @@ std::optional HandleConditionalOperatorLValueSimpleCase( // If the true case is live, we need to track its region. if (CondExprBool) CGF.incrementProfileCounter(E); + CGF.markStmtMaybeUsed(Dead); // If a throw expression we emit it and return an undefined lvalue // because it can't be used. if (auto *ThrowExpr = dyn_cast(Live->IgnoreParens())) { diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index b7f5b932c56b6..74e93f889f426 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -4982,8 +4982,10 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { } // 0 && RHS: If it is safe, just elide the RHS, and return 0/false. - if (!CGF.ContainsLabel(E->getRHS())) + if (!CGF.ContainsLabel(E->getRHS())) { + CGF.markStmtMaybeUsed(E->getRHS()); return llvm::Constant::getNullValue(ResTy); + } } // If the top of the logical operator nest, reset the MCDC temp to 0. @@ -5122,8 +5124,10 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { } // 1 || RHS: If it is safe, just elide the RHS, and return 1/true. - if (!CGF.ContainsLabel(E->getRHS())) + if (!CGF.ContainsLabel(E->getRHS())) { + CGF.markStmtMaybeUsed(E->getRHS()); return llvm::ConstantInt::get(ResTy, 1); + } } // If the top of the logical operator nest, reset the MCDC temp to 0. @@ -5247,6 +5251,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) { CGF.incrementProfileCounter(E); } Value *Result = Visit(live); + CGF.markStmtMaybeUsed(dead); // If the live part is a throw expression, it acts like it has a void // type, so evaluating it returns a null Value*. However, a conditional diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index 41dc91c578c80..dbc1ce9bf993c 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -76,6 +76,7 @@ void CodeGenFunction::EmitStmt(const Stmt *S, ArrayRef Attrs) { // Verify that any decl statements were handled as simple, they may be in // scope of subsequent reachable statements. assert(!isa(*S) && "Unexpected DeclStmt!"); + PGO.markStmtMaybeUsed(S); return; } @@ -845,6 +846,7 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) { RunCleanupsScope ExecutedScope(*this); EmitStmt(Executed); } + PGO.markStmtMaybeUsed(Skipped); return; } } @@ -2170,6 +2172,7 @@ void CodeGenFunction::EmitSwitchStmt(const SwitchStmt &S) { for (unsigned i = 0, e = CaseStmts.size(); i != e; ++i) EmitStmt(CaseStmts[i]); incrementProfileCounter(&S); + PGO.markStmtMaybeUsed(S.getBody()); // Now we want to restore the saved switch instance so that nested // switches continue to function properly diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 24723e392c2a3..371aa494e014b 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1606,6 +1606,8 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn, // Emit the standard function epilogue. FinishFunction(BodyRange.getEnd()); + PGO.verifyCounterMap(); + // If we haven't marked the function nothrow through other means, do // a quick pass now to see if we can. if (!CurFn->doesNotThrow()) @@ -1728,6 +1730,7 @@ bool CodeGenFunction::ConstantFoldsToSimpleInteger(const Expr *Cond, if (!AllowLabels && CodeGenFunction::ContainsLabel(Cond)) return false; // Contains a label. + PGO.markStmtMaybeUsed(Cond); ResultInt = Int; return true; } diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 9ba0ed02a564d..89ac3b342d0a7 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1620,6 +1620,12 @@ class CodeGenFunction : public CodeGenTypeCache { uint64_t LoopCount) const; public: + std::pair getIsCounterPair(const Stmt *S) const { + return PGO.getIsCounterPair(S); + } + + void markStmtMaybeUsed(const Stmt *S) { PGO.markStmtMaybeUsed(S); } + /// Increment the profiler's counter for the given statement by \p StepV. /// If \p StepV is null, the default increment is 1. void incrementProfileCounter(const Stmt *S, llvm::Value *StepV = nullptr) { diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index c58bb88035ca8..9dc497321b42a 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -101,6 +101,25 @@ enum ForDefinition_t : bool { ForDefinition = true }; +class CounterPair : public std::pair { +private: + static constexpr uint32_t None = (1u << 31); /// None is set + +public: + static constexpr uint32_t Mask = None - 1; + +public: + CounterPair(unsigned Val = 0) { + assert(!(Val & ~Mask)); + first = Val; + second = None; + } + + std::pair getIsCounterPair() const { + return {!(first & None), !(second & None)}; + } +}; + struct OrderGlobalInitsOrStermFinalizers { unsigned int priority; unsigned int lex_order; diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 820bb521ccf85..069469e3de856 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -164,7 +164,7 @@ struct MapRegionCounters : public RecursiveASTVisitor { /// The function hash. PGOHash Hash; /// The map of statements to counters. - llvm::DenseMap &CounterMap; + llvm::DenseMap &CounterMap; /// The state of MC/DC Coverage in this function. MCDC::State &MCDCState; /// Maximum number of supported MC/DC conditions in a boolean expression. @@ -175,7 +175,7 @@ struct MapRegionCounters : public RecursiveASTVisitor { DiagnosticsEngine &Diag; MapRegionCounters(PGOHashVersion HashVersion, uint64_t ProfileVersion, - llvm::DenseMap &CounterMap, + llvm::DenseMap &CounterMap, MCDC::State &MCDCState, unsigned MCDCMaxCond, DiagnosticsEngine &Diag) : NextCounter(0), Hash(HashVersion), CounterMap(CounterMap), @@ -1084,7 +1084,7 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) { (CGM.getCodeGenOpts().MCDCCoverage ? CGM.getCodeGenOpts().MCDCMaxConds : 0); - RegionCounterMap.reset(new llvm::DenseMap); + RegionCounterMap.reset(new llvm::DenseMap); RegionMCDCState.reset(new MCDC::State); MapRegionCounters Walker(HashVersion, ProfileVersion, *RegionCounterMap, *RegionMCDCState, MCDCMaxConditions, CGM.getDiags()); @@ -1186,12 +1186,18 @@ CodeGenPGO::applyFunctionAttributes(llvm::IndexedInstrProfReader *PGOReader, Fn->setEntryCount(FunctionCount); } +std::pair CodeGenPGO::getIsCounterPair(const Stmt *S) const { + if (!RegionCounterMap || RegionCounterMap->count(S) == 0) + return {false, false}; + return (*RegionCounterMap)[S].getIsCounterPair(); +} + void CodeGenPGO::emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, llvm::Value *StepV) { if (!RegionCounterMap || !Builder.GetInsertBlock()) return; - unsigned Counter = (*RegionCounterMap)[S]; + unsigned Counter = (*RegionCounterMap)[S].first; // Make sure that pointer to global is passed in with zero addrspace // This is relevant during GPU profiling diff --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h index 9d66ffad6f435..83f35785e5327 100644 --- a/clang/lib/CodeGen/CodeGenPGO.h +++ b/clang/lib/CodeGen/CodeGenPGO.h @@ -35,7 +35,7 @@ class CodeGenPGO { std::array NumValueSites; unsigned NumRegionCounters; uint64_t FunctionHash; - std::unique_ptr> RegionCounterMap; + std::unique_ptr> RegionCounterMap; std::unique_ptr> StmtCountMap; std::unique_ptr ProfRecord; std::unique_ptr RegionMCDCState; @@ -110,6 +110,7 @@ class CodeGenPGO { bool canEmitMCDCCoverage(const CGBuilderTy &Builder); public: + std::pair getIsCounterPair(const Stmt *S) const; void emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, llvm::Value *StepV); void emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder, const Expr *S, @@ -122,6 +123,18 @@ class CodeGenPGO { Address MCDCCondBitmapAddr, llvm::Value *Val, CodeGenFunction &CGF); + void markStmtAsUsed(bool Skipped, const Stmt *S) { + // Do nothing. + } + + void markStmtMaybeUsed(const Stmt *S) { + // Do nothing. + } + + void verifyCounterMap() { + // Do nothing. + } + /// Return the region count for the counter at the given index. uint64_t getRegionCount(const Stmt *S) { if (!RegionCounterMap) @@ -130,7 +143,7 @@ class CodeGenPGO { return 0; // With profiles from a differing version of clang we can have mismatched // decl counts. Don't crash in such a case. - auto Index = (*RegionCounterMap)[S]; + auto Index = (*RegionCounterMap)[S].first; if (Index >= RegionCounts.size()) return 0; return RegionCounts[Index]; diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 07015834bc84f..08e4ac80e0e87 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -885,7 +885,7 @@ struct CounterCoverageMappingBuilder : public CoverageMappingBuilder, public ConstStmtVisitor { /// The map of statements to count values. - llvm::DenseMap &CounterMap; + llvm::DenseMap &CounterMap; MCDC::State &MCDCState; @@ -938,7 +938,7 @@ struct CounterCoverageMappingBuilder /// /// This should only be called on statements that have a dedicated counter. Counter getRegionCounter(const Stmt *S) { - return Counter::getCounter(CounterMap[S]); + return Counter::getCounter(CounterMap[S].first); } /// Push a region onto the stack. @@ -1421,7 +1421,7 @@ struct CounterCoverageMappingBuilder CounterCoverageMappingBuilder( CoverageMappingModuleGen &CVM, - llvm::DenseMap &CounterMap, + llvm::DenseMap &CounterMap, MCDC::State &MCDCState, SourceManager &SM, const LangOptions &LangOpts) : CoverageMappingBuilder(CVM, SM, LangOpts), CounterMap(CounterMap), MCDCState(MCDCState), MCDCBuilder(CVM.getCodeGenModule(), MCDCState) {} diff --git a/clang/lib/CodeGen/CoverageMappingGen.h b/clang/lib/CodeGen/CoverageMappingGen.h index fe4b93f3af856..0ed50597e1dc3 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.h +++ b/clang/lib/CodeGen/CoverageMappingGen.h @@ -95,6 +95,7 @@ class CoverageSourceInfo : public PPCallbacks, namespace CodeGen { class CodeGenModule; +class CounterPair; namespace MCDC { struct State; @@ -158,7 +159,7 @@ class CoverageMappingGen { CoverageMappingModuleGen &CVM; SourceManager &SM; const LangOptions &LangOpts; - llvm::DenseMap *CounterMap; + llvm::DenseMap *CounterMap; MCDC::State *MCDCState; public: @@ -169,7 +170,7 @@ class CoverageMappingGen { CoverageMappingGen(CoverageMappingModuleGen &CVM, SourceManager &SM, const LangOptions &LangOpts, - llvm::DenseMap *CounterMap, + llvm::DenseMap *CounterMap, MCDC::State *MCDCState) : CVM(CVM), SM(SM), LangOpts(LangOpts), CounterMap(CounterMap), MCDCState(MCDCState) {} From 5e460594c8a2550c38c759b2e6f1c5dc4152f820 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Thu, 17 Oct 2024 22:15:12 +0900 Subject: [PATCH 04/34] [Coverage] Make additional counters available for BranchRegion. NFC. `getBranchCounterPair()` allocates an additional Counter to SkipPath in `SingleByteCoverage`. `IsCounterEqual()` calculates the comparison with rewinding counter replacements. `NumRegionCounters` is updated to take additional counters in account. `incrementProfileCounter()` has a few additiona arguments. - `UseSkipPath=true`, to specify setting counters for SkipPath. It assumes `UseSkipPath=false` is used together. - `UseBoth` may be specified for marking another path. It introduces the same effect as issueing `markStmtAsUsed(!SkipPath, S)`. `llvm-cov` discovers counters in `FalseCount` to allocate `MaxCounterID` for empty profile data. --- clang/lib/CodeGen/CodeGenFunction.h | 8 ++++- clang/lib/CodeGen/CodeGenPGO.cpp | 31 +++++++++++++++++-- clang/lib/CodeGen/CodeGenPGO.h | 1 + clang/lib/CodeGen/CoverageMappingGen.cpp | 31 ++++++++++++++----- .../ProfileData/Coverage/CoverageMapping.cpp | 4 +++ 5 files changed, 65 insertions(+), 10 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 89ac3b342d0a7..cb1192bf6e11f 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1629,11 +1629,17 @@ class CodeGenFunction : public CodeGenTypeCache { /// Increment the profiler's counter for the given statement by \p StepV. /// If \p StepV is null, the default increment is 1. void incrementProfileCounter(const Stmt *S, llvm::Value *StepV = nullptr) { + incrementProfileCounter(false, S, false, StepV); + } + + void incrementProfileCounter(bool UseSkipPath, const Stmt *S, + bool UseBoth = false, + llvm::Value *StepV = nullptr) { if (CGM.getCodeGenOpts().hasProfileClangInstr() && !CurFn->hasFnAttribute(llvm::Attribute::NoProfile) && !CurFn->hasFnAttribute(llvm::Attribute::SkipProfile)) { auto AL = ApplyDebugLocation::CreateArtificial(*this); - PGO.emitCounterSetOrIncrement(Builder, S, StepV); + PGO.emitCounterSetOrIncrement(Builder, S, UseSkipPath, UseBoth, StepV); } PGO.setCurrentStmt(S); } diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 069469e3de856..aefd53e12088b 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -1138,6 +1138,19 @@ void CodeGenPGO::emitCounterRegionMapping(const Decl *D) { if (CoverageMapping.empty()) return; + // Scan max(FalseCnt) and update NumRegionCounters. + unsigned MaxNumCounters = NumRegionCounters; + for (const auto [_, V] : *RegionCounterMap) { + auto HasCounters = V.getIsCounterPair(); + assert((!HasCounters.first || + MaxNumCounters > (V.first & CounterPair::Mask)) && + "TrueCnt should not be reassigned"); + if (HasCounters.second) + MaxNumCounters = + std::max(MaxNumCounters, (V.second & CounterPair::Mask) + 1); + } + NumRegionCounters = MaxNumCounters; + CGM.getCoverageMapping()->addFunctionMappingRecord( FuncNameVar, FuncName, FunctionHash, CoverageMapping); } @@ -1193,11 +1206,25 @@ std::pair CodeGenPGO::getIsCounterPair(const Stmt *S) const { } void CodeGenPGO::emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, + bool UseSkipPath, bool UseBoth, llvm::Value *StepV) { - if (!RegionCounterMap || !Builder.GetInsertBlock()) + if (!RegionCounterMap) return; - unsigned Counter = (*RegionCounterMap)[S].first; + unsigned Counter; + auto &TheMap = (*RegionCounterMap)[S]; + auto IsCounter = TheMap.getIsCounterPair(); + if (!UseSkipPath) { + assert(IsCounter.first); + Counter = (TheMap.first & CounterPair::Mask); + } else { + if (!IsCounter.second) + return; + Counter = (TheMap.second & CounterPair::Mask); + } + + if (!Builder.GetInsertBlock()) + return; // Make sure that pointer to global is passed in with zero addrspace // This is relevant during GPU profiling diff --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h index 83f35785e5327..8b769dd88d7f1 100644 --- a/clang/lib/CodeGen/CodeGenPGO.h +++ b/clang/lib/CodeGen/CodeGenPGO.h @@ -112,6 +112,7 @@ class CodeGenPGO { public: std::pair getIsCounterPair(const Stmt *S) const; void emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, + bool UseFalsePath, bool UseBoth, llvm::Value *StepV); void emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder, const Expr *S, Address MCDCCondBitmapAddr, diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index a5d83e7a743bb..0bcbd20593ae2 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -887,6 +887,9 @@ struct CounterCoverageMappingBuilder /// The map of statements to count values. llvm::DenseMap &CounterMap; + CounterExpressionBuilder::ReplaceMap MapToExpand; + unsigned NextCounterNum; + MCDC::State &MCDCState; /// A stack of currently live regions. @@ -922,15 +925,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); } @@ -943,14 +942,31 @@ struct CounterCoverageMappingBuilder std::pair getBranchCounterPair(const Stmt *S, Counter ParentCnt) { - Counter ExecCnt = getRegionCounter(S); - return {ExecCnt, Builder.subtract(ParentCnt, ExecCnt)}; + auto &TheMap = CounterMap[S]; + auto ExecCnt = Counter::getCounter(TheMap.first); + auto SkipExpr = Builder.subtract(ParentCnt, ExecCnt); + + if (!llvm::EnableSingleByteCoverage) + return {ExecCnt, SkipExpr}; + + // Assign second if second is not assigned yet. + if (!TheMap.getIsCounterPair().second) + TheMap.second = NextCounterNum++; + + Counter SkipCnt = Counter::getCounter(TheMap.second); + MapToExpand[SkipCnt] = SkipExpr; + return {ExecCnt, SkipCnt}; } bool IsCounterEqual(Counter OutCount, Counter ParentCount) { if (OutCount == ParentCount) return true; + // Try comaparison with pre-replaced expressions. + if (Builder.replace(Builder.subtract(OutCount, ParentCount), MapToExpand) + .isZero()) + return true; + return false; } @@ -1437,7 +1453,8 @@ struct CounterCoverageMappingBuilder llvm::DenseMap &CounterMap, MCDC::State &MCDCState, SourceManager &SM, const LangOptions &LangOpts) : CoverageMappingBuilder(CVM, SM, LangOpts), CounterMap(CounterMap), - MCDCState(MCDCState), MCDCBuilder(CVM.getCodeGenModule(), MCDCState) {} + NextCounterNum(CounterMap.size()), MCDCState(MCDCState), + MCDCBuilder(CVM.getCodeGenModule(), MCDCState) {} /// Write the mapping data to the output stream void write(llvm::raw_ostream &OS) { diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index b50f025d261e1..fc7f36c8599f5 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -640,6 +640,10 @@ static unsigned getMaxCounterID(const CounterMappingContext &Ctx, unsigned MaxCounterID = 0; for (const auto &Region : Record.MappingRegions) { MaxCounterID = std::max(MaxCounterID, Ctx.getMaxCounterID(Region.Count)); + if (Region.Kind == CounterMappingRegion::BranchRegion || + Region.Kind == CounterMappingRegion::MCDCBranchRegion) + MaxCounterID = + std::max(MaxCounterID, Ctx.getMaxCounterID(Region.FalseCount)); } return MaxCounterID; } From ad136910aad1c8e53a8c6091999ad2f90d180761 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Fri, 18 Oct 2024 09:37:18 +0900 Subject: [PATCH 05/34] 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 0bfad9cbcbe12..8bd9ab402f4e5 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); From 209ea4cfdb4f93d3c8fc60dc9af29af39fd24758 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sun, 20 Oct 2024 11:58:16 +0900 Subject: [PATCH 06/34] Update comments --- llvm/lib/ProfileData/Coverage/CoverageMapping.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index b50f025d261e1..6f44797be20e3 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -138,14 +138,14 @@ Counter CounterExpressionBuilder::subtract(Counter LHS, Counter RHS, Counter CounterExpressionBuilder::replace(Counter C, const ReplaceMap &Map) { auto I = Map.find(C); - // Replace C with the Map even if C is Expression. + // Replace C with the value found in Map even if C is Expression. if (I != Map.end()) return I->second; - // Traverse only Expression. if (!C.isExpression()) return C; + // Traverse both sides of Expression. auto CE = Expressions[C.getExpressionID()]; auto NewLHS = replace(CE.LHS, Map); auto NewRHS = replace(CE.RHS, Map); From f0afd04dd86573f1e7d868cc6e1c677d1779aa5f Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sun, 20 Oct 2024 12:00:23 +0900 Subject: [PATCH 07/34] Use initializer statements --- llvm/lib/ProfileData/Coverage/CoverageMapping.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index 6f44797be20e3..7ff2e5ba69e19 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -136,10 +136,8 @@ Counter CounterExpressionBuilder::subtract(Counter LHS, Counter RHS, } Counter CounterExpressionBuilder::replace(Counter C, const ReplaceMap &Map) { - auto I = Map.find(C); - // Replace C with the value found in Map even if C is Expression. - if (I != Map.end()) + if (auto I = Map.find(C); I != Map.end()) return I->second; if (!C.isExpression()) @@ -161,7 +159,7 @@ Counter CounterExpressionBuilder::replace(Counter C, const ReplaceMap &Map) { } // Reconfirm if the reconstructed expression would hit the Map. - if ((I = Map.find(C)) != Map.end()) + if (auto I = Map.find(C); I != Map.end()) return I->second; return C; From be516faa39e1152d637ac425229f8a88480ba41b Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sun, 20 Oct 2024 11:57:03 +0900 Subject: [PATCH 08/34] `first` may be cancelled. Currently `first` is not None by default. --- clang/lib/CodeGen/CodeGenPGO.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index aefd53e12088b..0f2090da47a37 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -1215,7 +1215,8 @@ void CodeGenPGO::emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, auto &TheMap = (*RegionCounterMap)[S]; auto IsCounter = TheMap.getIsCounterPair(); if (!UseSkipPath) { - assert(IsCounter.first); + if (!IsCounter.first) + return; Counter = (TheMap.first & CounterPair::Mask); } else { if (!IsCounter.second) From 03cfce188cb45adbe78f7e77c2fdd244650f5a3c Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 23 Oct 2024 14:39:35 +0000 Subject: [PATCH 09/34] CGF::markStmtAsUsed --- clang/lib/CodeGen/CodeGenFunction.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 89ac3b342d0a7..fcad1cbcae5e3 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1624,6 +1624,9 @@ class CodeGenFunction : public CodeGenTypeCache { return PGO.getIsCounterPair(S); } + void markStmtAsUsed(bool Skipped, const Stmt *S) { + PGO.markStmtAsUsed(Skipped, S); + } void markStmtMaybeUsed(const Stmt *S) { PGO.markStmtMaybeUsed(S); } /// Increment the profiler's counter for the given statement by \p StepV. From afc8481f7cf20da7de4e95a60bf3ccdb04bd08f3 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 23 Oct 2024 14:39:35 +0000 Subject: [PATCH 10/34] CGF.markStmtMaybeUsed for binop --- clang/lib/CodeGen/CGExprScalar.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 74e93f889f426..9e8533ca90a5c 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -4970,7 +4970,8 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) { CGF.incrementProfileCounter(E->getRHS()); CGF.EmitBranch(FBlock); CGF.EmitBlock(FBlock); - } + } else + CGF.markStmtMaybeUsed(E->getRHS()); CGF.MCDCLogOpStack.pop_back(); // If the top of the logical operator nest, update the MCDC bitmap. @@ -5112,7 +5113,8 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { CGF.incrementProfileCounter(E->getRHS()); CGF.EmitBranch(FBlock); CGF.EmitBlock(FBlock); - } + } else + CGF.markStmtMaybeUsed(E->getRHS()); CGF.MCDCLogOpStack.pop_back(); // If the top of the logical operator nest, update the MCDC bitmap. From ab84f17fc181cb4b38693dc6ea80ac44f44bf990 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sun, 27 Oct 2024 20:28:26 +0900 Subject: [PATCH 11/34] Introduce skeleton getSwitchImplicitDefaultCounter() --- clang/lib/CodeGen/CoverageMappingGen.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 1782434bdb9aa..532f6e8ba1820 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -947,6 +947,11 @@ struct CounterCoverageMappingBuilder return {ExecCnt, Builder.subtract(ParentCnt, ExecCnt)}; } + Counter getSwitchImplicitDefaultCounter(const Stmt *Cond, Counter ParentCount, + Counter CaseCountSum) { + return Builder.subtract(ParentCount, CaseCountSum); + } + bool IsCounterEqual(Counter OutCount, Counter ParentCount) { if (OutCount == ParentCount) return true; @@ -1903,7 +1908,8 @@ struct CounterCoverageMappingBuilder // the hidden branch, which will be added later by the CodeGen. This region // will be associated with the switch statement's condition. if (!HasDefaultCase) { - Counter DefaultCount = subtractCounters(ParentCount, CaseCountSum); + Counter DefaultCount = getSwitchImplicitDefaultCounter( + S->getCond(), ParentCount, CaseCountSum); createBranchRegion(S->getCond(), Counter::getZero(), DefaultCount); } } From a4608854e1b727817db3cc01234f97e78285f8c7 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sun, 27 Oct 2024 20:40:44 +0900 Subject: [PATCH 12/34] Update getSwitchImplicitDefaultCounter --- clang/lib/CodeGen/CoverageMappingGen.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index d4a1b2613eab9..cb861e61d3272 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -960,7 +960,10 @@ struct CounterCoverageMappingBuilder Counter getSwitchImplicitDefaultCounter(const Stmt *Cond, Counter ParentCount, Counter CaseCountSum) { - return Builder.subtract(ParentCount, CaseCountSum); + return ( + llvm::EnableSingleByteCoverage + ? Counter::getCounter(CounterMap[Cond].second = NextCounterNum++) + : Builder.subtract(ParentCount, CaseCountSum)); } bool IsCounterEqual(Counter OutCount, Counter ParentCount) { From 02853943407a5c550554e4920586b8724dd63a76 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Mon, 28 Oct 2024 00:55:49 +0900 Subject: [PATCH 13/34] Don't allocate second if SkipExpr isn't Expr. --- clang/lib/CodeGen/CoverageMappingGen.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index cb861e61d3272..8584f58548eae 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -946,8 +946,12 @@ struct CounterCoverageMappingBuilder auto ExecCnt = Counter::getCounter(TheMap.first); auto SkipExpr = Builder.subtract(ParentCnt, ExecCnt); - if (!llvm::EnableSingleByteCoverage) + if (!llvm::EnableSingleByteCoverage || !SkipExpr.isExpression()) { + assert( + !TheMap.getIsCounterPair().second && + "SkipCnt shouldn't be allocated but refer to an existing counter."); return {ExecCnt, SkipExpr}; + } // Assign second if second is not assigned yet. if (!TheMap.getIsCounterPair().second) From ed700c2cb5b7ceac1bcf5ac0414a11d3148b9990 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sat, 21 Dec 2024 17:49:06 +0900 Subject: [PATCH 14/34] s/replace/subst/ --- llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h | 6 +++--- llvm/lib/ProfileData/Coverage/CoverageMapping.cpp | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h index a55ca997d47fd..dad5f1276f27c 100644 --- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h +++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h @@ -215,10 +215,10 @@ class CounterExpressionBuilder { /// LHS. Counter subtract(Counter LHS, Counter RHS, bool Simplify = true); - using ReplaceMap = std::map; + using SubstMap = std::map; - /// Return a counter for each term in the expression replaced by ReplaceMap. - Counter replace(Counter C, const ReplaceMap &Map); + /// Return a counter for each term in the expression replaced by SubstMap. + Counter subst(Counter C, const SubstMap &Map); }; using LineColPair = std::pair; diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index 6d6f03c360639..3f89414bdbbb9 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -135,7 +135,7 @@ Counter CounterExpressionBuilder::subtract(Counter LHS, Counter RHS, return Simplify ? simplify(Cnt) : Cnt; } -Counter CounterExpressionBuilder::replace(Counter C, const ReplaceMap &Map) { +Counter CounterExpressionBuilder::subst(Counter C, const SubstMap &Map) { // Replace C with the value found in Map even if C is Expression. if (auto I = Map.find(C); I != Map.end()) return I->second; @@ -143,10 +143,9 @@ Counter CounterExpressionBuilder::replace(Counter C, const ReplaceMap &Map) { if (!C.isExpression()) return C; - // Traverse both sides of Expression. auto CE = Expressions[C.getExpressionID()]; - auto NewLHS = replace(CE.LHS, Map); - auto NewRHS = replace(CE.RHS, Map); + auto NewLHS = subst(CE.LHS, Map); + auto NewRHS = subst(CE.RHS, Map); // Reconstruct Expression with induced subexpressions. switch (CE.Kind) { From dbcf896f38c4c950abd23bf5fe8b9dff75476fef Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sat, 21 Dec 2024 18:06:25 +0900 Subject: [PATCH 15/34] getSwitchImplicitDefaultCounterPair --- clang/lib/CodeGen/CoverageMappingGen.cpp | 27 ++++++++++++------------ 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index e63c38fcd4ba2..dff804fbeee92 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -944,12 +944,18 @@ struct CounterCoverageMappingBuilder return {ExecCnt, Builder.subtract(ParentCnt, ExecCnt)}; } - Counter getSwitchImplicitDefaultCounter(const Stmt *Cond, Counter ParentCount, - Counter CaseCountSum) { - if (llvm::EnableSingleByteCoverage) - CaseCountSum = Counter::getZero(); + /// Returns {TrueCnt,FalseCnt} for "implicit default". + /// FalseCnt is considered as the False count on SwitchStmt. + std::pair + getSwitchImplicitDefaultCounterPair(const Stmt *Cond, Counter ParentCount, + Counter CaseCountSum) { + // Simplify is skipped while building the counters above: it can get + // really slow on top of switches with thousands of cases. Instead, + // trigger simplification by adding zero to the last counter. + CaseCountSum = + addCounters(CaseCountSum, Counter::getZero(), /*Simplify=*/true); - return Builder.subtract(ParentCount, CaseCountSum); + return {CaseCountSum, Builder.subtract(ParentCount, CaseCountSum)}; } bool IsCounterEqual(Counter OutCount, Counter ParentCount) { @@ -1910,16 +1916,9 @@ struct CounterCoverageMappingBuilder // the hidden branch, which will be added later by the CodeGen. This region // will be associated with the switch statement's condition. if (!HasDefaultCase) { - // Simplify is skipped while building the counters above: it can get - // really slow on top of switches with thousands of cases. Instead, - // trigger simplification by adding zero to the last counter. - CaseCountSum = - addCounters(CaseCountSum, Counter::getZero(), /*Simplify=*/true); - - // This is considered as the False count on SwitchStmt. - Counter SwitchFalse = getSwitchImplicitDefaultCounter( + auto Counters = getSwitchImplicitDefaultCounterPair( S->getCond(), ParentCount, CaseCountSum); - createBranchRegion(S->getCond(), CaseCountSum, SwitchFalse); + createBranchRegion(S->getCond(), Counters.first, Counters.second); } } From c0785e91103db81b53e000834ce748d8d448e5ef Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sat, 21 Dec 2024 21:31:20 +0900 Subject: [PATCH 16/34] Fold either in switchcase --- clang/lib/CodeGen/CoverageMappingGen.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 238235f72d731..c66280c98e80d 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -964,6 +964,10 @@ struct CounterCoverageMappingBuilder std::pair getSwitchImplicitDefaultCounterPair(const Stmt *Cond, Counter ParentCount, Counter CaseCountSum) { + if (llvm::EnableSingleByteCoverage) + return {Counter::getZero(), // Folded + Counter::getCounter(CounterMap[Cond].second = NextCounterNum++)}; + // Simplify is skipped while building the counters above: it can get // really slow on top of switches with thousands of cases. Instead, // trigger simplification by adding zero to the last counter. @@ -1195,12 +1199,14 @@ struct CounterCoverageMappingBuilder /// and add it to the function's SourceRegions. /// Returns Counter that corresponds to SC. Counter createSwitchCaseRegion(const SwitchCase *SC, Counter ParentCount) { + Counter TrueCnt = getRegionCounter(SC); + Counter FalseCnt = (llvm::EnableSingleByteCoverage + ? Counter::getZero() // Folded + : subtractCounters(ParentCount, TrueCnt)); // Push region onto RegionStack but immediately pop it (which adds it to // the function's SourceRegions) because it doesn't apply to any other // source other than the SwitchCase. - Counter TrueCnt = getRegionCounter(SC); - popRegions(pushRegion(TrueCnt, getStart(SC), SC->getColonLoc(), - subtractCounters(ParentCount, TrueCnt))); + popRegions(pushRegion(TrueCnt, getStart(SC), SC->getColonLoc(), FalseCnt)); return TrueCnt; } From bd1d96bb23b264f4976ce800470dd7ed7f74a709 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Mon, 23 Dec 2024 12:26:29 +0900 Subject: [PATCH 17/34] Introduce CounterMappingRegion::isBranch(). NFC. Conflicts: clang/lib/CodeGen/CoverageMappingGen.cpp llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h --- llvm/lib/ProfileData/Coverage/CoverageMapping.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index 63be03f231841..b2f4087465308 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -637,8 +637,7 @@ static unsigned getMaxCounterID(const CounterMappingContext &Ctx, unsigned MaxCounterID = 0; for (const auto &Region : Record.MappingRegions) { MaxCounterID = std::max(MaxCounterID, Ctx.getMaxCounterID(Region.Count)); - if (Region.Kind == CounterMappingRegion::BranchRegion || - Region.Kind == CounterMappingRegion::MCDCBranchRegion) + if (Region.isBranch()) MaxCounterID = std::max(MaxCounterID, Ctx.getMaxCounterID(Region.FalseCount)); } From 4e41b99fdf41094a85bf473d27d57c527b91a0d8 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Tue, 24 Dec 2024 15:53:14 +0900 Subject: [PATCH 18/34] Introduce BranchCounterPair --- clang/lib/CodeGen/CoverageMappingGen.cpp | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index dff804fbeee92..df29c2cb4ee17 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -938,8 +938,12 @@ struct CounterCoverageMappingBuilder return Counter::getCounter(CounterMap[S]); } - std::pair getBranchCounterPair(const Stmt *S, - Counter ParentCnt) { + struct BranchCounterPair { + Counter Executed; + Counter Skipped; + }; + + BranchCounterPair getBranchCounterPair(const Stmt *S, Counter ParentCnt) { Counter ExecCnt = getRegionCounter(S); return {ExecCnt, Builder.subtract(ParentCnt, ExecCnt)}; } @@ -1617,7 +1621,7 @@ struct CounterCoverageMappingBuilder : addCounters(ParentCount, BackedgeCount, BC.ContinueCount); auto [ExecCount, ExitCount] = (llvm::EnableSingleByteCoverage - ? std::make_pair(getRegionCounter(S), Counter::getZero()) + ? BranchCounterPair{getRegionCounter(S), Counter::getZero()} : getBranchCounterPair(S, CondCount)); if (!llvm::EnableSingleByteCoverage) { assert(ExecCount.isZero() || ExecCount == BodyCount); @@ -1674,7 +1678,7 @@ struct CounterCoverageMappingBuilder : addCounters(BackedgeCount, BC.ContinueCount); auto [ExecCount, ExitCount] = (llvm::EnableSingleByteCoverage - ? std::make_pair(getRegionCounter(S), Counter::getZero()) + ? BranchCounterPair{getRegionCounter(S), Counter::getZero()} : getBranchCounterPair(S, CondCount)); if (!llvm::EnableSingleByteCoverage) { assert(ExecCount.isZero() || ExecCount == BodyCount); @@ -1742,7 +1746,7 @@ struct CounterCoverageMappingBuilder IncrementBC.ContinueCount); auto [ExecCount, ExitCount] = (llvm::EnableSingleByteCoverage - ? std::make_pair(getRegionCounter(S), Counter::getZero()) + ? BranchCounterPair{getRegionCounter(S), Counter::getZero()} : getBranchCounterPair(S, CondCount)); if (!llvm::EnableSingleByteCoverage) { assert(ExecCount.isZero() || ExecCount == BodyCount); @@ -2049,9 +2053,9 @@ struct CounterCoverageMappingBuilder Counter ParentCount = getRegion().getCounter(); auto [ThenCount, ElseCount] = (llvm::EnableSingleByteCoverage - ? std::make_pair(getRegionCounter(S->getThen()), - (S->getElse() ? getRegionCounter(S->getElse()) - : Counter::getZero())) + ? 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 @@ -2124,8 +2128,8 @@ struct CounterCoverageMappingBuilder Counter ParentCount = getRegion().getCounter(); auto [TrueCount, FalseCount] = (llvm::EnableSingleByteCoverage - ? std::make_pair(getRegionCounter(E->getTrueExpr()), - getRegionCounter(E->getFalseExpr())) + ? BranchCounterPair{getRegionCounter(E->getTrueExpr()), + getRegionCounter(E->getFalseExpr())} : getBranchCounterPair(E, ParentCount)); Counter OutCount; From 306d77f3d9afdbb832f920e920008c719a3a3533 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Tue, 24 Dec 2024 15:58:13 +0900 Subject: [PATCH 19/34] void verifyCounterMap() const --- clang/lib/CodeGen/CodeGenPGO.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h index 83f35785e5327..9429b6b465207 100644 --- a/clang/lib/CodeGen/CodeGenPGO.h +++ b/clang/lib/CodeGen/CodeGenPGO.h @@ -131,7 +131,7 @@ class CodeGenPGO { // Do nothing. } - void verifyCounterMap() { + void verifyCounterMap() const { // Do nothing. } From a4f05c7a5ca60b8ed2eaf5337cdd04bab25e8170 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Tue, 24 Dec 2024 15:58:13 +0900 Subject: [PATCH 20/34] Introduce the dedicated class CounterPair instead of std::pair --- clang/lib/CodeGen/CodeGenFunction.h | 4 +- clang/lib/CodeGen/CodeGenModule.h | 53 +++++++++++++++++------- clang/lib/CodeGen/CodeGenPGO.cpp | 11 +++-- clang/lib/CodeGen/CodeGenPGO.h | 2 +- clang/lib/CodeGen/CoverageMappingGen.cpp | 2 +- 5 files changed, 50 insertions(+), 22 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 0396fa5bef136..87d016a0729ad 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1620,9 +1620,7 @@ class CodeGenFunction : public CodeGenTypeCache { uint64_t LoopCount) const; public: - std::pair getIsCounterPair(const Stmt *S) const { - return PGO.getIsCounterPair(S); - } + auto getIsCounterPair(const Stmt *S) const { return PGO.getIsCounterPair(S); } void markStmtAsUsed(bool Skipped, const Stmt *S) { PGO.markStmtAsUsed(Skipped, S); diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 8990ee13991e9..d5ef1a710eb40 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -102,23 +102,48 @@ enum ForDefinition_t : bool { ForDefinition = true }; -class CounterPair : public std::pair { -private: - static constexpr uint32_t None = (1u << 31); /// None is set - +/// The Counter with an optional additional Counter for +/// branches. `Skipped` counter can be calculated with `Executed` and +/// a common Counter (like `Parent`) as `(Parent-Executed)`. +/// +/// In SingleByte mode, Counters are binary. Subtraction is not +/// applicable (but addition is capable). In this case, both +/// `Executed` and `Skipped` counters are required. `Skipped` is +/// `None` by default. It is allocated in the coverage mapping. +/// +/// There might be cases that `Parent` could be induced with +/// `(Executed+Skipped)`. This is not always applicable. +class CounterPair { public: - static constexpr uint32_t Mask = None - 1; + /// Optional value. + class ValueOpt { + private: + static constexpr uint32_t None = (1u << 31); /// None is allocated. + static constexpr uint32_t Mask = None - 1; -public: - CounterPair(unsigned Val = 0) { - assert(!(Val & ~Mask)); - first = Val; - second = None; - } + uint32_t Val; - std::pair getIsCounterPair() const { - return {!(first & None), !(second & None)}; - } + public: + ValueOpt() : Val(None) {} + + ValueOpt(unsigned InitVal) { + assert(!(InitVal & ~Mask)); + Val = InitVal; + } + + bool hasValue() const { return !(Val & None); } + + operator uint32_t() const { return Val; } + }; + + ValueOpt Executed; + ValueOpt Skipped; /// May be None. + + /// Initialized with Skipped=None. + CounterPair(unsigned Val) : Executed(Val) {} + + // FIXME: Should work with {None, None} + CounterPair() : Executed(0) {} }; struct OrderGlobalInitsOrStermFinalizers { diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 75191f86d231c..792373839107f 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -1186,9 +1186,14 @@ CodeGenPGO::applyFunctionAttributes(llvm::IndexedInstrProfReader *PGOReader, } std::pair CodeGenPGO::getIsCounterPair(const Stmt *S) const { - if (!RegionCounterMap || RegionCounterMap->count(S) == 0) + if (!RegionCounterMap) return {false, false}; - return (*RegionCounterMap)[S].getIsCounterPair(); + + auto I = RegionCounterMap->find(S); + if (I == RegionCounterMap->end()) + return {false, false}; + + return {I->second.Executed.hasValue(), I->second.Skipped.hasValue()}; } void CodeGenPGO::emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, @@ -1196,7 +1201,7 @@ void CodeGenPGO::emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, if (!RegionCounterMap || !Builder.GetInsertBlock()) return; - unsigned Counter = (*RegionCounterMap)[S].first; + unsigned Counter = (*RegionCounterMap)[S].Executed; // Make sure that pointer to global is passed in with zero addrspace // This is relevant during GPU profiling diff --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h index 9429b6b465207..1944b640951d5 100644 --- a/clang/lib/CodeGen/CodeGenPGO.h +++ b/clang/lib/CodeGen/CodeGenPGO.h @@ -143,7 +143,7 @@ class CodeGenPGO { return 0; // With profiles from a differing version of clang we can have mismatched // decl counts. Don't crash in such a case. - auto Index = (*RegionCounterMap)[S].first; + auto Index = (*RegionCounterMap)[S].Executed; if (Index >= RegionCounts.size()) return 0; return RegionCounts[Index]; diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 9c191e9394769..6433f930ae41e 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -935,7 +935,7 @@ struct CounterCoverageMappingBuilder /// /// This should only be called on statements that have a dedicated counter. Counter getRegionCounter(const Stmt *S) { - return Counter::getCounter(CounterMap[S].first); + return Counter::getCounter(CounterMap[S].Executed); } /// Push a region onto the stack. From f6c5f4017cc48534d2c3adc68a7c3d97ad45d156 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Tue, 24 Dec 2024 16:13:54 +0900 Subject: [PATCH 21/34] Catch up the merge --- clang/lib/CodeGen/CodeGenPGO.cpp | 24 +++++++++++++++------- clang/lib/CodeGen/CoverageMappingGen.cpp | 26 +++++++++++++++++++++--- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 03856795ca51a..ffb1df9ec2cc6 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -1140,13 +1140,10 @@ void CodeGenPGO::emitCounterRegionMapping(const Decl *D) { // Scan max(FalseCnt) and update NumRegionCounters. unsigned MaxNumCounters = NumRegionCounters; for (const auto [_, V] : *RegionCounterMap) { - auto HasCounters = V.getIsCounterPair(); - assert((!HasCounters.first || - MaxNumCounters > (V.first & CounterPair::Mask)) && + assert((!V.Executed.hasValue() || MaxNumCounters > V.Executed) && "TrueCnt should not be reassigned"); - if (HasCounters.second) - MaxNumCounters = - std::max(MaxNumCounters, (V.second & CounterPair::Mask) + 1); + if (V.Skipped.hasValue()) + MaxNumCounters = std::max(MaxNumCounters, V.Skipped + 1); } NumRegionCounters = MaxNumCounters; @@ -1215,7 +1212,20 @@ void CodeGenPGO::emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S, if (!RegionCounterMap) return; - unsigned Counter = (*RegionCounterMap)[S].Executed; + unsigned Counter; + auto &TheMap = (*RegionCounterMap)[S]; + if (!UseSkipPath) { + if (!TheMap.Executed.hasValue()) + return; + Counter = TheMap.Executed; + } else { + if (!TheMap.Skipped.hasValue()) + return; + Counter = TheMap.Skipped; + } + + if (!Builder.GetInsertBlock()) + return; // Make sure that pointer to global is passed in with zero addrspace // This is relevant during GPU profiling diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index e0aa752bcbe9a..0764345af0b32 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -943,8 +943,27 @@ struct CounterCoverageMappingBuilder }; BranchCounterPair getBranchCounterPair(const Stmt *S, Counter ParentCnt) { - Counter ExecCnt = getRegionCounter(S); - return {ExecCnt, Builder.subtract(ParentCnt, ExecCnt)}; + auto &TheMap = CounterMap[S]; + auto ExecCnt = Counter::getCounter(TheMap.Executed); + BranchCounterPair Counters = {ExecCnt, + Builder.subtract(ParentCnt, ExecCnt)}; + + if (!llvm::EnableSingleByteCoverage || !Counters.Skipped.isExpression()) { + assert( + !TheMap.Skipped.hasValue() && + "SkipCnt shouldn't be allocated but refer to an existing counter."); + return Counters; + } + + // Assign second if second is not assigned yet. + if (!TheMap.Skipped.hasValue()) + TheMap.Skipped = NextCounterNum++; + + // Replace an expression (ParentCnt - ExecCnt) with SkipCnt. + Counter SkipCnt = Counter::getCounter(TheMap.Skipped); + MapToExpand[SkipCnt] = Counters.Skipped; + Counters.Skipped = SkipCnt; + return Counters; } /// Returns {TrueCnt,FalseCnt} for "implicit default". @@ -953,8 +972,9 @@ struct CounterCoverageMappingBuilder getSwitchImplicitDefaultCounterPair(const Stmt *Cond, Counter ParentCount, Counter CaseCountSum) { if (llvm::EnableSingleByteCoverage) + // Allocate the new Counter since `subtract(Parent - Sum)` is unavailable. return {Counter::getZero(), // Folded - Counter::getCounter(CounterMap[Cond].second = NextCounterNum++)}; + Counter::getCounter(CounterMap[Cond].Skipped = NextCounterNum++)}; // Simplify is skipped while building the counters above: it can get // really slow on top of switches with thousands of cases. Instead, From aca86d451ab886951a260c171c63c313522266d2 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Tue, 24 Dec 2024 16:13:54 +0900 Subject: [PATCH 22/34] Introduce {UseExecPath, UseSkipPath} instead of {false, true} --- clang/lib/CodeGen/CodeGenFunction.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 425fce241b983..6145c6a627fc8 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1627,20 +1627,31 @@ class CodeGenFunction : public CodeGenTypeCache { } void markStmtMaybeUsed(const Stmt *S) { PGO.markStmtMaybeUsed(S); } + enum CounterForIncrement { + UseExecPath = 0, + UseSkipPath, + }; + /// Increment the profiler's counter for the given statement by \p StepV. /// If \p StepV is null, the default increment is 1. void incrementProfileCounter(const Stmt *S, llvm::Value *StepV = nullptr) { - incrementProfileCounter(false, S, false, StepV); + incrementProfileCounter(UseExecPath, S, false, StepV); } - void incrementProfileCounter(bool UseSkipPath, const Stmt *S, + /// Emit increment of Counter. + /// \param ExecSkip Use `Skipped` Counter if UseSkipPath is specified. + /// \param S The Stmt that Counter is associated. + /// \param UseBoth Mark both Exec/Skip as used. (for verification) + /// \param StepV The offset Value for adding to Counter. + void incrementProfileCounter(CounterForIncrement ExecSkip, const Stmt *S, bool UseBoth = false, llvm::Value *StepV = nullptr) { if (CGM.getCodeGenOpts().hasProfileClangInstr() && !CurFn->hasFnAttribute(llvm::Attribute::NoProfile) && !CurFn->hasFnAttribute(llvm::Attribute::SkipProfile)) { auto AL = ApplyDebugLocation::CreateArtificial(*this); - PGO.emitCounterSetOrIncrement(Builder, S, UseSkipPath, UseBoth, StepV); + PGO.emitCounterSetOrIncrement(Builder, S, (ExecSkip == UseSkipPath), + UseBoth, StepV); } PGO.setCurrentStmt(S); } From 28c568ad84edbfb4eb4bc7841c8c6b39e9722094 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Mon, 6 Jan 2025 23:37:50 +0900 Subject: [PATCH 23/34] Add a test --- .../ProfileData/CoverageMappingTest.cpp | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp index ef147674591c5..46f881ecddb5f 100644 --- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp +++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp @@ -291,6 +291,45 @@ struct CoverageMappingTest : ::testing::TestWithParam> { } }; +TEST(CoverageMappingTest, expression_subst) { + CounterExpressionBuilder Builder; + CounterExpressionBuilder::SubstMap MapToExpand; + + auto C = [](unsigned ID) { return Counter::getCounter(ID); }; + auto A = [&](Counter LHS, Counter RHS) { return Builder.add(LHS, RHS); }; + // returns {E, N} in clangCodeGen + auto getBranchCounterPair = [&](Counter E, Counter P, Counter N) { + auto Skipped = Builder.subtract(P, E); + MapToExpand[N] = Builder.subst(Skipped, MapToExpand); + }; + + auto E18 = C(5); + auto P18 = C(2); + auto S18 = C(18); + // #18 => (#2 - #5) + getBranchCounterPair(E18, P18, S18); + + auto E22 = S18; + auto P22 = C(0); + auto S22 = C(22); + // #22 => #0 - (#2 - #5) + getBranchCounterPair(E22, P22, S22); + + auto E28 = A(A(C(9), C(11)), C(14)); + auto P28 = S22; + auto S28 = C(28); + // #28 => (((((#0 + #5) - #2) - #9) - #11) - #14) + getBranchCounterPair(E28, P28, S28); + + auto LHS = A(E28, A(S28, S18)); + auto RHS = C(0); + + // W/o subst, LHS cannot be reduced. + ASSERT_FALSE(Builder.subtract(LHS, RHS).isZero()); + // W/ subst, C(18) and C(28) in LHS will be reduced. + ASSERT_TRUE(Builder.subst(Builder.subtract(LHS, RHS), MapToExpand).isZero()); +} + TEST_P(CoverageMappingTest, basic_write_read) { startFunction("func", 0x1234); addCMR(Counter::getCounter(0), "foo", 1, 1, 1, 1); From d92a9d9c74814fc48c1ff5523d5215115021ac40 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Mon, 6 Jan 2025 23:38:11 +0900 Subject: [PATCH 24/34] Prune redundant logic --- llvm/lib/ProfileData/Coverage/CoverageMapping.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index 3f89414bdbbb9..87d305fc53636 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -157,10 +157,6 @@ Counter CounterExpressionBuilder::subst(Counter C, const SubstMap &Map) { break; } - // Reconfirm if the reconstructed expression would hit the Map. - if (auto I = Map.find(C); I != Map.end()) - return I->second; - return C; } From 82f2e92642fc00e0c07ed3c6506769319c39609a Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Tue, 7 Jan 2025 00:06:11 +0900 Subject: [PATCH 25/34] Expand RHS of MapToExpand. This will prevent recursion. --- clang/lib/CodeGen/CoverageMappingGen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 0764345af0b32..a00166881caaa 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -961,7 +961,7 @@ struct CounterCoverageMappingBuilder // Replace an expression (ParentCnt - ExecCnt) with SkipCnt. Counter SkipCnt = Counter::getCounter(TheMap.Skipped); - MapToExpand[SkipCnt] = Counters.Skipped; + MapToExpand[SkipCnt] = Builder.subst(Counters.Skipped, MapToExpand); Counters.Skipped = SkipCnt; return Counters; } From b90fdf61748ad0d585dc48987e5545878da98c59 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Tue, 7 Jan 2025 00:06:11 +0900 Subject: [PATCH 26/34] Append an explanation in the comment --- clang/lib/CodeGen/CoverageMappingGen.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index a00166881caaa..bd495f84793f6 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -990,6 +990,14 @@ struct CounterCoverageMappingBuilder return true; // Try comaparison with pre-replaced expressions. + // + // For example, getBranchCounterPair(#0) returns {#1, #0 - #1}. + // The sum of the pair should be equivalent to the Parent, #0. + // OTOH when (#0 - #1) is replaced with the new counter #2, + // The sum is (#1 + #2). If the reverse substitution #2 => (#0 - #1) + // can be applied, the sum can be transformed to (#1 + (#0 - #1)). + // To apply substitutions to both hand expressions, transform (LHS - RHS) + // and check isZero. if (Builder.subst(Builder.subtract(OutCount, ParentCount), MapToExpand) .isZero()) return true; From d854fb123be0449e83093f11e9f826c9baa3b766 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 8 Jan 2025 08:32:58 +0900 Subject: [PATCH 27/34] Rewind switch DefaultCase. (to #113112) --- clang/lib/CodeGen/CoverageMappingGen.cpp | 26 ++++++++---------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index df29c2cb4ee17..d1968c5861c95 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -948,20 +948,6 @@ struct CounterCoverageMappingBuilder return {ExecCnt, Builder.subtract(ParentCnt, ExecCnt)}; } - /// Returns {TrueCnt,FalseCnt} for "implicit default". - /// FalseCnt is considered as the False count on SwitchStmt. - std::pair - getSwitchImplicitDefaultCounterPair(const Stmt *Cond, Counter ParentCount, - Counter CaseCountSum) { - // Simplify is skipped while building the counters above: it can get - // really slow on top of switches with thousands of cases. Instead, - // trigger simplification by adding zero to the last counter. - CaseCountSum = - addCounters(CaseCountSum, Counter::getZero(), /*Simplify=*/true); - - return {CaseCountSum, Builder.subtract(ParentCount, CaseCountSum)}; - } - bool IsCounterEqual(Counter OutCount, Counter ParentCount) { if (OutCount == ParentCount) return true; @@ -1920,9 +1906,15 @@ struct CounterCoverageMappingBuilder // the hidden branch, which will be added later by the CodeGen. This region // will be associated with the switch statement's condition. if (!HasDefaultCase) { - auto Counters = getSwitchImplicitDefaultCounterPair( - S->getCond(), ParentCount, CaseCountSum); - createBranchRegion(S->getCond(), Counters.first, Counters.second); + // Simplify is skipped while building the counters above: it can get + // really slow on top of switches with thousands of cases. Instead, + // trigger simplification by adding zero to the last counter. + CaseCountSum = + addCounters(CaseCountSum, Counter::getZero(), /*Simplify=*/true); + + // This is considered as the False count on SwitchStmt. + Counter SwitchFalse = subtractCounters(ParentCount, CaseCountSum); + createBranchRegion(S->getCond(), CaseCountSum, SwitchFalse); } } From bac29679dcc3d1e2f4b01270d8b1abb0858c549e Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 8 Jan 2025 08:42:38 +0900 Subject: [PATCH 28/34] Enable addCounters --- clang/lib/CodeGen/CoverageMappingGen.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index d1968c5861c95..a973bcbb3f765 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); } From 6bae87d6193a88ab093ab472ff4a2d27f9e5e288 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 8 Jan 2025 09:05:46 +0900 Subject: [PATCH 29/34] Get rid of structual bindings --- clang/lib/CodeGen/CoverageMappingGen.cpp | 41 +++++++++++++----------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index a973bcbb3f765..c03eb6a55af5e 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -1601,12 +1601,13 @@ struct CounterCoverageMappingBuilder llvm::EnableSingleByteCoverage ? getRegionCounter(S->getCond()) : addCounters(ParentCount, BackedgeCount, BC.ContinueCount); - auto [ExecCount, ExitCount] = + auto BranchCount = (llvm::EnableSingleByteCoverage ? BranchCounterPair{getRegionCounter(S), Counter::getZero()} : getBranchCounterPair(S, CondCount)); if (!llvm::EnableSingleByteCoverage) { - assert(ExecCount.isZero() || ExecCount == BodyCount); + assert(BranchCount.Executed.isZero() || + BranchCount.Executed == BodyCount); } propagateCounts(CondCount, S->getCond()); adjustForOutOfOrderTraversal(getEnd(S)); @@ -1618,7 +1619,7 @@ struct CounterCoverageMappingBuilder Counter OutCount = llvm::EnableSingleByteCoverage ? getRegionCounter(S) - : addCounters(BC.BreakCount, ExitCount); + : addCounters(BC.BreakCount, BranchCount.Skipped); if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); @@ -1629,7 +1630,7 @@ struct CounterCoverageMappingBuilder // Create Branch Region around condition. if (!llvm::EnableSingleByteCoverage) - createBranchRegion(S->getCond(), BodyCount, ExitCount); + createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped); } void VisitDoStmt(const DoStmt *S) { @@ -1658,18 +1659,19 @@ struct CounterCoverageMappingBuilder Counter CondCount = llvm::EnableSingleByteCoverage ? getRegionCounter(S->getCond()) : addCounters(BackedgeCount, BC.ContinueCount); - auto [ExecCount, ExitCount] = + auto BranchCount = (llvm::EnableSingleByteCoverage ? BranchCounterPair{getRegionCounter(S), Counter::getZero()} : getBranchCounterPair(S, CondCount)); if (!llvm::EnableSingleByteCoverage) { - assert(ExecCount.isZero() || ExecCount == BodyCount); + assert(BranchCount.Executed.isZero() || + BranchCount.Executed == BodyCount); } propagateCounts(CondCount, S->getCond()); Counter OutCount = llvm::EnableSingleByteCoverage ? getRegionCounter(S) - : addCounters(BC.BreakCount, ExitCount); + : addCounters(BC.BreakCount, BranchCount.Skipped); if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; @@ -1677,7 +1679,7 @@ struct CounterCoverageMappingBuilder // Create Branch Region around condition. if (!llvm::EnableSingleByteCoverage) - createBranchRegion(S->getCond(), BodyCount, ExitCount); + createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped); if (BodyHasTerminateStmt) HasTerminateStmt = true; @@ -1726,12 +1728,13 @@ struct CounterCoverageMappingBuilder : addCounters( addCounters(ParentCount, BackedgeCount, BodyBC.ContinueCount), IncrementBC.ContinueCount); - auto [ExecCount, ExitCount] = + auto BranchCount = (llvm::EnableSingleByteCoverage ? BranchCounterPair{getRegionCounter(S), Counter::getZero()} : getBranchCounterPair(S, CondCount)); if (!llvm::EnableSingleByteCoverage) { - assert(ExecCount.isZero() || ExecCount == BodyCount); + assert(BranchCount.Executed.isZero() || + BranchCount.Executed == BodyCount); } if (const Expr *Cond = S->getCond()) { @@ -1747,7 +1750,8 @@ struct CounterCoverageMappingBuilder Counter OutCount = llvm::EnableSingleByteCoverage ? getRegionCounter(S) - : addCounters(BodyBC.BreakCount, IncrementBC.BreakCount, ExitCount); + : addCounters(BodyBC.BreakCount, IncrementBC.BreakCount, + BranchCount.Skipped); if (!IsCounterEqual(OutCount, ParentCount)) { pushRegion(OutCount); GapRegionCounter = OutCount; @@ -1757,7 +1761,7 @@ struct CounterCoverageMappingBuilder // Create Branch Region around condition. if (!llvm::EnableSingleByteCoverage) - createBranchRegion(S->getCond(), BodyCount, ExitCount); + createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped); } void VisitCXXForRangeStmt(const CXXForRangeStmt *S) { @@ -1792,9 +1796,10 @@ struct CounterCoverageMappingBuilder OutCount = getRegionCounter(S); else { LoopCount = addCounters(ParentCount, BackedgeCount, BC.ContinueCount); - auto [ExecCount, SkipCount] = getBranchCounterPair(S, LoopCount); - ExitCount = SkipCount; - assert(ExecCount.isZero() || ExecCount == BodyCount); + auto BranchCount = getBranchCounterPair(S, LoopCount); + ExitCount = BranchCount.Skipped; + assert(BranchCount.Executed.isZero() || + BranchCount.Executed == BodyCount); OutCount = addCounters(BC.BreakCount, ExitCount); } if (!IsCounterEqual(OutCount, ParentCount)) { @@ -1828,9 +1833,9 @@ struct CounterCoverageMappingBuilder Counter LoopCount = addCounters(ParentCount, BackedgeCount, BC.ContinueCount); - auto [ExecCount, ExitCount] = getBranchCounterPair(S, LoopCount); - assert(ExecCount.isZero() || ExecCount == BodyCount); - Counter OutCount = addCounters(BC.BreakCount, ExitCount); + 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; From c8edf58d644e80281e00577920125fa1d7618f47 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 8 Jan 2025 09:22:58 +0900 Subject: [PATCH 30/34] Flatten with getBranchCounterPair(SkipCntForOld) --- clang/lib/CodeGen/CoverageMappingGen.cpp | 95 +++++++++++------------- 1 file changed, 44 insertions(+), 51 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index c03eb6a55af5e..0495a60e53c24 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -939,8 +939,17 @@ struct CounterCoverageMappingBuilder Counter Skipped; }; - BranchCounterPair getBranchCounterPair(const Stmt *S, Counter ParentCnt) { + BranchCounterPair + getBranchCounterPair(const Stmt *S, Counter ParentCnt, + std::optional SkipCntForOld = std::nullopt) { Counter ExecCnt = getRegionCounter(S); + + // The old behavior of SingleByte shouldn't emit Branches. + if (llvm::EnableSingleByteCoverage) { + assert(SkipCntForOld); + return {ExecCnt, *SkipCntForOld}; + } + return {ExecCnt, Builder.subtract(ParentCnt, ExecCnt)}; } @@ -1601,14 +1610,10 @@ struct CounterCoverageMappingBuilder llvm::EnableSingleByteCoverage ? getRegionCounter(S->getCond()) : addCounters(ParentCount, BackedgeCount, BC.ContinueCount); - auto BranchCount = - (llvm::EnableSingleByteCoverage - ? BranchCounterPair{getRegionCounter(S), Counter::getZero()} - : getBranchCounterPair(S, CondCount)); - if (!llvm::EnableSingleByteCoverage) { - assert(BranchCount.Executed.isZero() || - BranchCount.Executed == BodyCount); - } + auto BranchCount = getBranchCounterPair(S, CondCount, getRegionCounter(S)); + assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount || + llvm::EnableSingleByteCoverage); + propagateCounts(CondCount, S->getCond()); adjustForOutOfOrderTraversal(getEnd(S)); @@ -1617,10 +1622,10 @@ struct CounterCoverageMappingBuilder if (Gap) fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount); - Counter OutCount = llvm::EnableSingleByteCoverage - ? getRegionCounter(S) - : addCounters(BC.BreakCount, BranchCount.Skipped); - + 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; @@ -1659,19 +1664,16 @@ struct CounterCoverageMappingBuilder Counter CondCount = llvm::EnableSingleByteCoverage ? getRegionCounter(S->getCond()) : addCounters(BackedgeCount, BC.ContinueCount); - auto BranchCount = - (llvm::EnableSingleByteCoverage - ? BranchCounterPair{getRegionCounter(S), Counter::getZero()} - : getBranchCounterPair(S, CondCount)); - if (!llvm::EnableSingleByteCoverage) { - assert(BranchCount.Executed.isZero() || - BranchCount.Executed == BodyCount); - } + 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, BranchCount.Skipped); + 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; @@ -1728,14 +1730,9 @@ struct CounterCoverageMappingBuilder : addCounters( addCounters(ParentCount, BackedgeCount, BodyBC.ContinueCount), IncrementBC.ContinueCount); - auto BranchCount = - (llvm::EnableSingleByteCoverage - ? BranchCounterPair{getRegionCounter(S), Counter::getZero()} - : getBranchCounterPair(S, CondCount)); - if (!llvm::EnableSingleByteCoverage) { - assert(BranchCount.Executed.isZero() || - BranchCount.Executed == BodyCount); - } + 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); @@ -1747,11 +1744,10 @@ struct CounterCoverageMappingBuilder if (Gap) fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount); - Counter OutCount = - llvm::EnableSingleByteCoverage - ? getRegionCounter(S) - : addCounters(BodyBC.BreakCount, IncrementBC.BreakCount, - BranchCount.Skipped); + 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; @@ -1789,19 +1785,16 @@ struct CounterCoverageMappingBuilder if (Gap) 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); - auto BranchCount = getBranchCounterPair(S, LoopCount); - ExitCount = BranchCount.Skipped; - assert(BranchCount.Executed.isZero() || - BranchCount.Executed == BodyCount); - OutCount = addCounters(BC.BreakCount, ExitCount); - } + 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; @@ -1811,7 +1804,7 @@ struct CounterCoverageMappingBuilder // Create Branch Region around condition. if (!llvm::EnableSingleByteCoverage) - createBranchRegion(S->getCond(), BodyCount, ExitCount); + createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped); } void VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S) { From f2ba2192053cd335f8f848071b1e08fc6f071808 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 8 Jan 2025 18:10:52 +0900 Subject: [PATCH 31/34] Update comments --- clang/lib/CodeGen/CoverageMappingGen.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 0495a60e53c24..8bc5d73bbd0ce 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -935,16 +935,32 @@ struct CounterCoverageMappingBuilder } struct BranchCounterPair { - Counter Executed; - Counter Skipped; + 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 SkipCntForOld = std::nullopt) { Counter ExecCnt = getRegionCounter(S); - // The old behavior of SingleByte shouldn't emit Branches. + // The old behavior of SingleByte is unaware of Branches. if (llvm::EnableSingleByteCoverage) { assert(SkipCntForOld); return {ExecCnt, *SkipCntForOld}; From 97015cb5e015a517d83bf67e3e8f65310f51bc55 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 8 Jan 2025 18:11:02 +0900 Subject: [PATCH 32/34] Decorate the mock --- clang/lib/CodeGen/CoverageMappingGen.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 8bc5d73bbd0ce..72c027fa8e566 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -962,7 +962,8 @@ struct CounterCoverageMappingBuilder // The old behavior of SingleByte is unaware of Branches. if (llvm::EnableSingleByteCoverage) { - assert(SkipCntForOld); + assert(SkipCntForOld && + "SingleByte must provide SkipCntForOld as a fake Skipped count."); return {ExecCnt, *SkipCntForOld}; } From 9a40d20d0c468f7da49d156fd4c7752e9902033b Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Thu, 9 Jan 2025 14:11:45 +0900 Subject: [PATCH 33/34] Will be pruned after the migration of SingleByte. --- clang/lib/CodeGen/CoverageMappingGen.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 72c027fa8e566..dfffa12b639f2 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -961,6 +961,7 @@ struct CounterCoverageMappingBuilder 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."); From b548e71a3cf55167d5b49e010cc76c8c82f29280 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Thu, 9 Jan 2025 15:19:00 +0900 Subject: [PATCH 34/34] Add comments --- llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h index dad5f1276f27c..edae71491191e 100644 --- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h +++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h @@ -215,9 +215,12 @@ class CounterExpressionBuilder { /// LHS. Counter subtract(Counter LHS, Counter RHS, bool Simplify = true); + /// K to V map. K will be Counter in most cases. V may be Counter or + /// Expression. using SubstMap = std::map; - /// Return a counter for each term in the expression replaced by SubstMap. + /// \return A counter equivalent to \C, with each term in its + /// expression replaced with term from \p Map. Counter subst(Counter C, const SubstMap &Map); };