[llvm-branch-commits] [clang] [llvm] [Coverage][Single] Enable Branch coverage for SwitchStmt (PR #113112)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/113112
>From ec05cc37e1177f06c9a44a1e39dadc9306cc5c68 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Mon, 21 Oct 2024 08:09:31 +0900
Subject: [PATCH 1/2] [Coverage][Single] Enable Branch coverage for SwitchStmt
---
clang/lib/CodeGen/CGStmt.cpp | 12 ++
clang/lib/CodeGen/CoverageMappingGen.cpp | 22 ++-
.../CoverageMapping/single-byte-counters.cpp | 13 ++-
3 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index dbc1ce9bf993cd..80fe5cf183de16 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -2259,6 +2259,18 @@ void CodeGenFunction::EmitSwitchStmt(const SwitchStmt
&S) {
ConditionScope.ForceCleanup();
+ // Close the last case (or DefaultBlock).
+ EmitBranch(SwitchExit.getBlock());
+
+ // Insert a False Counter if SwitchStmt doesn't have DefaultStmt.
+ if (getIsCounterPair(S.getCond()).second) {
+auto *ImplicitDefaultBlock = createBasicBlock("sw.false");
+EmitBlock(ImplicitDefaultBlock);
+incrementProfileCounter(true, S.getCond());
+Builder.CreateBr(SwitchInsn->getDefaultDest());
+SwitchInsn->setDefaultDest(ImplicitDefaultBlock);
+ }
+
// Emit continuation.
EmitBlock(SwitchExit.getBlock(), true);
incrementProfileCounter(&S);
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index a331d5bc68286b..c5fdf23299e4e9 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -958,6 +958,14 @@ struct CounterCoverageMappingBuilder
return {ExecCnt, SkipCnt};
}
+ Counter getSwitchImplicitDefaultCounter(const Stmt *Cond, Counter
ParentCount,
+ Counter CaseCountSum) {
+return (
+llvm::EnableSingleByteCoverage
+? Counter::getCounter(CounterMap[Cond].second = NextCounterNum++)
+: subtractCounters(ParentCount, CaseCountSum));
+ }
+
bool IsCounterEqual(Counter OutCount, Counter ParentCount) {
if (OutCount == ParentCount)
return true;
@@ -1885,7 +1893,7 @@ struct CounterCoverageMappingBuilder
propagateCounts(Counter::getZero(), Body);
BreakContinue BC = BreakContinueStack.pop_back_val();
-if (!BreakContinueStack.empty() && !llvm::EnableSingleByteCoverage)
+if (!BreakContinueStack.empty())
BreakContinueStack.back().ContinueCount = addCounters(
BreakContinueStack.back().ContinueCount, BC.ContinueCount);
@@ -1900,11 +1908,6 @@ struct CounterCoverageMappingBuilder
MostRecentLocation = getStart(S);
handleFileExit(ExitLoc);
-// When single byte coverage mode is enabled, do not create branch region
by
-// early returning.
-if (llvm::EnableSingleByteCoverage)
- return;
-
// Create a Branch Region around each Case. Subtract the case's
// counter from the Parent counter to track the "False" branch count.
Counter CaseCountSum;
@@ -1920,7 +1923,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);
}
}
@@ -1929,9 +1933,7 @@ struct CounterCoverageMappingBuilder
extendRegion(S);
SourceMappingRegion &Parent = getRegion();
-Counter Count = llvm::EnableSingleByteCoverage
-? getRegionCounter(S)
-: addCounters(Parent.getCounter(),
getRegionCounter(S));
+Counter Count = addCounters(Parent.getCounter(), getRegionCounter(S));
// Reuse the existing region if it starts at our label. This is typical of
// the first case in a switch.
diff --git a/clang/test/CoverageMapping/single-byte-counters.cpp
b/clang/test/CoverageMapping/single-byte-counters.cpp
index d20b695bc2636a..464fa370d86f09 100644
--- a/clang/test/CoverageMapping/single-byte-counters.cpp
+++ b/clang/test/CoverageMapping/single-byte-counters.cpp
@@ -34,19 +34,22 @@ int testIfElseReturn(int x) { // CHECK-NEXT: File 0,
[[@LINE]]:29 -> [[@LINE+9]]
}
// CHECK-NEXT: testSwitch
-int testSwitch(int x) { // CHECK-NEXT: File 0, [[@LINE]]:23 -> [[@LINE+17]]:2
= [[C30:#0]]
+int testSwitch(int x) { // CHECK-NEXT: File 0, [[@LINE]]:23 -> [[@LINE+20]]:2
= [[C30:#0]]
int result;
switch (x) {
-// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 ->
[[@LINE+10]]:15 = 0
- case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 =
[[C31:#2]]
+// CHECK-NEXT: Gap,File
[llvm-branch-commits] [clang] [llvm] [Coverage][Single] Enable Branch coverage for IfStmt (PR #113111)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/113111
>From 3ea6383e2142889550f37389dfaaee81e5ae7d9c Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Sun, 20 Oct 2024 15:15:03 +0900
Subject: [PATCH 1/2] [Coverage][Single] Enable Branch coverage for IfStmt
---
clang/lib/CodeGen/CGStmt.cpp | 31 +++-
clang/lib/CodeGen/CodeGenPGO.cpp | 12 ---
clang/lib/CodeGen/CoverageMappingGen.cpp | 21 +++
.../CoverageMapping/single-byte-counters.cpp | 36 ++-
4 files changed, 38 insertions(+), 62 deletions(-)
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index dbc1ce9bf993cd..c511e5f4f4213a 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -840,8 +840,7 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
// If the skipped block has no labels in it, just emit the executed block.
// This avoids emitting dead code and simplifies the CFG substantially.
if (S.isConstexpr() || !ContainsLabel(Skipped)) {
- if (CondConstant)
-incrementProfileCounter(&S);
+ incrementProfileCounter(!CondConstant, &S, true);
if (Executed) {
RunCleanupsScope ExecutedScope(*this);
EmitStmt(Executed);
@@ -851,14 +850,14 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
}
}
+ auto HasSkip = getIsCounterPair(&S);
+
// Otherwise, the condition did not fold, or we couldn't elide it. Just emit
// the conditional branch.
llvm::BasicBlock *ThenBlock = createBasicBlock("if.then");
llvm::BasicBlock *ContBlock = createBasicBlock("if.end");
- llvm::BasicBlock *ElseBlock = ContBlock;
- if (Else)
-ElseBlock = createBasicBlock("if.else");
-
+ llvm::BasicBlock *ElseBlock =
+ (Else || HasSkip.second ? createBasicBlock("if.else") : ContBlock);
// Prefer the PGO based weights over the likelihood attribute.
// When the build isn't optimized the metadata isn't used, so don't generate
// it.
@@ -891,10 +890,7 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
// Emit the 'then' code.
EmitBlock(ThenBlock);
- if (llvm::EnableSingleByteCoverage)
-incrementProfileCounter(S.getThen());
- else
-incrementProfileCounter(&S);
+ incrementProfileCounter(false, &S);
{
RunCleanupsScope ThenScope(*this);
EmitStmt(S.getThen());
@@ -908,9 +904,9 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
auto NL = ApplyDebugLocation::CreateEmpty(*this);
EmitBlock(ElseBlock);
}
-// When single byte coverage mode is enabled, add a counter to else block.
-if (llvm::EnableSingleByteCoverage)
- incrementProfileCounter(Else);
+// Add a counter to else block unless it has CounterExpr.
+if (HasSkip.second)
+ incrementProfileCounter(true, &S);
{
RunCleanupsScope ElseScope(*this);
EmitStmt(Else);
@@ -920,15 +916,14 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
auto NL = ApplyDebugLocation::CreateEmpty(*this);
EmitBranch(ContBlock);
}
+ } else if (HasSkip.second) {
+EmitBlock(ElseBlock);
+incrementProfileCounter(true, &S);
+EmitBranch(ContBlock);
}
// Emit the continuation block for code after the if.
EmitBlock(ContBlock, true);
-
- // When single byte coverage mode is enabled, add a counter to continuation
- // block.
- if (llvm::EnableSingleByteCoverage)
-incrementProfileCounter(&S);
}
bool CodeGenFunction::checkIfLoopMustProgress(const Expr
*ControllingExpression,
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 0f2090da47a374..f6b9b5c82952c4 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -366,18 +366,6 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (Hash.getHashVersion() == PGO_HASH_V1)
return Base::TraverseIfStmt(If);
-// When single byte coverage mode is enabled, add a counter to then and
-// else.
-bool NoSingleByteCoverage = !llvm::EnableSingleByteCoverage;
-for (Stmt *CS : If->children()) {
- if (!CS || NoSingleByteCoverage)
-continue;
- if (CS == If->getThen())
-CounterMap[If->getThen()] = NextCounter++;
- else if (CS == If->getElse())
-CounterMap[If->getElse()] = NextCounter++;
-}
-
// Otherwise, keep track of which branch we're in while traversing.
VisitStmt(If);
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index a331d5bc68286b..6c6aecb9994c6b 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -2050,12 +2050,7 @@ struct CounterCoverageMappingBuilder
extendRegion(S->getCond());
Counter ParentCount = getRegion().getCounter();
-auto [ThenCount, ElseCount] =
-(llvm::EnableSingleByteCoverage
- ? std::make_pair(getRegionCounter(S->getThen())
[llvm-branch-commits] [clang] [llvm] [Coverage][Single] Enable Branch coverage for CondOp (PR #113110)
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
Date: Sun, 20 Oct 2024 14:46:07 +0900
Subject: [PATCH 1/4] [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
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 EnableSingleByteCoverage;
-} // namespace llvm
-
namespace {
class AggExprEmitter : public StmtVisitor {
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 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);
Com
[llvm-branch-commits] [clang] [llvm] [Coverage][Single] Enable Branch coverage for `BinLAnd` and `BinLOr` (PR #113113)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/113113
>From 16e2bb8b73bcde1c2618bb358a905a9f463c1217 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Sun, 20 Oct 2024 16:24:26 +0900
Subject: [PATCH 1/3] [Coverage][Single] Enable Branch coverage for `BinLAnd`
and `BinLOr`
---
clang/lib/CodeGen/CGExprScalar.cpp | 83 +++-
clang/lib/CodeGen/CGStmt.cpp | 4 --
clang/lib/CodeGen/CodeGenFunction.cpp| 43 ++--
clang/lib/CodeGen/CoverageMappingGen.cpp | 6 --
4 files changed, 104 insertions(+), 32 deletions(-)
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp
b/clang/lib/CodeGen/CGExprScalar.cpp
index 11d4ec8a267605..83962ba96aa484 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -4918,6 +4918,9 @@ Value *ScalarExprEmitter::VisitBinAssign(const
BinaryOperator *E) {
}
Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) {
+ auto HasLHSSkip = CGF.getIsCounterPair(E);
+ auto HasRHSSkip = CGF.getIsCounterPair(E->getRHS());
+
// Perform vector logical and on comparisons with zero vectors.
if (E->getType()->isVectorType()) {
CGF.incrementProfileCounter(E);
@@ -4964,11 +4967,17 @@ Value *ScalarExprEmitter::VisitBinLAnd(const
BinaryOperator *E) {
CodeGenFunction::isInstrumentedCondition(E->getRHS())) {
CGF.maybeUpdateMCDCCondBitmap(E->getRHS(), RHSCond);
llvm::BasicBlock *FBlock = CGF.createBasicBlock("land.end");
+llvm::BasicBlock *RHSSkip =
+(HasRHSSkip.second ? CGF.createBasicBlock("land.rhsskip") :
FBlock);
llvm::BasicBlock *RHSBlockCnt = CGF.createBasicBlock("land.rhscnt");
-Builder.CreateCondBr(RHSCond, RHSBlockCnt, FBlock);
+Builder.CreateCondBr(RHSCond, RHSBlockCnt, RHSSkip);
CGF.EmitBlock(RHSBlockCnt);
-CGF.incrementProfileCounter(E->getRHS());
+CGF.incrementProfileCounter(false, E->getRHS());
CGF.EmitBranch(FBlock);
+if (HasRHSSkip.second) {
+ CGF.EmitBlock(RHSSkip);
+ CGF.incrementProfileCounter(true, E->getRHS());
+}
CGF.EmitBlock(FBlock);
}
@@ -4997,12 +5006,21 @@ Value *ScalarExprEmitter::VisitBinLAnd(const
BinaryOperator *E) {
llvm::BasicBlock *ContBlock = CGF.createBasicBlock("land.end");
llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("land.rhs");
+ llvm::BasicBlock *LHSFalseBlock =
+ (HasLHSSkip.second ? CGF.createBasicBlock("land.lhsskip") : ContBlock);
+
CodeGenFunction::ConditionalEvaluation eval(CGF);
// Branch on the LHS first. If it is false, go to the failure (cont) block.
- CGF.EmitBranchOnBoolExpr(E->getLHS(), RHSBlock, ContBlock,
+ CGF.EmitBranchOnBoolExpr(E->getLHS(), RHSBlock, LHSFalseBlock,
CGF.getProfileCount(E->getRHS()));
+ if (HasLHSSkip.second) {
+CGF.EmitBlock(LHSFalseBlock);
+CGF.incrementProfileCounter(true, E);
+CGF.EmitBranch(ContBlock);
+ }
+
// Any edges into the ContBlock are now from an (indeterminate number of)
// edges from this first condition. All of these values will be false.
Start
// setting up the PHI node in the Cont Block for this.
@@ -5014,7 +5032,7 @@ Value *ScalarExprEmitter::VisitBinLAnd(const
BinaryOperator *E) {
eval.begin(CGF);
CGF.EmitBlock(RHSBlock);
- CGF.incrementProfileCounter(E);
+ CGF.incrementProfileCounter(false, E);
Value *RHSCond = CGF.EvaluateExprAsBool(E->getRHS());
eval.end(CGF);
@@ -5024,15 +5042,24 @@ Value *ScalarExprEmitter::VisitBinLAnd(const
BinaryOperator *E) {
// If we're generating for profiling or coverage, generate a branch on the
// RHS to a block that increments the RHS true counter needed to track branch
// condition coverage.
+ llvm::BasicBlock *ContIncoming = RHSBlock;
if (InstrumentRegions &&
CodeGenFunction::isInstrumentedCondition(E->getRHS())) {
CGF.maybeUpdateMCDCCondBitmap(E->getRHS(), RHSCond);
llvm::BasicBlock *RHSBlockCnt = CGF.createBasicBlock("land.rhscnt");
-Builder.CreateCondBr(RHSCond, RHSBlockCnt, ContBlock);
+llvm::BasicBlock *RHSBlockSkip =
+(HasRHSSkip.second ? CGF.createBasicBlock("land.rhsskip") : ContBlock);
+Builder.CreateCondBr(RHSCond, RHSBlockCnt, RHSBlockSkip);
CGF.EmitBlock(RHSBlockCnt);
-CGF.incrementProfileCounter(E->getRHS());
+CGF.incrementProfileCounter(false, E->getRHS());
CGF.EmitBranch(ContBlock);
PN->addIncoming(RHSCond, RHSBlockCnt);
+if (HasRHSSkip.second) {
+ CGF.EmitBlock(RHSBlockSkip);
+ CGF.incrementProfileCounter(true, E->getRHS());
+ CGF.EmitBranch(ContBlock);
+ ContIncoming = RHSBlockSkip;
+}
}
// Emit an unconditional branch from this block to ContBlock.
@@ -5042,7 +5069,7 @@ Value *ScalarExprEmitter::VisitBinLAnd(const
BinaryOperator *E) {
CGF.EmitBlock(ContBlock);
}
// Insert an entry into the phi node for the edge with the value of RHSCond.
[llvm-branch-commits] [clang] [llvm] [Coverage][Single] Enable Branch coverage for `BinLAnd` and `BinLOr` (PR #113113)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/113113
>From 16e2bb8b73bcde1c2618bb358a905a9f463c1217 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Sun, 20 Oct 2024 16:24:26 +0900
Subject: [PATCH 1/3] [Coverage][Single] Enable Branch coverage for `BinLAnd`
and `BinLOr`
---
clang/lib/CodeGen/CGExprScalar.cpp | 83 +++-
clang/lib/CodeGen/CGStmt.cpp | 4 --
clang/lib/CodeGen/CodeGenFunction.cpp| 43 ++--
clang/lib/CodeGen/CoverageMappingGen.cpp | 6 --
4 files changed, 104 insertions(+), 32 deletions(-)
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp
b/clang/lib/CodeGen/CGExprScalar.cpp
index 11d4ec8a267605..83962ba96aa484 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -4918,6 +4918,9 @@ Value *ScalarExprEmitter::VisitBinAssign(const
BinaryOperator *E) {
}
Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) {
+ auto HasLHSSkip = CGF.getIsCounterPair(E);
+ auto HasRHSSkip = CGF.getIsCounterPair(E->getRHS());
+
// Perform vector logical and on comparisons with zero vectors.
if (E->getType()->isVectorType()) {
CGF.incrementProfileCounter(E);
@@ -4964,11 +4967,17 @@ Value *ScalarExprEmitter::VisitBinLAnd(const
BinaryOperator *E) {
CodeGenFunction::isInstrumentedCondition(E->getRHS())) {
CGF.maybeUpdateMCDCCondBitmap(E->getRHS(), RHSCond);
llvm::BasicBlock *FBlock = CGF.createBasicBlock("land.end");
+llvm::BasicBlock *RHSSkip =
+(HasRHSSkip.second ? CGF.createBasicBlock("land.rhsskip") :
FBlock);
llvm::BasicBlock *RHSBlockCnt = CGF.createBasicBlock("land.rhscnt");
-Builder.CreateCondBr(RHSCond, RHSBlockCnt, FBlock);
+Builder.CreateCondBr(RHSCond, RHSBlockCnt, RHSSkip);
CGF.EmitBlock(RHSBlockCnt);
-CGF.incrementProfileCounter(E->getRHS());
+CGF.incrementProfileCounter(false, E->getRHS());
CGF.EmitBranch(FBlock);
+if (HasRHSSkip.second) {
+ CGF.EmitBlock(RHSSkip);
+ CGF.incrementProfileCounter(true, E->getRHS());
+}
CGF.EmitBlock(FBlock);
}
@@ -4997,12 +5006,21 @@ Value *ScalarExprEmitter::VisitBinLAnd(const
BinaryOperator *E) {
llvm::BasicBlock *ContBlock = CGF.createBasicBlock("land.end");
llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("land.rhs");
+ llvm::BasicBlock *LHSFalseBlock =
+ (HasLHSSkip.second ? CGF.createBasicBlock("land.lhsskip") : ContBlock);
+
CodeGenFunction::ConditionalEvaluation eval(CGF);
// Branch on the LHS first. If it is false, go to the failure (cont) block.
- CGF.EmitBranchOnBoolExpr(E->getLHS(), RHSBlock, ContBlock,
+ CGF.EmitBranchOnBoolExpr(E->getLHS(), RHSBlock, LHSFalseBlock,
CGF.getProfileCount(E->getRHS()));
+ if (HasLHSSkip.second) {
+CGF.EmitBlock(LHSFalseBlock);
+CGF.incrementProfileCounter(true, E);
+CGF.EmitBranch(ContBlock);
+ }
+
// Any edges into the ContBlock are now from an (indeterminate number of)
// edges from this first condition. All of these values will be false.
Start
// setting up the PHI node in the Cont Block for this.
@@ -5014,7 +5032,7 @@ Value *ScalarExprEmitter::VisitBinLAnd(const
BinaryOperator *E) {
eval.begin(CGF);
CGF.EmitBlock(RHSBlock);
- CGF.incrementProfileCounter(E);
+ CGF.incrementProfileCounter(false, E);
Value *RHSCond = CGF.EvaluateExprAsBool(E->getRHS());
eval.end(CGF);
@@ -5024,15 +5042,24 @@ Value *ScalarExprEmitter::VisitBinLAnd(const
BinaryOperator *E) {
// If we're generating for profiling or coverage, generate a branch on the
// RHS to a block that increments the RHS true counter needed to track branch
// condition coverage.
+ llvm::BasicBlock *ContIncoming = RHSBlock;
if (InstrumentRegions &&
CodeGenFunction::isInstrumentedCondition(E->getRHS())) {
CGF.maybeUpdateMCDCCondBitmap(E->getRHS(), RHSCond);
llvm::BasicBlock *RHSBlockCnt = CGF.createBasicBlock("land.rhscnt");
-Builder.CreateCondBr(RHSCond, RHSBlockCnt, ContBlock);
+llvm::BasicBlock *RHSBlockSkip =
+(HasRHSSkip.second ? CGF.createBasicBlock("land.rhsskip") : ContBlock);
+Builder.CreateCondBr(RHSCond, RHSBlockCnt, RHSBlockSkip);
CGF.EmitBlock(RHSBlockCnt);
-CGF.incrementProfileCounter(E->getRHS());
+CGF.incrementProfileCounter(false, E->getRHS());
CGF.EmitBranch(ContBlock);
PN->addIncoming(RHSCond, RHSBlockCnt);
+if (HasRHSSkip.second) {
+ CGF.EmitBlock(RHSBlockSkip);
+ CGF.incrementProfileCounter(true, E->getRHS());
+ CGF.EmitBranch(ContBlock);
+ ContIncoming = RHSBlockSkip;
+}
}
// Emit an unconditional branch from this block to ContBlock.
@@ -5042,7 +5069,7 @@ Value *ScalarExprEmitter::VisitBinLAnd(const
BinaryOperator *E) {
CGF.EmitBlock(ContBlock);
}
// Insert an entry into the phi node for the edge with the value of RHSCond.
[llvm-branch-commits] [clang] [llvm] [Coverage][Single] Enable Branch coverage for loop statements (PR #113109)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/113109
>From 5d19c77551c6fc585d1b15c4c2a71c3c3f99ef8a Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Fri, 18 Oct 2024 09:33:51 +0900
Subject: [PATCH 1/3] [Coverage][Single] Enable Branch coverage for loop
statements
---
clang/lib/CodeGen/CGStmt.cpp | 82 ---
clang/lib/CodeGen/CodeGenFunction.cpp | 11 +-
clang/lib/CodeGen/CodeGenPGO.cpp | 79 +--
clang/lib/CodeGen/CoverageMappingGen.cpp | 130 +-
.../CoverageMapping/single-byte-counters.cpp | 53 +++
5 files changed, 97 insertions(+), 258 deletions(-)
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());
[llvm-branch-commits] [llvm] [Coverage] Move SingleByteCoverage out of CountedRegion (PR #110966)
https://github.com/chapuni edited https://github.com/llvm/llvm-project/pull/110966 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [Coverage] Make additional counters available for BranchRegion. NFC. (PR #112730)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/112730
>From 5e460594c8a2550c38c759b2e6f1c5dc4152f820 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Thu, 17 Oct 2024 22:15:12 +0900
Subject: [PATCH 1/5] [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
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 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,
[llvm-branch-commits] [compiler-rt] [llvm] [Coverage][Single] Round Counters to boolean after evaluation (PR #110972)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/110972
>From 178b57cbbcb2b21b7d13f90ffd75903417913438 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Wed, 2 Oct 2024 23:11:36 +0900
Subject: [PATCH 1/5] [Coverage] Move SingleByteCoverage out of CountedRegion
`SingleByteCoverage` is not per-region attribute at least.
At the moment, this change moves it into `FunctionRecord`.
---
.../ProfileData/Coverage/CoverageMapping.h| 27 +++---
.../ProfileData/Coverage/CoverageMapping.cpp | 36 ---
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index fa07b3a9e8b14f..77c447efe1a7c9 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -359,19 +359,15 @@ struct CountedRegion : public CounterMappingRegion {
uint64_t ExecutionCount;
uint64_t FalseExecutionCount;
bool Folded;
- bool HasSingleByteCoverage;
- CountedRegion(const CounterMappingRegion &R, uint64_t ExecutionCount,
-bool HasSingleByteCoverage)
+ CountedRegion(const CounterMappingRegion &R, uint64_t ExecutionCount)
: CounterMappingRegion(R), ExecutionCount(ExecutionCount),
-FalseExecutionCount(0), Folded(false),
-HasSingleByteCoverage(HasSingleByteCoverage) {}
+FalseExecutionCount(0), Folded(false) {}
CountedRegion(const CounterMappingRegion &R, uint64_t ExecutionCount,
-uint64_t FalseExecutionCount, bool HasSingleByteCoverage)
+uint64_t FalseExecutionCount)
: CounterMappingRegion(R), ExecutionCount(ExecutionCount),
-FalseExecutionCount(FalseExecutionCount), Folded(false),
-HasSingleByteCoverage(HasSingleByteCoverage) {}
+FalseExecutionCount(FalseExecutionCount), Folded(false) {}
};
/// MCDC Record grouping all information together.
@@ -702,9 +698,12 @@ struct FunctionRecord {
std::vector MCDCRecords;
/// The number of times this function was executed.
uint64_t ExecutionCount = 0;
+ bool SingleByteCoverage;
- FunctionRecord(StringRef Name, ArrayRef Filenames)
- : Name(Name), Filenames(Filenames.begin(), Filenames.end()) {}
+ FunctionRecord(StringRef Name, ArrayRef Filenames,
+ bool SingleByteCoverage)
+ : Name(Name), Filenames(Filenames.begin(), Filenames.end()),
+SingleByteCoverage(SingleByteCoverage) {}
FunctionRecord(FunctionRecord &&FR) = default;
FunctionRecord &operator=(FunctionRecord &&) = default;
@@ -714,11 +713,10 @@ struct FunctionRecord {
}
void pushRegion(CounterMappingRegion Region, uint64_t Count,
- uint64_t FalseCount, bool HasSingleByteCoverage) {
+ uint64_t FalseCount) {
if (Region.Kind == CounterMappingRegion::BranchRegion ||
Region.Kind == CounterMappingRegion::MCDCBranchRegion) {
- CountedBranchRegions.emplace_back(Region, Count, FalseCount,
-HasSingleByteCoverage);
+ CountedBranchRegions.emplace_back(Region, Count, FalseCount);
// If both counters are hard-coded to zero, then this region represents a
// constant-folded branch.
if (Region.Count.isZero() && Region.FalseCount.isZero())
@@ -727,8 +725,7 @@ struct FunctionRecord {
}
if (CountedRegions.empty())
ExecutionCount = Count;
-CountedRegions.emplace_back(Region, Count, FalseCount,
-HasSingleByteCoverage);
+CountedRegions.emplace_back(Region, Count, FalseCount);
}
};
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 18643c6b44485e..a02136d5b0386d 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -808,6 +808,7 @@ Error CoverageMapping::loadFunctionRecord(
else
OrigFuncName = getFuncNameWithoutPrefix(OrigFuncName, Record.Filenames[0]);
+ bool SingleByteCoverage = ProfileReader.hasSingleByteCoverage();
CounterMappingContext Ctx(Record.Expressions);
std::vector Counts;
@@ -855,7 +856,7 @@ Error CoverageMapping::loadFunctionRecord(
return Error::success();
MCDCDecisionRecorder MCDCDecisions;
- FunctionRecord Function(OrigFuncName, Record.Filenames);
+ FunctionRecord Function(OrigFuncName, Record.Filenames, SingleByteCoverage);
for (const auto &Region : Record.MappingRegions) {
// MCDCDecisionRegion should be handled first since it overlaps with
// others inside.
@@ -873,8 +874,7 @@ Error CoverageMapping::loadFunctionRecord(
consumeError(std::move(E));
return Error::success();
}
-Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount,
-ProfileReader.hasSingleByteCoverage());
+Function.pushRegion(Regio
[llvm-branch-commits] [compiler-rt] [llvm] [Coverage][Single] Round Counters to boolean after evaluation (PR #110972)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/110972
>From 178b57cbbcb2b21b7d13f90ffd75903417913438 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Wed, 2 Oct 2024 23:11:36 +0900
Subject: [PATCH 1/5] [Coverage] Move SingleByteCoverage out of CountedRegion
`SingleByteCoverage` is not per-region attribute at least.
At the moment, this change moves it into `FunctionRecord`.
---
.../ProfileData/Coverage/CoverageMapping.h| 27 +++---
.../ProfileData/Coverage/CoverageMapping.cpp | 36 ---
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index fa07b3a9e8b14f..77c447efe1a7c9 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -359,19 +359,15 @@ struct CountedRegion : public CounterMappingRegion {
uint64_t ExecutionCount;
uint64_t FalseExecutionCount;
bool Folded;
- bool HasSingleByteCoverage;
- CountedRegion(const CounterMappingRegion &R, uint64_t ExecutionCount,
-bool HasSingleByteCoverage)
+ CountedRegion(const CounterMappingRegion &R, uint64_t ExecutionCount)
: CounterMappingRegion(R), ExecutionCount(ExecutionCount),
-FalseExecutionCount(0), Folded(false),
-HasSingleByteCoverage(HasSingleByteCoverage) {}
+FalseExecutionCount(0), Folded(false) {}
CountedRegion(const CounterMappingRegion &R, uint64_t ExecutionCount,
-uint64_t FalseExecutionCount, bool HasSingleByteCoverage)
+uint64_t FalseExecutionCount)
: CounterMappingRegion(R), ExecutionCount(ExecutionCount),
-FalseExecutionCount(FalseExecutionCount), Folded(false),
-HasSingleByteCoverage(HasSingleByteCoverage) {}
+FalseExecutionCount(FalseExecutionCount), Folded(false) {}
};
/// MCDC Record grouping all information together.
@@ -702,9 +698,12 @@ struct FunctionRecord {
std::vector MCDCRecords;
/// The number of times this function was executed.
uint64_t ExecutionCount = 0;
+ bool SingleByteCoverage;
- FunctionRecord(StringRef Name, ArrayRef Filenames)
- : Name(Name), Filenames(Filenames.begin(), Filenames.end()) {}
+ FunctionRecord(StringRef Name, ArrayRef Filenames,
+ bool SingleByteCoverage)
+ : Name(Name), Filenames(Filenames.begin(), Filenames.end()),
+SingleByteCoverage(SingleByteCoverage) {}
FunctionRecord(FunctionRecord &&FR) = default;
FunctionRecord &operator=(FunctionRecord &&) = default;
@@ -714,11 +713,10 @@ struct FunctionRecord {
}
void pushRegion(CounterMappingRegion Region, uint64_t Count,
- uint64_t FalseCount, bool HasSingleByteCoverage) {
+ uint64_t FalseCount) {
if (Region.Kind == CounterMappingRegion::BranchRegion ||
Region.Kind == CounterMappingRegion::MCDCBranchRegion) {
- CountedBranchRegions.emplace_back(Region, Count, FalseCount,
-HasSingleByteCoverage);
+ CountedBranchRegions.emplace_back(Region, Count, FalseCount);
// If both counters are hard-coded to zero, then this region represents a
// constant-folded branch.
if (Region.Count.isZero() && Region.FalseCount.isZero())
@@ -727,8 +725,7 @@ struct FunctionRecord {
}
if (CountedRegions.empty())
ExecutionCount = Count;
-CountedRegions.emplace_back(Region, Count, FalseCount,
-HasSingleByteCoverage);
+CountedRegions.emplace_back(Region, Count, FalseCount);
}
};
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 18643c6b44485e..a02136d5b0386d 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -808,6 +808,7 @@ Error CoverageMapping::loadFunctionRecord(
else
OrigFuncName = getFuncNameWithoutPrefix(OrigFuncName, Record.Filenames[0]);
+ bool SingleByteCoverage = ProfileReader.hasSingleByteCoverage();
CounterMappingContext Ctx(Record.Expressions);
std::vector Counts;
@@ -855,7 +856,7 @@ Error CoverageMapping::loadFunctionRecord(
return Error::success();
MCDCDecisionRecorder MCDCDecisions;
- FunctionRecord Function(OrigFuncName, Record.Filenames);
+ FunctionRecord Function(OrigFuncName, Record.Filenames, SingleByteCoverage);
for (const auto &Region : Record.MappingRegions) {
// MCDCDecisionRegion should be handled first since it overlaps with
// others inside.
@@ -873,8 +874,7 @@ Error CoverageMapping::loadFunctionRecord(
consumeError(std::move(E));
return Error::success();
}
-Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount,
-ProfileReader.hasSingleByteCoverage());
+Function.pushRegion(Regio
[llvm-branch-commits] [clang] [llvm] [Coverage] Make additional counters available for BranchRegion. NFC. (PR #112730)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/112730
>From 5e460594c8a2550c38c759b2e6f1c5dc4152f820 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Thu, 17 Oct 2024 22:15:12 +0900
Subject: [PATCH 1/5] [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
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 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,
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -63,80 +113,140 @@ static OpBuilder::InsertPoint computeInsertPoint(Value
value) {
return OpBuilder::InsertPoint(insertBlock, insertPt);
}
+/// Helper function that computes an insertion point where the given values are
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(ArrayRef vals) {
+ assert(!vals.empty() && "expected at least one value");
+ OpBuilder::InsertPoint pt = computeInsertPoint(vals.front());
+ for (Value v : vals.drop_front())
+pt = chooseLaterInsertPoint(pt, computeInsertPoint(v));
+ return pt;
+}
+
//===--===//
// ConversionValueMapping
//===--===//
+/// A vector of SSA values, optimized for the most common case of a single
+/// value.
+using ValueVector = SmallVector;
+
namespace {
+
+/// Helper class to make it possible to use `ValueVector` as a key in DenseMap.
+struct ValueVectorMapInfo {
+ static ValueVector getEmptyKey() { return ValueVector{}; }
+ static ValueVector getTombstoneKey() { return ValueVector{}; }
+ static ::llvm::hash_code getHashValue(ValueVector val) {
+return ::llvm::hash_combine_range(val.begin(), val.end());
+ }
+ static bool isEqual(ValueVector LHS, ValueVector RHS) { return LHS == RHS; }
+};
+
/// This class wraps a IRMapping to provide recursive lookup
/// functionality, i.e. we will traverse if the mapped value also has a
mapping.
struct ConversionValueMapping {
/// Return "true" if an SSA value is mapped to the given value. May return
/// false positives.
bool isMappedTo(Value value) const { return mappedTo.contains(value); }
- /// Lookup the most recently mapped value with the desired type in the
+ /// Lookup the most recently mapped values with the desired types in the
/// mapping.
///
/// Special cases:
- /// - If the desired type is "null", simply return the most recently mapped
- /// value.
- /// - If there is no mapping to the desired type, also return the most
- /// recently mapped value.
- /// - If there is no mapping for the given value at all, return the given
- /// value.
- Value lookupOrDefault(Value from, Type desiredType = nullptr) const;
-
- /// Lookup a mapped value within the map, or return null if a mapping does
not
- /// exist. If a mapping exists, this follows the same behavior of
- /// `lookupOrDefault`.
- Value lookupOrNull(Value from, Type desiredType = nullptr) const;
+ /// - If the desired type range is empty, simply return the most recently
+ /// mapped values.
+ /// - If there is no mapping to the desired types, also return the most
+ /// recently mapped values.
+ /// - If there is no mapping for the given values at all, return the given
+ /// values.
+ ValueVector lookupOrDefault(ValueVector from,
+ TypeRange desiredTypes = {}) const;
+
+ /// Lookup the given values within the map, or return an empty vector if the
+ /// values are not mapped. If they are mapped, this follows the same behavior
+ /// as `lookupOrDefault`.
+ ValueVector lookupOrNull(const ValueVector &from,
+ TypeRange desiredTypes = {}) const;
/// Map a value to the one provided.
- void map(Value oldVal, Value newVal) {
+ void map(const ValueVector &oldVal, const ValueVector &newVal) {
LLVM_DEBUG({
- for (Value it = newVal; it; it = mapping.lookupOrNull(it))
-assert(it != oldVal && "inserting cyclic mapping");
+ ValueVector next = newVal;
+ while (true) {
+assert(next != oldVal && "inserting cyclic mapping");
+auto it = mapping.find(next);
+if (it == mapping.end())
+ break;
+next = it->second;
+ }
});
-mapping.map(oldVal, newVal);
-mappedTo.insert(newVal);
+mapping[oldVal] = newVal;
+for (Value v : newVal)
+ mappedTo.insert(v);
}
- /// Drop the last mapping for the given value.
- void erase(Value value) { mapping.erase(value); }
+ /// Drop the last mapping for the given values.
+ void erase(ValueVector value) { mapping.erase(value); }
private:
/// Current value mappings.
- IRMapping mapping;
+ DenseMap mapping;
/// All SSA values that are mapped to. May contain false positives.
DenseSet mappedTo;
};
} // namespace
-Value ConversionValueMapping::lookupOrDefault(Value from,
- Type desiredType) const {
- // Try to find the deepest value that has the desired type. If there is no
- // such value, simply return the deepest value.
- Value desiredValue;
+ValueVector
+ConversionValueMapping::lookupOrDefault(ValueVector from,
+TypeRange desiredTypes) const {
+ // Try to find the deepest values that have the desired types. If there is no
+ // such mapping, simply return the deepest values.
+ ValueVe
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -63,80 +113,140 @@ static OpBuilder::InsertPoint computeInsertPoint(Value
value) {
return OpBuilder::InsertPoint(insertBlock, insertPt);
}
+/// Helper function that computes an insertion point where the given values are
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(ArrayRef vals) {
+ assert(!vals.empty() && "expected at least one value");
+ OpBuilder::InsertPoint pt = computeInsertPoint(vals.front());
+ for (Value v : vals.drop_front())
+pt = chooseLaterInsertPoint(pt, computeInsertPoint(v));
+ return pt;
+}
+
//===--===//
// ConversionValueMapping
//===--===//
+/// A vector of SSA values, optimized for the most common case of a single
+/// value.
+using ValueVector = SmallVector;
+
namespace {
+
+/// Helper class to make it possible to use `ValueVector` as a key in DenseMap.
+struct ValueVectorMapInfo {
+ static ValueVector getEmptyKey() { return ValueVector{}; }
+ static ValueVector getTombstoneKey() { return ValueVector{}; }
+ static ::llvm::hash_code getHashValue(ValueVector val) {
+return ::llvm::hash_combine_range(val.begin(), val.end());
+ }
+ static bool isEqual(ValueVector LHS, ValueVector RHS) { return LHS == RHS; }
+};
+
/// This class wraps a IRMapping to provide recursive lookup
/// functionality, i.e. we will traverse if the mapped value also has a
mapping.
struct ConversionValueMapping {
/// Return "true" if an SSA value is mapped to the given value. May return
/// false positives.
bool isMappedTo(Value value) const { return mappedTo.contains(value); }
- /// Lookup the most recently mapped value with the desired type in the
+ /// Lookup the most recently mapped values with the desired types in the
/// mapping.
///
/// Special cases:
- /// - If the desired type is "null", simply return the most recently mapped
- /// value.
- /// - If there is no mapping to the desired type, also return the most
- /// recently mapped value.
- /// - If there is no mapping for the given value at all, return the given
- /// value.
- Value lookupOrDefault(Value from, Type desiredType = nullptr) const;
-
- /// Lookup a mapped value within the map, or return null if a mapping does
not
- /// exist. If a mapping exists, this follows the same behavior of
- /// `lookupOrDefault`.
- Value lookupOrNull(Value from, Type desiredType = nullptr) const;
+ /// - If the desired type range is empty, simply return the most recently
+ /// mapped values.
+ /// - If there is no mapping to the desired types, also return the most
+ /// recently mapped values.
+ /// - If there is no mapping for the given values at all, return the given
+ /// values.
+ ValueVector lookupOrDefault(ValueVector from,
+ TypeRange desiredTypes = {}) const;
+
+ /// Lookup the given values within the map, or return an empty vector if the
+ /// values are not mapped. If they are mapped, this follows the same behavior
+ /// as `lookupOrDefault`.
+ ValueVector lookupOrNull(const ValueVector &from,
+ TypeRange desiredTypes = {}) const;
/// Map a value to the one provided.
- void map(Value oldVal, Value newVal) {
+ void map(const ValueVector &oldVal, const ValueVector &newVal) {
zero9178 wrote:
There's a few callers that call these with r-values where a move construction
would avoid copying entirely.
Could we change these to be universal references and then use `std::forward`
for the `operator[]` + assignment at line 184?
I prototyped this a bit roughly here if you'd like to cherry-pick in the
changes:
https://github.com/llvm/llvm-project/commit/5e857ac6348e7455f1c72fd49eb887fc1406949c
https://github.com/llvm/llvm-project/pull/116524
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -1192,51 +1256,28 @@ LogicalResult
ConversionPatternRewriterImpl::remapValues(
});
return failure();
}
-
// If a type is converted to 0 types, there is nothing to do.
if (legalTypes.empty()) {
remapped.push_back({});
continue;
}
-if (legalTypes.size() != 1) {
- // TODO: This is a 1:N conversion. The conversion value mapping does not
- // store such materializations yet. If the types of the most recently
- // mapped values do not match, build a target materialization.
- ValueRange unpackedRange(unpacked);
- if (TypeRange(unpackedRange) == legalTypes) {
-remapped.push_back(std::move(unpacked));
-continue;
- }
-
- // Insert a target materialization if the current pattern expects
- // different legalized types.
- ValueRange targetMat = buildUnresolvedMaterialization(
- MaterializationKind::Target, computeInsertPoint(repl), operandLoc,
- /*valueToMap=*/Value(), /*inputs=*/unpacked,
- /*outputType=*/legalTypes, /*originalType=*/origType,
- currentTypeConverter);
- remapped.push_back(targetMat);
+ValueVector repl = mapping.lookupOrDefault({operand}, legalTypes);
+if (!repl.empty() && TypeRange(repl) == legalTypes) {
+ // Mapped values have the correct type or there is an existing
+ // materialization. Or the opreand is not mapped at all and has the
+ // correct type.
+ remapped.push_back(repl);
zero9178 wrote:
```suggestion
remapped.push_back(std::move(repl));
```
https://github.com/llvm/llvm-project/pull/116524
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -63,80 +113,140 @@ static OpBuilder::InsertPoint computeInsertPoint(Value
value) {
return OpBuilder::InsertPoint(insertBlock, insertPt);
}
+/// Helper function that computes an insertion point where the given values are
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(ArrayRef vals) {
+ assert(!vals.empty() && "expected at least one value");
+ OpBuilder::InsertPoint pt = computeInsertPoint(vals.front());
+ for (Value v : vals.drop_front())
+pt = chooseLaterInsertPoint(pt, computeInsertPoint(v));
+ return pt;
+}
+
//===--===//
// ConversionValueMapping
//===--===//
+/// A vector of SSA values, optimized for the most common case of a single
+/// value.
+using ValueVector = SmallVector;
+
namespace {
+
+/// Helper class to make it possible to use `ValueVector` as a key in DenseMap.
+struct ValueVectorMapInfo {
+ static ValueVector getEmptyKey() { return ValueVector{}; }
+ static ValueVector getTombstoneKey() { return ValueVector{}; }
+ static ::llvm::hash_code getHashValue(ValueVector val) {
+return ::llvm::hash_combine_range(val.begin(), val.end());
+ }
+ static bool isEqual(ValueVector LHS, ValueVector RHS) { return LHS == RHS; }
+};
+
/// This class wraps a IRMapping to provide recursive lookup
/// functionality, i.e. we will traverse if the mapped value also has a
mapping.
struct ConversionValueMapping {
/// Return "true" if an SSA value is mapped to the given value. May return
/// false positives.
bool isMappedTo(Value value) const { return mappedTo.contains(value); }
- /// Lookup the most recently mapped value with the desired type in the
+ /// Lookup the most recently mapped values with the desired types in the
/// mapping.
///
/// Special cases:
- /// - If the desired type is "null", simply return the most recently mapped
- /// value.
- /// - If there is no mapping to the desired type, also return the most
- /// recently mapped value.
- /// - If there is no mapping for the given value at all, return the given
- /// value.
- Value lookupOrDefault(Value from, Type desiredType = nullptr) const;
-
- /// Lookup a mapped value within the map, or return null if a mapping does
not
- /// exist. If a mapping exists, this follows the same behavior of
- /// `lookupOrDefault`.
- Value lookupOrNull(Value from, Type desiredType = nullptr) const;
+ /// - If the desired type range is empty, simply return the most recently
+ /// mapped values.
+ /// - If there is no mapping to the desired types, also return the most
+ /// recently mapped values.
+ /// - If there is no mapping for the given values at all, return the given
+ /// values.
+ ValueVector lookupOrDefault(ValueVector from,
+ TypeRange desiredTypes = {}) const;
+
+ /// Lookup the given values within the map, or return an empty vector if the
+ /// values are not mapped. If they are mapped, this follows the same behavior
+ /// as `lookupOrDefault`.
+ ValueVector lookupOrNull(const ValueVector &from,
+ TypeRange desiredTypes = {}) const;
/// Map a value to the one provided.
- void map(Value oldVal, Value newVal) {
+ void map(const ValueVector &oldVal, const ValueVector &newVal) {
LLVM_DEBUG({
- for (Value it = newVal; it; it = mapping.lookupOrNull(it))
-assert(it != oldVal && "inserting cyclic mapping");
+ ValueVector next = newVal;
+ while (true) {
+assert(next != oldVal && "inserting cyclic mapping");
+auto it = mapping.find(next);
+if (it == mapping.end())
+ break;
+next = it->second;
+ }
});
-mapping.map(oldVal, newVal);
-mappedTo.insert(newVal);
+mapping[oldVal] = newVal;
+for (Value v : newVal)
+ mappedTo.insert(v);
}
- /// Drop the last mapping for the given value.
- void erase(Value value) { mapping.erase(value); }
+ /// Drop the last mapping for the given values.
+ void erase(ValueVector value) { mapping.erase(value); }
private:
/// Current value mappings.
- IRMapping mapping;
+ DenseMap mapping;
/// All SSA values that are mapped to. May contain false positives.
DenseSet mappedTo;
};
} // namespace
-Value ConversionValueMapping::lookupOrDefault(Value from,
- Type desiredType) const {
- // Try to find the deepest value that has the desired type. If there is no
- // such value, simply return the deepest value.
- Value desiredValue;
+ValueVector
+ConversionValueMapping::lookupOrDefault(ValueVector from,
+TypeRange desiredTypes) const {
+ // Try to find the deepest values that have the desired types. If there is no
+ // such mapping, simply return the deepest values.
+ ValueVe
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -1478,34 +1497,12 @@ Value
ConversionPatternRewriterImpl::findOrBuildReplacementValue(
}
Value castValue = buildUnresolvedMaterialization(
MaterializationKind::Source, computeInsertPoint(repl), value.getLoc(),
- /*valueToMap=*/value, /*inputs=*/repl, /*outputType=*/value.getType(),
- /*originalType=*/Type(), converter);
- mapping.map(value, castValue);
+ /*valuesToMap=*/{value}, /*inputs=*/repl, /*outputType=*/value.getType(),
+ /*originalType=*/Type(), converter)[0];
zero9178 wrote:
```suggestion
/*originalType=*/Type(), converter).front();
```
https://github.com/llvm/llvm-project/pull/116524
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -1192,51 +1256,28 @@ LogicalResult
ConversionPatternRewriterImpl::remapValues(
});
return failure();
}
-
// If a type is converted to 0 types, there is nothing to do.
if (legalTypes.empty()) {
remapped.push_back({});
continue;
}
-if (legalTypes.size() != 1) {
- // TODO: This is a 1:N conversion. The conversion value mapping does not
- // store such materializations yet. If the types of the most recently
- // mapped values do not match, build a target materialization.
- ValueRange unpackedRange(unpacked);
- if (TypeRange(unpackedRange) == legalTypes) {
-remapped.push_back(std::move(unpacked));
-continue;
- }
-
- // Insert a target materialization if the current pattern expects
- // different legalized types.
- ValueRange targetMat = buildUnresolvedMaterialization(
- MaterializationKind::Target, computeInsertPoint(repl), operandLoc,
- /*valueToMap=*/Value(), /*inputs=*/unpacked,
- /*outputType=*/legalTypes, /*originalType=*/origType,
- currentTypeConverter);
- remapped.push_back(targetMat);
+ValueVector repl = mapping.lookupOrDefault({operand}, legalTypes);
+if (!repl.empty() && TypeRange(repl) == legalTypes) {
+ // Mapped values have the correct type or there is an existing
+ // materialization. Or the opreand is not mapped at all and has the
zero9178 wrote:
```suggestion
// materialization. Or the operand is not mapped at all and has the
```
https://github.com/llvm/llvm-project/pull/116524
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -63,80 +113,140 @@ static OpBuilder::InsertPoint computeInsertPoint(Value
value) {
return OpBuilder::InsertPoint(insertBlock, insertPt);
}
+/// Helper function that computes an insertion point where the given values are
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(ArrayRef vals) {
+ assert(!vals.empty() && "expected at least one value");
+ OpBuilder::InsertPoint pt = computeInsertPoint(vals.front());
+ for (Value v : vals.drop_front())
+pt = chooseLaterInsertPoint(pt, computeInsertPoint(v));
+ return pt;
+}
+
//===--===//
// ConversionValueMapping
//===--===//
+/// A vector of SSA values, optimized for the most common case of a single
+/// value.
+using ValueVector = SmallVector;
+
namespace {
+
+/// Helper class to make it possible to use `ValueVector` as a key in DenseMap.
+struct ValueVectorMapInfo {
+ static ValueVector getEmptyKey() { return ValueVector{}; }
+ static ValueVector getTombstoneKey() { return ValueVector{}; }
+ static ::llvm::hash_code getHashValue(ValueVector val) {
+return ::llvm::hash_combine_range(val.begin(), val.end());
+ }
+ static bool isEqual(ValueVector LHS, ValueVector RHS) { return LHS == RHS; }
+};
+
/// This class wraps a IRMapping to provide recursive lookup
/// functionality, i.e. we will traverse if the mapped value also has a
mapping.
struct ConversionValueMapping {
/// Return "true" if an SSA value is mapped to the given value. May return
/// false positives.
bool isMappedTo(Value value) const { return mappedTo.contains(value); }
- /// Lookup the most recently mapped value with the desired type in the
+ /// Lookup the most recently mapped values with the desired types in the
/// mapping.
///
/// Special cases:
- /// - If the desired type is "null", simply return the most recently mapped
- /// value.
- /// - If there is no mapping to the desired type, also return the most
- /// recently mapped value.
- /// - If there is no mapping for the given value at all, return the given
- /// value.
- Value lookupOrDefault(Value from, Type desiredType = nullptr) const;
-
- /// Lookup a mapped value within the map, or return null if a mapping does
not
- /// exist. If a mapping exists, this follows the same behavior of
- /// `lookupOrDefault`.
- Value lookupOrNull(Value from, Type desiredType = nullptr) const;
+ /// - If the desired type range is empty, simply return the most recently
+ /// mapped values.
+ /// - If there is no mapping to the desired types, also return the most
+ /// recently mapped values.
+ /// - If there is no mapping for the given values at all, return the given
+ /// values.
+ ValueVector lookupOrDefault(ValueVector from,
+ TypeRange desiredTypes = {}) const;
+
+ /// Lookup the given values within the map, or return an empty vector if the
+ /// values are not mapped. If they are mapped, this follows the same behavior
+ /// as `lookupOrDefault`.
+ ValueVector lookupOrNull(const ValueVector &from,
+ TypeRange desiredTypes = {}) const;
/// Map a value to the one provided.
- void map(Value oldVal, Value newVal) {
+ void map(const ValueVector &oldVal, const ValueVector &newVal) {
LLVM_DEBUG({
- for (Value it = newVal; it; it = mapping.lookupOrNull(it))
-assert(it != oldVal && "inserting cyclic mapping");
+ ValueVector next = newVal;
+ while (true) {
+assert(next != oldVal && "inserting cyclic mapping");
+auto it = mapping.find(next);
+if (it == mapping.end())
+ break;
+next = it->second;
+ }
});
-mapping.map(oldVal, newVal);
-mappedTo.insert(newVal);
+mapping[oldVal] = newVal;
+for (Value v : newVal)
+ mappedTo.insert(v);
}
- /// Drop the last mapping for the given value.
- void erase(Value value) { mapping.erase(value); }
+ /// Drop the last mapping for the given values.
+ void erase(ValueVector value) { mapping.erase(value); }
private:
/// Current value mappings.
- IRMapping mapping;
+ DenseMap mapping;
/// All SSA values that are mapped to. May contain false positives.
DenseSet mappedTo;
};
} // namespace
-Value ConversionValueMapping::lookupOrDefault(Value from,
- Type desiredType) const {
- // Try to find the deepest value that has the desired type. If there is no
- // such value, simply return the deepest value.
- Value desiredValue;
+ValueVector
+ConversionValueMapping::lookupOrDefault(ValueVector from,
+TypeRange desiredTypes) const {
+ // Try to find the deepest values that have the desired types. If there is no
+ // such mapping, simply return the deepest values.
+ ValueVe
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -1478,34 +1497,12 @@ Value ConversionPatternRewriterImpl::findOrBuildReplacementValue( } Value castValue = buildUnresolvedMaterialization( MaterializationKind::Source, computeInsertPoint(repl), value.getLoc(), zero9178 wrote: ```suggestion MaterializationKind::Source, computeInsertPoint(value), value.getLoc(), ``` I believe (at least for the current callers of `findOrBuildReplacementValue`) that this should be sound. Alternatively, I think an insertion point should be passed as argument as all callers should have enough context to have a suitable insertion point which avoids the new `computeInsertionPoint` overload. This would be very desireable IMO https://github.com/llvm/llvm-project/pull/116524 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -63,80 +113,140 @@ static OpBuilder::InsertPoint computeInsertPoint(Value
value) {
return OpBuilder::InsertPoint(insertBlock, insertPt);
}
+/// Helper function that computes an insertion point where the given values are
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(ArrayRef vals) {
+ assert(!vals.empty() && "expected at least one value");
+ OpBuilder::InsertPoint pt = computeInsertPoint(vals.front());
+ for (Value v : vals.drop_front())
+pt = chooseLaterInsertPoint(pt, computeInsertPoint(v));
+ return pt;
+}
+
//===--===//
// ConversionValueMapping
//===--===//
+/// A vector of SSA values, optimized for the most common case of a single
+/// value.
+using ValueVector = SmallVector;
+
namespace {
+
+/// Helper class to make it possible to use `ValueVector` as a key in DenseMap.
+struct ValueVectorMapInfo {
+ static ValueVector getEmptyKey() { return ValueVector{}; }
+ static ValueVector getTombstoneKey() { return ValueVector{}; }
+ static ::llvm::hash_code getHashValue(ValueVector val) {
+return ::llvm::hash_combine_range(val.begin(), val.end());
+ }
+ static bool isEqual(ValueVector LHS, ValueVector RHS) { return LHS == RHS; }
+};
+
/// This class wraps a IRMapping to provide recursive lookup
/// functionality, i.e. we will traverse if the mapped value also has a
mapping.
struct ConversionValueMapping {
/// Return "true" if an SSA value is mapped to the given value. May return
/// false positives.
bool isMappedTo(Value value) const { return mappedTo.contains(value); }
- /// Lookup the most recently mapped value with the desired type in the
+ /// Lookup the most recently mapped values with the desired types in the
/// mapping.
///
/// Special cases:
- /// - If the desired type is "null", simply return the most recently mapped
- /// value.
- /// - If there is no mapping to the desired type, also return the most
- /// recently mapped value.
- /// - If there is no mapping for the given value at all, return the given
- /// value.
- Value lookupOrDefault(Value from, Type desiredType = nullptr) const;
-
- /// Lookup a mapped value within the map, or return null if a mapping does
not
- /// exist. If a mapping exists, this follows the same behavior of
- /// `lookupOrDefault`.
- Value lookupOrNull(Value from, Type desiredType = nullptr) const;
+ /// - If the desired type range is empty, simply return the most recently
+ /// mapped values.
+ /// - If there is no mapping to the desired types, also return the most
+ /// recently mapped values.
+ /// - If there is no mapping for the given values at all, return the given
+ /// values.
+ ValueVector lookupOrDefault(ValueVector from,
+ TypeRange desiredTypes = {}) const;
+
+ /// Lookup the given values within the map, or return an empty vector if the
+ /// values are not mapped. If they are mapped, this follows the same behavior
+ /// as `lookupOrDefault`.
+ ValueVector lookupOrNull(const ValueVector &from,
+ TypeRange desiredTypes = {}) const;
/// Map a value to the one provided.
- void map(Value oldVal, Value newVal) {
+ void map(const ValueVector &oldVal, const ValueVector &newVal) {
LLVM_DEBUG({
- for (Value it = newVal; it; it = mapping.lookupOrNull(it))
-assert(it != oldVal && "inserting cyclic mapping");
+ ValueVector next = newVal;
+ while (true) {
+assert(next != oldVal && "inserting cyclic mapping");
+auto it = mapping.find(next);
+if (it == mapping.end())
+ break;
+next = it->second;
+ }
});
-mapping.map(oldVal, newVal);
-mappedTo.insert(newVal);
+mapping[oldVal] = newVal;
+for (Value v : newVal)
+ mappedTo.insert(v);
}
- /// Drop the last mapping for the given value.
- void erase(Value value) { mapping.erase(value); }
+ /// Drop the last mapping for the given values.
+ void erase(ValueVector value) { mapping.erase(value); }
private:
/// Current value mappings.
- IRMapping mapping;
+ DenseMap mapping;
/// All SSA values that are mapped to. May contain false positives.
DenseSet mappedTo;
};
} // namespace
-Value ConversionValueMapping::lookupOrDefault(Value from,
- Type desiredType) const {
- // Try to find the deepest value that has the desired type. If there is no
- // such value, simply return the deepest value.
- Value desiredValue;
+ValueVector
+ConversionValueMapping::lookupOrDefault(ValueVector from,
+TypeRange desiredTypes) const {
+ // Try to find the deepest values that have the desired types. If there is no
+ // such mapping, simply return the deepest values.
+ ValueVe
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -1101,18 +1172,18 @@ void CreateOperationRewrite::rollback() {
UnresolvedMaterializationRewrite::UnresolvedMaterializationRewrite(
ConversionPatternRewriterImpl &rewriterImpl, UnrealizedConversionCastOp op,
const TypeConverter *converter, MaterializationKind kind, Type
originalType,
-Value mappedValue)
+ValueVector mappedValues)
: OperationRewrite(Kind::UnresolvedMaterialization, rewriterImpl, op),
converterAndKind(converter, kind), originalType(originalType),
- mappedValue(mappedValue) {
+ mappedValues(mappedValues) {
zero9178 wrote:
```suggestion
mappedValues(std::move(mappedValues)) {
```
https://github.com/llvm/llvm-project/pull/116524
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -63,80 +113,140 @@ static OpBuilder::InsertPoint computeInsertPoint(Value
value) {
return OpBuilder::InsertPoint(insertBlock, insertPt);
}
+/// Helper function that computes an insertion point where the given values are
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(ArrayRef vals) {
+ assert(!vals.empty() && "expected at least one value");
+ OpBuilder::InsertPoint pt = computeInsertPoint(vals.front());
+ for (Value v : vals.drop_front())
+pt = chooseLaterInsertPoint(pt, computeInsertPoint(v));
+ return pt;
+}
+
//===--===//
// ConversionValueMapping
//===--===//
+/// A vector of SSA values, optimized for the most common case of a single
+/// value.
+using ValueVector = SmallVector;
+
namespace {
+
+/// Helper class to make it possible to use `ValueVector` as a key in DenseMap.
+struct ValueVectorMapInfo {
+ static ValueVector getEmptyKey() { return ValueVector{}; }
+ static ValueVector getTombstoneKey() { return ValueVector{}; }
+ static ::llvm::hash_code getHashValue(ValueVector val) {
+return ::llvm::hash_combine_range(val.begin(), val.end());
+ }
+ static bool isEqual(ValueVector LHS, ValueVector RHS) { return LHS == RHS; }
+};
+
/// This class wraps a IRMapping to provide recursive lookup
/// functionality, i.e. we will traverse if the mapped value also has a
mapping.
struct ConversionValueMapping {
/// Return "true" if an SSA value is mapped to the given value. May return
/// false positives.
bool isMappedTo(Value value) const { return mappedTo.contains(value); }
- /// Lookup the most recently mapped value with the desired type in the
+ /// Lookup the most recently mapped values with the desired types in the
/// mapping.
///
/// Special cases:
- /// - If the desired type is "null", simply return the most recently mapped
- /// value.
- /// - If there is no mapping to the desired type, also return the most
- /// recently mapped value.
- /// - If there is no mapping for the given value at all, return the given
- /// value.
- Value lookupOrDefault(Value from, Type desiredType = nullptr) const;
-
- /// Lookup a mapped value within the map, or return null if a mapping does
not
- /// exist. If a mapping exists, this follows the same behavior of
- /// `lookupOrDefault`.
- Value lookupOrNull(Value from, Type desiredType = nullptr) const;
+ /// - If the desired type range is empty, simply return the most recently
+ /// mapped values.
+ /// - If there is no mapping to the desired types, also return the most
+ /// recently mapped values.
+ /// - If there is no mapping for the given values at all, return the given
+ /// values.
+ ValueVector lookupOrDefault(ValueVector from,
+ TypeRange desiredTypes = {}) const;
+
+ /// Lookup the given values within the map, or return an empty vector if the
+ /// values are not mapped. If they are mapped, this follows the same behavior
+ /// as `lookupOrDefault`.
+ ValueVector lookupOrNull(const ValueVector &from,
+ TypeRange desiredTypes = {}) const;
zero9178 wrote:
Neither of these two functions are ever called with more than just a single
`Value`. Could we change the parameters accordingly?
https://github.com/llvm/llvm-project/pull/116524
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -1423,37 +1458,21 @@ ValueRange
ConversionPatternRewriterImpl::buildUnresolvedMaterialization(
builder.setInsertionPoint(ip.getBlock(), ip.getPoint());
auto convertOp =
builder.create(loc, outputTypes, inputs);
- if (valueToMap) {
-assert(outputTypes.size() == 1 && "1:N mapping is not supported");
-mapping.map(valueToMap, convertOp.getResult(0));
- }
+ if (!valuesToMap.empty())
+mapping.map(valuesToMap, convertOp.getResults());
if (castOp)
*castOp = convertOp;
appendRewrite(convertOp, converter, kind,
- originalType, valueToMap);
+ originalType, valuesToMap);
zero9178 wrote:
```suggestion
originalType,
std::move(valuesToMap));
```
https://github.com/llvm/llvm-project/pull/116524
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
@@ -1192,51 +1256,28 @@ LogicalResult
ConversionPatternRewriterImpl::remapValues(
});
return failure();
}
-
// If a type is converted to 0 types, there is nothing to do.
if (legalTypes.empty()) {
remapped.push_back({});
continue;
}
-if (legalTypes.size() != 1) {
- // TODO: This is a 1:N conversion. The conversion value mapping does not
- // store such materializations yet. If the types of the most recently
- // mapped values do not match, build a target materialization.
- ValueRange unpackedRange(unpacked);
- if (TypeRange(unpackedRange) == legalTypes) {
-remapped.push_back(std::move(unpacked));
-continue;
- }
-
- // Insert a target materialization if the current pattern expects
- // different legalized types.
- ValueRange targetMat = buildUnresolvedMaterialization(
- MaterializationKind::Target, computeInsertPoint(repl), operandLoc,
- /*valueToMap=*/Value(), /*inputs=*/unpacked,
- /*outputType=*/legalTypes, /*originalType=*/origType,
- currentTypeConverter);
- remapped.push_back(targetMat);
+ValueVector repl = mapping.lookupOrDefault({operand}, legalTypes);
+if (!repl.empty() && TypeRange(repl) == legalTypes) {
+ // Mapped values have the correct type or there is an existing
+ // materialization. Or the opreand is not mapped at all and has the
+ // correct type.
+ remapped.push_back(repl);
continue;
}
-// Handle 1->1 type conversions.
-Type desiredType = legalTypes.front();
-// Try to find a mapped value with the desired type. (Or the operand itself
-// if the value is not mapped at all.)
-Value newOperand = mapping.lookupOrDefault(operand, desiredType);
-if (newOperand.getType() != desiredType) {
- // If the looked up value's type does not have the desired type, it means
- // that the value was replaced with a value of different type and no
- // target materialization was created yet.
- Value castValue = buildUnresolvedMaterialization(
- MaterializationKind::Target, computeInsertPoint(newOperand),
- operandLoc, /*valueToMap=*/newOperand, /*inputs=*/unpacked,
- /*outputType=*/desiredType, /*originalType=*/origType,
- currentTypeConverter);
- newOperand = castValue;
-}
-remapped.push_back({newOperand});
+// Create a materialization for the most recently mapped values.
+repl = mapping.lookupOrDefault({operand});
+ValueRange castValues = buildUnresolvedMaterialization(
+MaterializationKind::Target, computeInsertPoint(repl), operandLoc,
zero9178 wrote:
```suggestion
MaterializationKind::Target, rewriter.saveInsertionPoint(), operandLoc,
```
Using the insertion point from pattern rewriter makes more sense to me here and
is simpler:
* For the caller in `ConversionPattern::matchAndRewrite` it is guaranteed to be
a valid insertion point where the operands are reachable
* For `getRemappedValue[s]` the user expectation is likely that the current
insertion point would be used for any materializations that occur
https://github.com/llvm/llvm-project/pull/116524
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
