[llvm-branch-commits] [clang] [Coverage][Single] Enable Branch coverage for CondOp (PR #113110)
NAKAMURA Takumi via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Oct 27 15:52:24 PDT 2024
https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/113110
>From 744c5b634de08f9214c82d6fcfde7179bc4edfb0 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sun, 20 Oct 2024 14:46:07 +0900
Subject: [PATCH 1/3] [Coverage][Single] Enable Branch coverage for CondOp
---
clang/lib/CodeGen/CGExpr.cpp | 6 +--
clang/lib/CodeGen/CGExprAgg.cpp | 14 +------
clang/lib/CodeGen/CGExprComplex.cpp | 15 +-------
clang/lib/CodeGen/CGExprScalar.cpp | 37 +++----------------
clang/lib/CodeGen/CodeGenFunction.cpp | 3 +-
clang/lib/CodeGen/CodeGenPGO.cpp | 8 ----
clang/lib/CodeGen/CoverageMappingGen.cpp | 16 ++------
.../CoverageMapping/single-byte-counters.cpp | 11 +++---
8 files changed, 25 insertions(+), 85 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index cc85f05ad9f70c..67e3a1de17e679 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5137,8 +5137,7 @@ std::optional<LValue> HandleConditionalOperatorLValueSimpleCase(
if (!CGF.ContainsLabel(Dead)) {
// If the true case is live, we need to track its region.
- if (CondExprBool)
- CGF.incrementProfileCounter(E);
+ CGF.incrementProfileCounter(!CondExprBool, E, true);
CGF.markStmtMaybeUsed(Dead);
// If a throw expression we emit it and return an undefined lvalue
// because it can't be used.
@@ -5177,7 +5176,7 @@ ConditionalInfo EmitConditionalBlocks(CodeGenFunction &CGF,
// Any temporaries created here are conditional.
CGF.EmitBlock(Info.lhsBlock);
- CGF.incrementProfileCounter(E);
+ CGF.incrementProfileCounter(false, E);
eval.begin(CGF);
Info.LHS = BranchGenFunc(CGF, E->getTrueExpr());
eval.end(CGF);
@@ -5188,6 +5187,7 @@ ConditionalInfo EmitConditionalBlocks(CodeGenFunction &CGF,
// Any temporaries created here are conditional.
CGF.EmitBlock(Info.rhsBlock);
+ CGF.incrementProfileCounter(true, E);
eval.begin(CGF);
Info.RHS = BranchGenFunc(CGF, E->getFalseExpr());
eval.end(CGF);
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 2ad6587089f101..0c778ef185532f 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -36,10 +36,6 @@ using namespace CodeGen;
// Aggregate Expression Emitter
//===----------------------------------------------------------------------===//
-namespace llvm {
-extern cl::opt<bool> EnableSingleByteCoverage;
-} // namespace llvm
-
namespace {
class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
CodeGenFunction &CGF;
@@ -1293,10 +1289,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
eval.begin(CGF);
CGF.EmitBlock(LHSBlock);
- if (llvm::EnableSingleByteCoverage)
- CGF.incrementProfileCounter(E->getTrueExpr());
- else
- CGF.incrementProfileCounter(E);
+ CGF.incrementProfileCounter(false, E);
Visit(E->getTrueExpr());
eval.end(CGF);
@@ -1311,8 +1304,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
eval.begin(CGF);
CGF.EmitBlock(RHSBlock);
- if (llvm::EnableSingleByteCoverage)
- CGF.incrementProfileCounter(E->getFalseExpr());
+ CGF.incrementProfileCounter(true, E);
Visit(E->getFalseExpr());
eval.end(CGF);
@@ -1321,8 +1313,6 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
E->getType());
CGF.EmitBlock(ContBlock);
- if (llvm::EnableSingleByteCoverage)
- CGF.incrementProfileCounter(E);
}
void AggExprEmitter::VisitChooseExpr(const ChooseExpr *CE) {
diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp
index fef26e7b4ccdbd..bcece9431de764 100644
--- a/clang/lib/CodeGen/CGExprComplex.cpp
+++ b/clang/lib/CodeGen/CGExprComplex.cpp
@@ -28,10 +28,6 @@ using namespace CodeGen;
// Complex Expression Emitter
//===----------------------------------------------------------------------===//
-namespace llvm {
-extern cl::opt<bool> EnableSingleByteCoverage;
-} // namespace llvm
-
typedef CodeGenFunction::ComplexPairTy ComplexPairTy;
/// Return the complex type that we are meant to emit.
@@ -1381,11 +1377,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
eval.begin(CGF);
CGF.EmitBlock(LHSBlock);
- if (llvm::EnableSingleByteCoverage)
- CGF.incrementProfileCounter(E->getTrueExpr());
- else
- CGF.incrementProfileCounter(E);
-
+ CGF.incrementProfileCounter(false, E);
ComplexPairTy LHS = Visit(E->getTrueExpr());
LHSBlock = Builder.GetInsertBlock();
CGF.EmitBranch(ContBlock);
@@ -1393,13 +1385,10 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
eval.begin(CGF);
CGF.EmitBlock(RHSBlock);
- if (llvm::EnableSingleByteCoverage)
- CGF.incrementProfileCounter(E->getFalseExpr());
+ CGF.incrementProfileCounter(true, E);
ComplexPairTy RHS = Visit(E->getFalseExpr());
RHSBlock = Builder.GetInsertBlock();
CGF.EmitBlock(ContBlock);
- if (llvm::EnableSingleByteCoverage)
- CGF.incrementProfileCounter(E);
eval.end(CGF);
// Create a PHI node for the real part.
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index ca9ab6025128f0..11d4ec8a267605 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -55,10 +55,6 @@ using llvm::Value;
// Scalar Expression Emitter
//===----------------------------------------------------------------------===//
-namespace llvm {
-extern cl::opt<bool> EnableSingleByteCoverage;
-} // namespace llvm
-
namespace {
/// Determine whether the given binary operation may overflow.
@@ -5247,13 +5243,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
// If the dead side doesn't have labels we need, just emit the Live part.
if (!CGF.ContainsLabel(dead)) {
- if (CondExprBool) {
- if (llvm::EnableSingleByteCoverage) {
- CGF.incrementProfileCounter(lhsExpr);
- CGF.incrementProfileCounter(rhsExpr);
- }
- CGF.incrementProfileCounter(E);
- }
+ CGF.incrementProfileCounter(!CondExprBool, E, true);
Value *Result = Visit(live);
CGF.markStmtMaybeUsed(dead);
@@ -5328,17 +5318,13 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
// If this is a really simple expression (like x ? 4 : 5), emit this as a
// select instead of as control flow. We can only do this if it is cheap and
// safe to evaluate the LHS and RHS unconditionally.
- if (isCheapEnoughToEvaluateUnconditionally(lhsExpr, CGF) &&
+ if (!CGF.getIsCounterPair(E).second &&
+ isCheapEnoughToEvaluateUnconditionally(lhsExpr, CGF) &&
isCheapEnoughToEvaluateUnconditionally(rhsExpr, CGF)) {
llvm::Value *CondV = CGF.EvaluateExprAsBool(condExpr);
llvm::Value *StepV = Builder.CreateZExtOrBitCast(CondV, CGF.Int64Ty);
- if (llvm::EnableSingleByteCoverage) {
- CGF.incrementProfileCounter(lhsExpr);
- CGF.incrementProfileCounter(rhsExpr);
- CGF.incrementProfileCounter(E);
- } else
- CGF.incrementProfileCounter(E, StepV);
+ CGF.incrementProfileCounter(E, StepV);
llvm::Value *LHS = Visit(lhsExpr);
llvm::Value *RHS = Visit(rhsExpr);
@@ -5370,11 +5356,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
if (CGF.MCDCLogOpStack.empty())
CGF.maybeUpdateMCDCTestVectorBitmap(condExpr);
- if (llvm::EnableSingleByteCoverage)
- CGF.incrementProfileCounter(lhsExpr);
- else
- CGF.incrementProfileCounter(E);
-
+ CGF.incrementProfileCounter(false, E);
eval.begin(CGF);
Value *LHS = Visit(lhsExpr);
eval.end(CGF);
@@ -5390,9 +5372,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
if (CGF.MCDCLogOpStack.empty())
CGF.maybeUpdateMCDCTestVectorBitmap(condExpr);
- if (llvm::EnableSingleByteCoverage)
- CGF.incrementProfileCounter(rhsExpr);
-
+ CGF.incrementProfileCounter(true, E);
eval.begin(CGF);
Value *RHS = Visit(rhsExpr);
eval.end(CGF);
@@ -5411,11 +5391,6 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
PN->addIncoming(LHS, LHSBlock);
PN->addIncoming(RHS, RHSBlock);
- // When single byte coverage mode is enabled, add a counter to continuation
- // block.
- if (llvm::EnableSingleByteCoverage)
- CGF.incrementProfileCounter(E);
-
return PN;
}
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index df15d09276c2fb..2bcba9bef2628c 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2004,7 +2004,7 @@ void CodeGenFunction::EmitBranchOnBoolExpr(
cond.begin(*this);
EmitBlock(LHSBlock);
- incrementProfileCounter(CondOp);
+ incrementProfileCounter(false, CondOp);
{
ApplyDebugLocation DL(*this, Cond);
EmitBranchOnBoolExpr(CondOp->getLHS(), TrueBlock, FalseBlock,
@@ -2014,6 +2014,7 @@ void CodeGenFunction::EmitBranchOnBoolExpr(
cond.begin(*this);
EmitBlock(RHSBlock);
+ incrementProfileCounter(true, CondOp);
EmitBranchOnBoolExpr(CondOp->getRHS(), TrueBlock, FalseBlock,
TrueCount - LHSScaledTrueCount, LH, CondOp);
cond.end(*this);
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 0f2090da47a374..69f66290979840 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -343,14 +343,6 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
return Base::VisitBinaryOperator(S);
}
- bool VisitConditionalOperator(ConditionalOperator *S) {
- if (llvm::EnableSingleByteCoverage && S->getTrueExpr())
- CounterMap[S->getTrueExpr()] = NextCounter++;
- if (llvm::EnableSingleByteCoverage && S->getFalseExpr())
- CounterMap[S->getFalseExpr()] = NextCounter++;
- return Base::VisitConditionalOperator(S);
- }
-
/// Include \p S in the function hash.
bool VisitStmt(Stmt *S) {
auto Type = updateCounterMappings(S);
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index a331d5bc68286b..77e73992098061 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -2125,11 +2125,7 @@ struct CounterCoverageMappingBuilder
extendRegion(E);
Counter ParentCount = getRegion().getCounter();
- auto [TrueCount, FalseCount] =
- (llvm::EnableSingleByteCoverage
- ? std::make_pair(getRegionCounter(E->getTrueExpr()),
- getRegionCounter(E->getFalseExpr()))
- : getBranchCounterPair(E, ParentCount));
+ auto [TrueCount, FalseCount] = getBranchCounterPair(E, ParentCount);
Counter OutCount;
if (const auto *BCO = dyn_cast<BinaryConditionalOperator>(E)) {
@@ -2148,11 +2144,8 @@ struct CounterCoverageMappingBuilder
}
extendRegion(E->getFalseExpr());
- Counter FalseOutCount = propagateCounts(FalseCount, E->getFalseExpr());
- if (llvm::EnableSingleByteCoverage)
- OutCount = getRegionCounter(E);
- else
- OutCount = addCounters(OutCount, FalseOutCount);
+ OutCount =
+ addCounters(OutCount, propagateCounts(FalseCount, E->getFalseExpr()));
if (!IsCounterEqual(OutCount, ParentCount)) {
pushRegion(OutCount);
@@ -2160,8 +2153,7 @@ struct CounterCoverageMappingBuilder
}
// Create Branch Region around condition.
- if (!llvm::EnableSingleByteCoverage)
- createBranchRegion(E->getCond(), TrueCount, FalseCount);
+ createBranchRegion(E->getCond(), TrueCount, FalseCount);
}
void createOrCancelDecision(const BinaryOperator *E, unsigned Since) {
diff --git a/clang/test/CoverageMapping/single-byte-counters.cpp b/clang/test/CoverageMapping/single-byte-counters.cpp
index d20b695bc2636a..be0454df002bd0 100644
--- a/clang/test/CoverageMapping/single-byte-counters.cpp
+++ b/clang/test/CoverageMapping/single-byte-counters.cpp
@@ -127,10 +127,11 @@ int testDo() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+9]]:2 = [
}
// CHECK-NEXT: testConditional
-int testConditional(int x) { // CHECK-NEXT: File 0, [[@LINE]]:28 -> [[@LINE+6]]:2 = [[C90:#0]]
+int testConditional(int x) { // CHECK-NEXT: File 0, [[@LINE]]:28 -> [[@LINE+7]]:2 = [[C90:#0]]
int result = (x > 0) ? 1 : -1; // CHECK-NEXT: File 0, [[@LINE]]:15 -> [[@LINE]]:22 = [[C90]]
- // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:24 -> [[@LINE-1]]:25 = [[C9T:#2]]
- // CHECK-NEXT: File 0, [[@LINE-2]]:25 -> [[@LINE-2]]:26 = [[C9T]]
- // CHECK-NEXT: File 0, [[@LINE-3]]:29 -> [[@LINE-3]]:31 = [[C9F:#3]]
- return result; // CHECK-NEXT: File 0, [[@LINE]]:2 -> [[@LINE]]:15 = [[C9E:#1]]
+ // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:22 = [[C9T:#1]], [[C9F:#2]]
+ // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:24 -> [[@LINE-2]]:25 = [[C9T]]
+ // CHECK-NEXT: File 0, [[@LINE-3]]:25 -> [[@LINE-3]]:26 = [[C9T]]
+ // CHECK-NEXT: File 0, [[@LINE-4]]:29 -> [[@LINE-4]]:31 = [[C9F]]
+ return result; // #0
}
>From 49e139ec5ac2b79baf27ac756bf0bf24bf053ae7 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Mon, 28 Oct 2024 07:24:19 +0900
Subject: [PATCH 2/3] Suppress StepV in EnableSingleByteCoverage
---
clang/lib/CodeGen/CGExprScalar.cpp | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 11d4ec8a267605..7d858e1d404c8f 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -55,6 +55,10 @@ using llvm::Value;
// Scalar Expression Emitter
//===----------------------------------------------------------------------===//
+namespace llvm {
+extern cl::opt<bool> EnableSingleByteCoverage;
+} // namespace llvm
+
namespace {
/// Determine whether the given binary operation may overflow.
@@ -5318,7 +5322,7 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
// If this is a really simple expression (like x ? 4 : 5), emit this as a
// select instead of as control flow. We can only do this if it is cheap and
// safe to evaluate the LHS and RHS unconditionally.
- if (!CGF.getIsCounterPair(E).second &&
+ if (!llvm::EnableSingleByteCoverage &&
isCheapEnoughToEvaluateUnconditionally(lhsExpr, CGF) &&
isCheapEnoughToEvaluateUnconditionally(rhsExpr, CGF)) {
llvm::Value *CondV = CGF.EvaluateExprAsBool(condExpr);
>From 1f35324eb57614da5420943b96b5b63f11637294 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Mon, 28 Oct 2024 07:44:46 +0900
Subject: [PATCH 3/3] assert(!StepV)
---
clang/lib/CodeGen/CodeGenPGO.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 69f66290979840..b4edc72ce75aab 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -1229,10 +1229,11 @@ void CodeGenPGO::emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S,
NormalizedFuncNameVarPtr, Builder.getInt64(FunctionHash),
Builder.getInt32(NumRegionCounters), Builder.getInt32(Counter), StepV};
- if (llvm::EnableSingleByteCoverage)
+ if (llvm::EnableSingleByteCoverage) {
+ assert(!StepV && "StepV is impossible in SingleByte");
Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::instrprof_cover),
ArrayRef(Args, 4));
- else if (!StepV)
+ } else if (!StepV)
Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::instrprof_increment),
ArrayRef(Args, 4));
else
More information about the llvm-branch-commits
mailing list