https://github.com/chapuni created https://github.com/llvm/llvm-project/pull/120930
`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. https://discourse.llvm.org/t/rfc-integrating-singlebytecoverage-with-branch-coverage/82492 Depends on: #112698 #112702 #112724 >From 5e460594c8a2550c38c759b2e6f1c5dc4152f820 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Thu, 17 Oct 2024 22:15:12 +0900 Subject: [PATCH 1/6] [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 89ac3b342d0a7c..cb1192bf6e11fe 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 069469e3de856b..aefd53e12088b4 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<bool, bool> 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 83f35785e5327d..8b769dd88d7f1e 100644 --- a/clang/lib/CodeGen/CodeGenPGO.h +++ b/clang/lib/CodeGen/CodeGenPGO.h @@ -112,6 +112,7 @@ class CodeGenPGO { public: std::pair<bool, bool> 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 a5d83e7a743bbd..0bcbd20593ae22 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<const Stmt *, CounterPair> &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<Counter, Counter> 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<const Stmt *, CounterPair> &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 b50f025d261e13..fc7f36c8599f54 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 be516faa39e1152d637ac425229f8a88480ba41b Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Sun, 20 Oct 2024 11:57:03 +0900 Subject: [PATCH 2/6] `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 aefd53e12088b4..0f2090da47a374 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 a4608854e1b727817db3cc01234f97e78285f8c7 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Sun, 27 Oct 2024 20:40:44 +0900 Subject: [PATCH 3/6] 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 d4a1b2613eab92..cb861e61d32727 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 <geek4ci...@gmail.com> Date: Mon, 28 Oct 2024 00:55:49 +0900 Subject: [PATCH 4/6] 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 cb861e61d32727..8584f58548eae0 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 c0785e91103db81b53e000834ce748d8d448e5ef Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Sat, 21 Dec 2024 21:31:20 +0900 Subject: [PATCH 5/6] 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 238235f72d731a..c66280c98e80d9 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -964,6 +964,10 @@ struct CounterCoverageMappingBuilder std::pair<Counter, Counter> 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 <geek4ci...@gmail.com> Date: Mon, 23 Dec 2024 12:26:29 +0900 Subject: [PATCH 6/6] 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 63be03f2318418..b2f40874653083 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)); } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
