Skip to content

Commit 72b2d4d

Browse files
authored
[llvm-cov] Fix branch counts of template functions (second attempt) (#135074)
This PR is a second attempt for issue #111743 to finish reverted PR #113925. Added option "--unify-instantiations" to llvm-cov export to combine branch execution counts of C++ template instantiations. Fix non-deterministic behavior.
1 parent 0ab330b commit 72b2d4d

File tree

5 files changed

+188
-31
lines changed

5 files changed

+188
-31
lines changed
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
2+
// RUN: llvm-profdata merge %S/Inputs/branch-templates.proftext -o %t.profdata
3+
// RUN: llvm-cov export --format=lcov --unify-instantiations=true %S/Inputs/branch-templates.o32l -instr-profile %t.profdata | FileCheck %s -check-prefix=UNIFY
4+
5+
// UNIFY-DAG: BRDA:14,0,0,1
6+
// UNIFY-DAG: BRDA:14,0,1,2
7+
// UNIFY-DAG: BRDA:30,0,0,1
8+
// UNIFY-DAG: BRDA:30,0,1,0
9+
// UNIFY-DAG: BRDA:32,0,0,0
10+
// UNIFY-DAG: BRDA:32,0,1,1
11+
// UNIFY-DAG: BRDA:34,0,0,1
12+
// UNIFY-DAG: BRDA:34,0,1,0
13+
// UNIFY-NOT: BRDA
14+
// UNIFY: BRF:8
15+
// UNIFY: BRH:4
16+
// UNIFY: LF:17
17+
// UNIFY: LH:13
18+
19+
// RUN: llvm-profdata merge %S/Inputs/branch-templates.proftext -o %t.profdata
20+
// RUN: llvm-cov export --format=lcov --unify-instantiations=false %S/Inputs/branch-templates.o32l -instr-profile %t.profdata | FileCheck %s
21+
22+
// CHECK-DAG: BRDA:14,0,0,0
23+
// CHECK-DAG: BRDA:14,0,1,1
24+
// CHECK-DAG: BRDA:14,1,2,1
25+
// CHECK-DAG: BRDA:14,1,3,0
26+
// CHECK-DAG: BRDA:14,2,4,0
27+
// CHECK-DAG: BRDA:14,2,5,1
28+
// CHECK-DAG: BRDA:30,0,0,1
29+
// CHECK-DAG: BRDA:30,0,1,0
30+
// CHECK-DAG: BRDA:32,0,0,0
31+
// CHECK-DAG: BRDA:32,0,1,1
32+
// CHECK-DAG: BRDA:34,0,0,1
33+
// CHECK-DAG: BRDA:34,0,1,0
34+
// CHECK-NOT: BRDA
35+
// CHECK: BRF:8
36+
// CHECK: BRH:4
37+
// CHECK: LF:17
38+
// CHECK: LH:13

llvm/test/tools/llvm-cov/branch-export-lcov.test

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939

4040
// Check recursive macro-expansions.
4141
// RUN: llvm-profdata merge %S/Inputs/branch-macros.proftext -o %t.profdata
42-
// RUN: llvm-cov export --format=lcov %S/Inputs/branch-macros.o32l -instr-profile %t.profdata | FileCheck %s -check-prefix=MACROS
42+
// RUN: llvm-cov export --format=lcov --unify-instantiations=false %S/Inputs/branch-macros.o32l -instr-profile %t.profdata | FileCheck %s -check-prefix=MACROS
4343
// RUN: llvm-cov export --format=lcov --skip-branches %S/Inputs/branch-macros.o32l -instr-profile %t.profdata | FileCheck %s -check-prefix=NOBRANCH
4444

4545
// MACROS-COUNT-4: BRDA:17
@@ -78,3 +78,38 @@
7878
// NOBRANCH-NOT: BRF
7979
// NOBRANCH-NOT: BRH
8080

81+
// Check recursive macro-expansions with unify mode.
82+
// RUN: llvm-profdata merge %S/Inputs/branch-macros.proftext -o %t.profdata
83+
// RUN: llvm-cov export --format=lcov --unify-instantiations=true %S/Inputs/branch-macros.o32l -instr-profile %t.profdata | FileCheck %s -check-prefix=MACROS2
84+
85+
// MACROS2-COUNT-4: BRDA:17
86+
// MACROS2-NOT: BRDA:17
87+
88+
// MACROS2-COUNT-4: BRDA:19
89+
// MACROS2-NOT: BRDA:19
90+
91+
// MACROS2-COUNT-4: BRDA:21
92+
// MACROS2-NOT: BRDA:21
93+
94+
// MACROS2-COUNT-4: BRDA:23
95+
// MACROS2-NOT: BRDA:23
96+
97+
// MACROS2-COUNT-4: BRDA:25
98+
// MACROS2-NOT: BRDA:25
99+
100+
// MACROS2: BRDA:27,0,0,0
101+
// MACROS2: BRDA:27,0,1,3
102+
// MACROS2: BRDA:27,1,2,-
103+
// MACROS2: BRDA:27,1,3,-
104+
// MACROS2: BRDA:27,2,4,-
105+
// MACROS2: BRDA:27,2,5,-
106+
// MACROS2: BRDA:27,3,6,-
107+
// MACROS2: BRDA:27,3,7,-
108+
// MACROS2: BRDA:27,4,8,-
109+
// MACROS2: BRDA:27,4,9,-
110+
111+
// MACROS2-COUNT-10: BRDA:37
112+
// MACROS2-NOT: BRDA:37
113+
// MACROS2-NOT: BRDA
114+
// MACROS2: BRF:40
115+
// MACROS2: BRH:24

llvm/tools/llvm-cov/CodeCoverage.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,13 +1283,18 @@ int CodeCoverageTool::doExport(int argc, const char **argv,
12831283
cl::desc("Don't export branch data (LCOV)"),
12841284
cl::cat(ExportCategory));
12851285

1286+
cl::opt<bool> UnifyInstantiations("unify-instantiations", cl::Optional,
1287+
cl::desc("Unify function instantiations"),
1288+
cl::init(true), cl::cat(ExportCategory));
1289+
12861290
auto Err = commandLineParser(argc, argv);
12871291
if (Err)
12881292
return Err;
12891293

12901294
ViewOpts.SkipExpansions = SkipExpansions;
12911295
ViewOpts.SkipFunctions = SkipFunctions;
12921296
ViewOpts.SkipBranches = SkipBranches;
1297+
ViewOpts.UnifyFunctionInstantiations = UnifyInstantiations;
12931298

12941299
if (ViewOpts.Format != CoverageViewOptions::OutputFormat::Text &&
12951300
ViewOpts.Format != CoverageViewOptions::OutputFormat::Lcov) {

llvm/tools/llvm-cov/CoverageExporterLcov.cpp

Lines changed: 108 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,29 @@
4343
#include "CoverageReport.h"
4444

4545
using namespace llvm;
46+
using namespace coverage;
4647

4748
namespace {
4849

50+
struct NestedCountedRegion : public coverage::CountedRegion {
51+
// Contains the path to default and expanded branches.
52+
// Size is 1 for default branches and greater 1 for expanded branches.
53+
std::vector<LineColPair> NestedPath;
54+
// Contains the original index of this element used to keep the original order
55+
// in case of equal nested path.
56+
unsigned Position;
57+
// Indicates whether this item should be ignored at rendering.
58+
bool Ignore = false;
59+
60+
NestedCountedRegion(llvm::coverage::CountedRegion Region,
61+
std::vector<LineColPair> NestedPath, unsigned Position)
62+
: llvm::coverage::CountedRegion(std::move(Region)),
63+
NestedPath(std::move(NestedPath)), Position(Position) {}
64+
65+
// Returns the root line of the branch.
66+
unsigned getEffectiveLine() const { return NestedPath.front().first; }
67+
};
68+
4969
void renderFunctionSummary(raw_ostream &OS,
5070
const FileCoverageSummary &Summary) {
5171
OS << "FNF:" << Summary.FunctionCoverage.getNumFunctions() << '\n'
@@ -75,71 +95,128 @@ void renderLineExecutionCounts(raw_ostream &OS,
7595
}
7696
}
7797

78-
std::vector<llvm::coverage::CountedRegion>
98+
std::vector<NestedCountedRegion>
7999
collectNestedBranches(const coverage::CoverageMapping &Coverage,
80100
ArrayRef<llvm::coverage::ExpansionRecord> Expansions,
81-
int ViewDepth = 0, int SrcLine = 0) {
82-
std::vector<llvm::coverage::CountedRegion> Branches;
101+
std::vector<LineColPair> &NestedPath,
102+
unsigned &PositionCounter) {
103+
std::vector<NestedCountedRegion> Branches;
83104
for (const auto &Expansion : Expansions) {
84105
auto ExpansionCoverage = Coverage.getCoverageForExpansion(Expansion);
85106

86-
// If we're at the top level, set the corresponding source line.
87-
if (ViewDepth == 0)
88-
SrcLine = Expansion.Region.LineStart;
107+
// Track the path to the nested expansions.
108+
NestedPath.push_back(Expansion.Region.startLoc());
89109

90110
// Recursively collect branches from nested expansions.
91111
auto NestedExpansions = ExpansionCoverage.getExpansions();
92112
auto NestedExBranches = collectNestedBranches(Coverage, NestedExpansions,
93-
ViewDepth + 1, SrcLine);
113+
NestedPath, PositionCounter);
94114
append_range(Branches, NestedExBranches);
95115

96116
// Add branches from this level of expansion.
97117
auto ExBranches = ExpansionCoverage.getBranches();
98-
for (auto B : ExBranches)
118+
for (auto &B : ExBranches)
99119
if (B.FileID == Expansion.FileID) {
100-
B.LineStart = SrcLine;
101-
Branches.push_back(B);
120+
Branches.push_back(
121+
NestedCountedRegion(B, NestedPath, PositionCounter++));
102122
}
123+
124+
NestedPath.pop_back();
103125
}
104126

105127
return Branches;
106128
}
107129

108-
bool sortLine(llvm::coverage::CountedRegion I,
109-
llvm::coverage::CountedRegion J) {
110-
return (I.LineStart < J.LineStart) ||
111-
((I.LineStart == J.LineStart) && (I.ColumnStart < J.ColumnStart));
130+
void appendNestedCountedRegions(const std::vector<CountedRegion> &Src,
131+
std::vector<NestedCountedRegion> &Dst) {
132+
auto Unfolded = make_filter_range(Src, [](auto &Region) {
133+
return !Region.TrueFolded || !Region.FalseFolded;
134+
});
135+
Dst.reserve(Dst.size() + Src.size());
136+
unsigned PositionCounter = Dst.size();
137+
std::transform(Unfolded.begin(), Unfolded.end(), std::back_inserter(Dst),
138+
[=, &PositionCounter](auto &Region) {
139+
return NestedCountedRegion(Region, {Region.startLoc()},
140+
PositionCounter++);
141+
});
142+
}
143+
144+
void appendNestedCountedRegions(const std::vector<NestedCountedRegion> &Src,
145+
std::vector<NestedCountedRegion> &Dst) {
146+
auto Unfolded = make_filter_range(Src, [](auto &NestedRegion) {
147+
return !NestedRegion.TrueFolded || !NestedRegion.FalseFolded;
148+
});
149+
Dst.reserve(Dst.size() + Src.size());
150+
std::copy(Unfolded.begin(), Unfolded.end(), std::back_inserter(Dst));
151+
}
152+
153+
bool sortNested(const NestedCountedRegion &I, const NestedCountedRegion &J) {
154+
// This sorts each element by line and column.
155+
// Implies that all elements are first sorted by getEffectiveLine().
156+
// Use original position if NestedPath is equal.
157+
return std::tie(I.NestedPath, I.Position) <
158+
std::tie(J.NestedPath, J.Position);
159+
}
160+
161+
void combineInstanceCounts(std::vector<NestedCountedRegion> &Branches) {
162+
auto NextBranch = Branches.begin();
163+
auto EndBranch = Branches.end();
164+
165+
while (NextBranch != EndBranch) {
166+
auto SumBranch = NextBranch++;
167+
168+
// Ensure that only branches with the same NestedPath are summed up.
169+
while (NextBranch != EndBranch &&
170+
SumBranch->NestedPath == NextBranch->NestedPath) {
171+
SumBranch->ExecutionCount += NextBranch->ExecutionCount;
172+
SumBranch->FalseExecutionCount += NextBranch->FalseExecutionCount;
173+
// Mark this branch as ignored.
174+
NextBranch->Ignore = true;
175+
176+
NextBranch++;
177+
}
178+
}
112179
}
113180

114181
void renderBranchExecutionCounts(raw_ostream &OS,
115182
const coverage::CoverageMapping &Coverage,
116-
const coverage::CoverageData &FileCoverage) {
117-
std::vector<llvm::coverage::CountedRegion> Branches =
118-
FileCoverage.getBranches();
183+
const coverage::CoverageData &FileCoverage,
184+
bool UnifyInstances) {
185+
186+
std::vector<NestedCountedRegion> Branches;
187+
188+
appendNestedCountedRegions(FileCoverage.getBranches(), Branches);
119189

120190
// Recursively collect branches for all file expansions.
121-
std::vector<llvm::coverage::CountedRegion> ExBranches =
122-
collectNestedBranches(Coverage, FileCoverage.getExpansions());
191+
std::vector<LineColPair> NestedPath;
192+
unsigned PositionCounter = 0;
193+
std::vector<NestedCountedRegion> ExBranches = collectNestedBranches(
194+
Coverage, FileCoverage.getExpansions(), NestedPath, PositionCounter);
123195

124196
// Append Expansion Branches to Source Branches.
125-
append_range(Branches, ExBranches);
197+
appendNestedCountedRegions(ExBranches, Branches);
126198

127199
// Sort branches based on line number to ensure branches corresponding to the
128200
// same source line are counted together.
129-
llvm::sort(Branches, sortLine);
201+
llvm::sort(Branches, sortNested);
202+
203+
if (UnifyInstances) {
204+
combineInstanceCounts(Branches);
205+
}
130206

131207
auto NextBranch = Branches.begin();
132208
auto EndBranch = Branches.end();
133209

134210
// Branches with the same source line are enumerated individually
135211
// (BranchIndex) as well as based on True/False pairs (PairIndex).
136212
while (NextBranch != EndBranch) {
137-
unsigned CurrentLine = NextBranch->LineStart;
213+
unsigned CurrentLine = NextBranch->getEffectiveLine();
138214
unsigned PairIndex = 0;
139215
unsigned BranchIndex = 0;
140216

141-
while (NextBranch != EndBranch && CurrentLine == NextBranch->LineStart) {
142-
if (!NextBranch->TrueFolded || !NextBranch->FalseFolded) {
217+
while (NextBranch != EndBranch &&
218+
CurrentLine == NextBranch->getEffectiveLine()) {
219+
if (!NextBranch->Ignore) {
143220
unsigned BC1 = NextBranch->ExecutionCount;
144221
unsigned BC2 = NextBranch->FalseExecutionCount;
145222
bool BranchNotExecuted = (BC1 == 0 && BC2 == 0);
@@ -173,7 +250,7 @@ void renderBranchSummary(raw_ostream &OS, const FileCoverageSummary &Summary) {
173250
void renderFile(raw_ostream &OS, const coverage::CoverageMapping &Coverage,
174251
const std::string &Filename,
175252
const FileCoverageSummary &FileReport, bool ExportSummaryOnly,
176-
bool SkipFunctions, bool SkipBranches) {
253+
bool SkipFunctions, bool SkipBranches, bool UnifyInstances) {
177254
OS << "SF:" << Filename << '\n';
178255

179256
if (!ExportSummaryOnly && !SkipFunctions) {
@@ -186,7 +263,7 @@ void renderFile(raw_ostream &OS, const coverage::CoverageMapping &Coverage,
186263
auto FileCoverage = Coverage.getCoverageForFile(Filename);
187264
renderLineExecutionCounts(OS, FileCoverage);
188265
if (!SkipBranches)
189-
renderBranchExecutionCounts(OS, Coverage, FileCoverage);
266+
renderBranchExecutionCounts(OS, Coverage, FileCoverage, UnifyInstances);
190267
}
191268
if (!SkipBranches)
192269
renderBranchSummary(OS, FileReport);
@@ -198,11 +275,11 @@ void renderFile(raw_ostream &OS, const coverage::CoverageMapping &Coverage,
198275
void renderFiles(raw_ostream &OS, const coverage::CoverageMapping &Coverage,
199276
ArrayRef<std::string> SourceFiles,
200277
ArrayRef<FileCoverageSummary> FileReports,
201-
bool ExportSummaryOnly, bool SkipFunctions,
202-
bool SkipBranches) {
278+
bool ExportSummaryOnly, bool SkipFunctions, bool SkipBranches,
279+
bool UnifyInstances) {
203280
for (unsigned I = 0, E = SourceFiles.size(); I < E; ++I)
204281
renderFile(OS, Coverage, SourceFiles[I], FileReports[I], ExportSummaryOnly,
205-
SkipFunctions, SkipBranches);
282+
SkipFunctions, SkipBranches, UnifyInstances);
206283
}
207284

208285
} // end anonymous namespace
@@ -221,5 +298,6 @@ void CoverageExporterLcov::renderRoot(ArrayRef<std::string> SourceFiles) {
221298
auto FileReports = CoverageReport::prepareFileReports(Coverage, Totals,
222299
SourceFiles, Options);
223300
renderFiles(OS, Coverage, SourceFiles, FileReports, Options.ExportSummaryOnly,
224-
Options.SkipFunctions, Options.SkipBranches);
301+
Options.SkipFunctions, Options.SkipBranches,
302+
Options.UnifyFunctionInstantiations);
225303
}

llvm/tools/llvm-cov/CoverageViewOptions.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ struct CoverageViewOptions {
3535
bool ShowBranchPercents;
3636
bool ShowExpandedRegions;
3737
bool ShowFunctionInstantiations;
38+
bool UnifyFunctionInstantiations;
3839
bool ShowFullFilenames;
3940
bool ShowBranchSummary;
4041
bool ShowMCDCSummary;

0 commit comments

Comments
 (0)