[llvm-branch-commits] [clang] [Coverage][Single] Enable Branch coverage for loop statements (PR #113109)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Oct 20 17:37:24 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: NAKAMURA Takumi (chapuni)
<details>
<summary>Changes</summary>
---
Patch is 29.51 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/113109.diff
5 Files Affected:
- (modified) clang/lib/CodeGen/CGStmt.cpp (+25-57)
- (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+2-9)
- (modified) clang/lib/CodeGen/CodeGenPGO.cpp (+4-75)
- (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+37-93)
- (modified) clang/test/CoverageMapping/single-byte-counters.cpp (+29-24)
``````````diff
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index dbc1ce9bf993cd..7d778ce58a1487 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -1039,15 +1039,11 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
SourceLocToDebugLoc(R.getEnd()),
checkIfLoopMustProgress(S.getCond(), hasEmptyLoopBody(S)));
- // When single byte coverage mode is enabled, add a counter to loop condition.
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(S.getCond());
-
// As long as the condition is true, go to the loop body.
llvm::BasicBlock *LoopBody = createBasicBlock("while.body");
if (EmitBoolCondBranch) {
llvm::BasicBlock *ExitBlock = LoopExit.getBlock();
- if (ConditionScope.requiresCleanups())
+ if (getIsCounterPair(&S).second || ConditionScope.requiresCleanups())
ExitBlock = createBasicBlock("while.exit");
llvm::MDNode *Weights =
createProfileWeightsForLoop(S.getCond(), getProfileCount(S.getBody()));
@@ -1058,6 +1054,7 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
if (ExitBlock != LoopExit.getBlock()) {
EmitBlock(ExitBlock);
+ incrementProfileCounter(true, &S);
EmitBranchThroughCleanup(LoopExit);
}
} else if (const Attr *A = Stmt::getLikelihoodAttr(S.getBody())) {
@@ -1075,11 +1072,7 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
{
RunCleanupsScope BodyScope(*this);
EmitBlock(LoopBody);
- // When single byte coverage mode is enabled, add a counter to the body.
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(S.getBody());
- else
- incrementProfileCounter(&S);
+ incrementProfileCounter(false, &S);
EmitStmt(S.getBody());
}
@@ -1099,13 +1092,10 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
// The LoopHeader typically is just a branch if we skipped emitting
// a branch, try to erase it.
- if (!EmitBoolCondBranch)
+ if (!EmitBoolCondBranch) {
SimplifyForwardingBlocks(LoopHeader.getBlock());
-
- // When single byte coverage mode is enabled, add a counter to continuation
- // block.
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(&S);
+ PGO.markStmtAsUsed(true, &S);
+ }
if (CGM.shouldEmitConvergenceTokens())
ConvergenceTokenStack.pop_back();
@@ -1124,10 +1114,7 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S,
// Emit the body of the loop.
llvm::BasicBlock *LoopBody = createBasicBlock("do.body");
- if (llvm::EnableSingleByteCoverage)
- EmitBlockWithFallThrough(LoopBody, S.getBody());
- else
- EmitBlockWithFallThrough(LoopBody, &S);
+ EmitBlockWithFallThrough(LoopBody, &S);
if (CGM.shouldEmitConvergenceTokens())
ConvergenceTokenStack.push_back(
@@ -1139,9 +1126,6 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S,
}
EmitBlock(LoopCond.getBlock());
- // When single byte coverage mode is enabled, add a counter to loop condition.
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(S.getCond());
// C99 6.8.5.2: "The evaluation of the controlling expression takes place
// after each execution of the loop body."
@@ -1164,16 +1148,25 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S,
SourceLocToDebugLoc(R.getEnd()),
checkIfLoopMustProgress(S.getCond(), hasEmptyLoopBody(S)));
+ auto *LoopFalse =
+ (getIsCounterPair(&S).second ? createBasicBlock("do.loopfalse")
+ : LoopExit.getBlock());
+
// As long as the condition is true, iterate the loop.
if (EmitBoolCondBranch) {
uint64_t BackedgeCount = getProfileCount(S.getBody()) - ParentCount;
Builder.CreateCondBr(
- BoolCondVal, LoopBody, LoopExit.getBlock(),
+ BoolCondVal, LoopBody, LoopFalse,
createProfileWeightsForLoop(S.getCond(), BackedgeCount));
}
LoopStack.pop();
+ if (LoopFalse != LoopExit.getBlock()) {
+ EmitBlock(LoopFalse);
+ incrementProfileCounter(true, &S, true);
+ }
+
// Emit the exit block.
EmitBlock(LoopExit.getBlock());
@@ -1182,11 +1175,6 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S,
if (!EmitBoolCondBranch)
SimplifyForwardingBlocks(LoopCond.getBlock());
- // When single byte coverage mode is enabled, add a counter to continuation
- // block.
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(&S);
-
if (CGM.shouldEmitConvergenceTokens())
ConvergenceTokenStack.pop_back();
}
@@ -1247,15 +1235,10 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
BreakContinueStack.back().ContinueBlock = Continue;
}
- // When single byte coverage mode is enabled, add a counter to loop
- // condition.
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(S.getCond());
-
llvm::BasicBlock *ExitBlock = LoopExit.getBlock();
// If there are any cleanups between here and the loop-exit scope,
// create a block to stage a loop exit along.
- if (ForScope.requiresCleanups())
+ if (getIsCounterPair(&S).second || ForScope.requiresCleanups())
ExitBlock = createBasicBlock("for.cond.cleanup");
// As long as the condition is true, iterate the loop.
@@ -1274,6 +1257,7 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
if (ExitBlock != LoopExit.getBlock()) {
EmitBlock(ExitBlock);
+ incrementProfileCounter(true, &S);
EmitBranchThroughCleanup(LoopExit);
}
@@ -1281,13 +1265,11 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
} else {
// Treat it as a non-zero constant. Don't even create a new block for the
// body, just fall into it.
+ PGO.markStmtAsUsed(true, &S);
}
- // When single byte coverage mode is enabled, add a counter to the body.
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(S.getBody());
- else
- incrementProfileCounter(&S);
+ incrementProfileCounter(false, &S);
+
{
// Create a separate cleanup scope for the body, in case it is not
// a compound statement.
@@ -1299,8 +1281,6 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
if (S.getInc()) {
EmitBlock(Continue.getBlock());
EmitStmt(S.getInc());
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(S.getInc());
}
BreakContinueStack.pop_back();
@@ -1317,11 +1297,6 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
// Emit the fall-through block.
EmitBlock(LoopExit.getBlock(), true);
- // When single byte coverage mode is enabled, add a counter to continuation
- // block.
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(&S);
-
if (CGM.shouldEmitConvergenceTokens())
ConvergenceTokenStack.pop_back();
}
@@ -1358,7 +1333,7 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
// If there are any cleanups between here and the loop-exit scope,
// create a block to stage a loop exit along.
llvm::BasicBlock *ExitBlock = LoopExit.getBlock();
- if (ForScope.requiresCleanups())
+ if (getIsCounterPair(&S).second || ForScope.requiresCleanups())
ExitBlock = createBasicBlock("for.cond.cleanup");
// The loop body, consisting of the specified body and the loop variable.
@@ -1376,14 +1351,12 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
if (ExitBlock != LoopExit.getBlock()) {
EmitBlock(ExitBlock);
+ incrementProfileCounter(true, &S);
EmitBranchThroughCleanup(LoopExit);
}
EmitBlock(ForBody);
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(S.getBody());
- else
- incrementProfileCounter(&S);
+ incrementProfileCounter(false, &S);
// Create a block for the increment. In case of a 'continue', we jump there.
JumpDest Continue = getJumpDestInCurrentScope("for.inc");
@@ -1414,11 +1387,6 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
// Emit the fall-through block.
EmitBlock(LoopExit.getBlock(), true);
- // When single byte coverage mode is enabled, add a counter to continuation
- // block.
- if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(&S);
-
if (CGM.shouldEmitConvergenceTokens())
ConvergenceTokenStack.pop_back();
}
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index df15d09276c2fb..848f8d1a69c107 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -53,10 +53,6 @@
using namespace clang;
using namespace CodeGen;
-namespace llvm {
-extern cl::opt<bool> EnableSingleByteCoverage;
-} // namespace llvm
-
/// shouldEmitLifetimeMarkers - Decide whether we need emit the life-time
/// markers.
static bool shouldEmitLifetimeMarkers(const CodeGenOptions &CGOpts,
@@ -1361,10 +1357,7 @@ void CodeGenFunction::EmitFunctionBody(const Stmt *Body) {
void CodeGenFunction::EmitBlockWithFallThrough(llvm::BasicBlock *BB,
const Stmt *S) {
llvm::BasicBlock *SkipCountBB = nullptr;
- // Do not skip over the instrumentation when single byte coverage mode is
- // enabled.
- if (HaveInsertPoint() && CGM.getCodeGenOpts().hasProfileClangInstr() &&
- !llvm::EnableSingleByteCoverage) {
+ if (HaveInsertPoint() && CGM.getCodeGenOpts().hasProfileClangInstr()) {
// When instrumenting for profiling, the fallthrough to certain
// statements needs to skip over the instrumentation code so that we
// get an accurate count.
@@ -1373,7 +1366,7 @@ void CodeGenFunction::EmitBlockWithFallThrough(llvm::BasicBlock *BB,
}
EmitBlock(BB);
uint64_t CurrentCount = getCurrentProfileCount();
- incrementProfileCounter(S);
+ incrementProfileCounter(false, S);
setCurrentProfileCount(getCurrentProfileCount() + CurrentCount);
if (SkipCountBB)
EmitBlock(SkipCountBB);
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 0f2090da47a374..6020a611d1a576 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -394,81 +394,6 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
return true;
}
- bool TraverseWhileStmt(WhileStmt *While) {
- // When single byte coverage mode is enabled, add a counter to condition and
- // body.
- bool NoSingleByteCoverage = !llvm::EnableSingleByteCoverage;
- for (Stmt *CS : While->children()) {
- if (!CS || NoSingleByteCoverage)
- continue;
- if (CS == While->getCond())
- CounterMap[While->getCond()] = NextCounter++;
- else if (CS == While->getBody())
- CounterMap[While->getBody()] = NextCounter++;
- }
-
- Base::TraverseWhileStmt(While);
- if (Hash.getHashVersion() != PGO_HASH_V1)
- Hash.combine(PGOHash::EndOfScope);
- return true;
- }
-
- bool TraverseDoStmt(DoStmt *Do) {
- // When single byte coverage mode is enabled, add a counter to condition and
- // body.
- bool NoSingleByteCoverage = !llvm::EnableSingleByteCoverage;
- for (Stmt *CS : Do->children()) {
- if (!CS || NoSingleByteCoverage)
- continue;
- if (CS == Do->getCond())
- CounterMap[Do->getCond()] = NextCounter++;
- else if (CS == Do->getBody())
- CounterMap[Do->getBody()] = NextCounter++;
- }
-
- Base::TraverseDoStmt(Do);
- if (Hash.getHashVersion() != PGO_HASH_V1)
- Hash.combine(PGOHash::EndOfScope);
- return true;
- }
-
- bool TraverseForStmt(ForStmt *For) {
- // When single byte coverage mode is enabled, add a counter to condition,
- // increment and body.
- bool NoSingleByteCoverage = !llvm::EnableSingleByteCoverage;
- for (Stmt *CS : For->children()) {
- if (!CS || NoSingleByteCoverage)
- continue;
- if (CS == For->getCond())
- CounterMap[For->getCond()] = NextCounter++;
- else if (CS == For->getInc())
- CounterMap[For->getInc()] = NextCounter++;
- else if (CS == For->getBody())
- CounterMap[For->getBody()] = NextCounter++;
- }
-
- Base::TraverseForStmt(For);
- if (Hash.getHashVersion() != PGO_HASH_V1)
- Hash.combine(PGOHash::EndOfScope);
- return true;
- }
-
- bool TraverseCXXForRangeStmt(CXXForRangeStmt *ForRange) {
- // When single byte coverage mode is enabled, add a counter to body.
- bool NoSingleByteCoverage = !llvm::EnableSingleByteCoverage;
- for (Stmt *CS : ForRange->children()) {
- if (!CS || NoSingleByteCoverage)
- continue;
- if (CS == ForRange->getBody())
- CounterMap[ForRange->getBody()] = NextCounter++;
- }
-
- Base::TraverseCXXForRangeStmt(ForRange);
- if (Hash.getHashVersion() != PGO_HASH_V1)
- Hash.combine(PGOHash::EndOfScope);
- return true;
- }
-
// If the statement type \p N is nestable, and its nesting impacts profile
// stability, define a custom traversal which tracks the end of the statement
// in the hash (provided we're not using the V1 hash).
@@ -480,6 +405,10 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
return true; \
}
+ DEFINE_NESTABLE_TRAVERSAL(WhileStmt)
+ DEFINE_NESTABLE_TRAVERSAL(DoStmt)
+ DEFINE_NESTABLE_TRAVERSAL(ForStmt)
+ DEFINE_NESTABLE_TRAVERSAL(CXXForRangeStmt)
DEFINE_NESTABLE_TRAVERSAL(ObjCForCollectionStmt)
DEFINE_NESTABLE_TRAVERSAL(CXXTryStmt)
DEFINE_NESTABLE_TRAVERSAL(CXXCatchStmt)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index a331d5bc68286b..4062c531d0b797 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1570,9 +1570,8 @@ struct CounterCoverageMappingBuilder
void VisitBreakStmt(const BreakStmt *S) {
assert(!BreakContinueStack.empty() && "break not in a loop or switch!");
- if (!llvm::EnableSingleByteCoverage)
- BreakContinueStack.back().BreakCount = addCounters(
- BreakContinueStack.back().BreakCount, getRegion().getCounter());
+ BreakContinueStack.back().BreakCount = addCounters(
+ BreakContinueStack.back().BreakCount, getRegion().getCounter());
// FIXME: a break in a switch should terminate regions for all preceding
// case statements, not just the most recent one.
terminateRegion(S);
@@ -1580,9 +1579,8 @@ struct CounterCoverageMappingBuilder
void VisitContinueStmt(const ContinueStmt *S) {
assert(!BreakContinueStack.empty() && "continue stmt not in a loop!");
- if (!llvm::EnableSingleByteCoverage)
- BreakContinueStack.back().ContinueCount = addCounters(
- BreakContinueStack.back().ContinueCount, getRegion().getCounter());
+ BreakContinueStack.back().ContinueCount = addCounters(
+ BreakContinueStack.back().ContinueCount, getRegion().getCounter());
terminateRegion(S);
}
@@ -1600,9 +1598,7 @@ struct CounterCoverageMappingBuilder
extendRegion(S);
Counter ParentCount = getRegion().getCounter();
- Counter BodyCount = llvm::EnableSingleByteCoverage
- ? getRegionCounter(S->getBody())
- : getRegionCounter(S);
+ Counter BodyCount = getRegionCounter(S);
// Handle the body first so that we can get the backedge count.
BreakContinueStack.push_back(BreakContinue());
@@ -1615,16 +1611,10 @@ struct CounterCoverageMappingBuilder
// Go back to handle the condition.
Counter CondCount =
- 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);
- }
+ addCounters(ParentCount, BackedgeCount, BC.ContinueCount);
+ auto [ExecCount, ExitCount] = getBranchCounterPair(S, CondCount);
+ assert(ExecCount.isZero() || ExecCount == BodyCount);
+
propagateCounts(CondCount, S->getCond());
adjustForOutOfOrderTraversal(getEnd(S));
@@ -1633,10 +1623,7 @@ struct CounterCoverageMappingBuilder
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount);
- Counter OutCount = llvm::EnableSingleByteCoverage
- ? getRegionCounter(S)
- : addCounters(BC.BreakCount, ExitCount);
-
+ Counter OutCount = addCounters(BC.BreakCount, ExitCount);
if (!IsCounterEqual(OutCount, ParentCount)) {
pushRegion(OutCount);
GapRegionCounter = OutCount;
@@ -1645,56 +1632,40 @@ struct CounterCoverageMappingBuilder
}
// Create Branch Region around condition.
- if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(S->getCond(), BodyCount, ExitCount);
+ createBranchRegion(S->getCond(), BodyCount, ExitCount);
}
void VisitDoStmt(const DoStmt *S) {
extendRegion(S);
Counter ParentCount = getRegion().getCounter();
- Counter BodyCount = llvm::EnableSingleByteCoverage
- ? getRegionCounter(S->getBody())
- : getRegionCounter(S);
+ Counter BodyCount = getRegionCounter(S);
BreakContinueStack.push_back(BreakContinue());
extendRegion(S->getBody());
- Counter BackedgeCount;
- if (llvm::EnableSingleByteCoverage)
- propagateCounts(BodyCount, S->getBody());
- else
- BackedgeCount =
- propagateCounts(addCounters(ParentCount, BodyCount), S->getBody());
+ Counter BackedgeCount =
+ propagateCounts(addCounters(ParentCount, BodyCount), S->getBody());
BreakContinue BC = BreakContinueStack.pop_back_val();
bool BodyHasTerminateStmt = HasTerminateStmt;
HasTerminateStmt = false;
- 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);
- }
+ Counter CondCount = addCounters(BackedgeCount, BC.ContinueCount);
+ auto [ExecCount, ExitCount] = getBranchCounterPair(S, CondCount);
+ assert(ExecCount.isZero() || ExecCount == BodyCount);
+
propagateCounts(CondCount, S->getCond());
- Counter OutCount = llvm::EnableSingleByteCoverage
- ? getRegionCounter(S)
- : addCounters(BC.BreakCount, ExitCount);
+ Counter OutCount = addCounters(BC.BreakCount, ExitCount);
if (!IsCounterEqual(OutCount, ParentCount)) {
pushRegion(OutCount);
GapRegionCounter = OutCount;
}
// Create Branch Region around condition.
- if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(S->getCond(), BodyCount, ExitCount);
+ createBranchRegion(S->getCond(), BodyCount, ExitCount);
if (BodyHasTerminateStmt)
HasTerminateStmt = true;
@@ -1706,9 +1677,7 @@ struct CounterCoverageMappingBuilder
Visit(S->getInit());
Counter ParentCount = getRegion().getCounter();
- Counter BodyCount = llvm::EnableSingleByteCoverage
- ? getRegionCounter(S->getBody())
- : getRegionCounter(S);
+ Counter BodyCount = getRegionCounter(S);
// The loop increment may contain a break or continue.
if (S->getInc())
@@ -1727,29 +1696,16 @@ struct CounterCoverageMappingBuilder
// the count for all the continue statements.
BreakContinue IncrementBC;
if (const Stmt *Inc = S->getInc()) {
- Counter IncCount;
- if (llvm::EnableSingleByteCoverage)
- IncCount = getRegionCounter(S->getInc());
- else
- IncCount = addCounters(BackedgeCount, BodyBC.ContinueCount);
- propagateCounts(IncCount, Inc);
+ propagateCounts(addCounters(BackedgeCount, BodyBC.ContinueCount), Inc);
IncrementBC = BreakContinueStack.pop_back_val();
}
// Go back to handle the condition.
- Counter ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/113109
More information about the llvm-branch-commits
mailing list