[llvm-branch-commits] [clang] [llvm] Coverage] Make additional counters available for BranchRegion. NFC. (PR #120930)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Dec 22 20:30:06 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-pgo
Author: NAKAMURA Takumi (chapuni)
<details>
<summary>Changes</summary>
`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
---
Full diff: https://github.com/llvm/llvm-project/pull/120930.diff
5 Files Affected:
- (modified) clang/lib/CodeGen/CodeGenFunction.h (+7-1)
- (modified) clang/lib/CodeGen/CodeGenPGO.cpp (+30-2)
- (modified) clang/lib/CodeGen/CodeGenPGO.h (+1)
- (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+37-10)
- (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+3)
``````````diff
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 0396fa5bef136b..dfb008548454e6 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1632,11 +1632,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 75191f86d231ce..2e50e47f337fff 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -1137,6 +1137,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);
}
@@ -1192,11 +1205,26 @@ 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) {
+ if (!IsCounter.first)
+ return;
+ 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 e2b4b7cfd760a7..c66280c98e80d9 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -884,6 +884,9 @@ struct CounterCoverageMappingBuilder
/// The map of statements to count values.
llvm::DenseMap<const Stmt *, CounterPair> &CounterMap;
+ CounterExpressionBuilder::SubstMap MapToExpand;
+ unsigned NextCounterNum;
+
MCDC::State &MCDCState;
/// A stack of currently live regions.
@@ -919,15 +922,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);
}
@@ -940,8 +939,24 @@ 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 || !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)
+ TheMap.second = NextCounterNum++;
+
+ Counter SkipCnt = Counter::getCounter(TheMap.second);
+ MapToExpand[SkipCnt] = SkipExpr;
+ return {ExecCnt, SkipCnt};
}
/// Returns {TrueCnt,FalseCnt} for "implicit default".
@@ -949,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.
@@ -962,6 +981,11 @@ struct CounterCoverageMappingBuilder
if (OutCount == ParentCount)
return true;
+ // Try comaparison with pre-replaced expressions.
+ if (Builder.subst(Builder.subtract(OutCount, ParentCount), MapToExpand)
+ .isZero())
+ return true;
+
return false;
}
@@ -1175,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;
}
@@ -1447,7 +1473,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 3f89414bdbbb99..b2f40874653083 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -637,6 +637,9 @@ 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.isBranch())
+ MaxCounterID =
+ std::max(MaxCounterID, Ctx.getMaxCounterID(Region.FalseCount));
}
return MaxCounterID;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/120930
More information about the llvm-branch-commits
mailing list