bruno created this revision.
bruno added reviewers: alanphipps, vsk, zequanwu.
Herald added subscribers: hoy, modimo, wenlei, hiraditya.
Herald added a project: All.
bruno requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
After D84467 <https://reviews.llvm.org/D84467>, C++ generated code with huge
number of switch cases chokes badly while emitting
coverage mapping, in our specific testcase (~72k cases), it won't stop after
hours.
This patch makes the frontend job to finish in 4.5s and shrinks down
`@__covrec_`
by 288k when compared to disabling simplification altogether.
There's probably no good way to create a testcase for this, but it's easy to
reproduce, just add thousands of cases in the below switch, and build with
`-fprofile-instr-generate -fcoverage-mapping`.
enum type : int {
FEATURE_INVALID = 0,
FEATURE_A = 1,
...
};
const char *to_string(type e) {
switch (e) {
case type::FEATURE_INVALID: return "FEATURE_INVALID";
case type::FEATURE_A: return "FEATURE_A";}
...
}
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126345
Files:
clang/lib/CodeGen/CoverageMappingGen.cpp
llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
Index: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
===================================================================
--- llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -123,13 +123,15 @@
return C;
}
-Counter CounterExpressionBuilder::add(Counter LHS, Counter RHS) {
- return simplify(get(CounterExpression(CounterExpression::Add, LHS, RHS)));
+Counter CounterExpressionBuilder::add(Counter LHS, Counter RHS, bool Simplify)
{
+ auto Cnt = get(CounterExpression(CounterExpression::Add, LHS, RHS));
+ return Simplify ? simplify(Cnt) : Cnt;
}
-Counter CounterExpressionBuilder::subtract(Counter LHS, Counter RHS) {
- return simplify(
- get(CounterExpression(CounterExpression::Subtract, LHS, RHS)));
+Counter CounterExpressionBuilder::subtract(Counter LHS, Counter RHS,
+ bool Simplify) {
+ auto Cnt = get(CounterExpression(CounterExpression::Subtract, LHS, RHS));
+ return Simplify ? simplify(Cnt) : Cnt;
}
void CounterMappingContext::dump(const Counter &C, raw_ostream &OS) const {
Index: llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
===================================================================
--- llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -195,11 +195,11 @@
ArrayRef<CounterExpression> getExpressions() const { return Expressions; }
/// Return a counter that represents the expression that adds LHS and RHS.
- Counter add(Counter LHS, Counter RHS);
+ Counter add(Counter LHS, Counter RHS, bool Simplify = true);
/// Return a counter that represents the expression that subtracts RHS from
/// LHS.
- Counter subtract(Counter LHS, Counter RHS);
+ Counter subtract(Counter LHS, Counter RHS, bool Simplify = true);
};
using LineColPair = std::pair<unsigned, unsigned>;
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===================================================================
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -550,17 +550,18 @@
Counter GapRegionCounter;
/// Return a counter for the subtraction of \c RHS from \c LHS
- Counter subtractCounters(Counter LHS, Counter RHS) {
- return Builder.subtract(LHS, RHS);
+ Counter subtractCounters(Counter LHS, Counter RHS, bool Simplify = true) {
+ return Builder.subtract(LHS, RHS, Simplify);
}
/// Return a counter for the sum of \c LHS and \c RHS.
- Counter addCounters(Counter LHS, Counter RHS) {
- return Builder.add(LHS, RHS);
+ Counter addCounters(Counter LHS, Counter RHS, bool Simplify = true) {
+ return Builder.add(LHS, RHS, Simplify);
}
- Counter addCounters(Counter C1, Counter C2, Counter C3) {
- return addCounters(addCounters(C1, C2), C3);
+ Counter addCounters(Counter C1, Counter C2, Counter C3,
+ bool Simplify = true) {
+ return addCounters(addCounters(C1, C2, Simplify), C3, Simplify);
}
/// Return the region counter for the given statement.
@@ -1317,11 +1318,16 @@
const SwitchCase *Case = S->getSwitchCaseList();
for (; Case; Case = Case->getNextSwitchCase()) {
HasDefaultCase = HasDefaultCase || isa<DefaultStmt>(Case);
- CaseCountSum = addCounters(CaseCountSum, getRegionCounter(Case));
+ CaseCountSum =
+ addCounters(CaseCountSum, getRegionCounter(Case),
/*Simplify=*/false);
createSwitchCaseRegion(
Case, getRegionCounter(Case),
subtractCounters(ParentCount, getRegionCounter(Case)));
}
+ // Simplify is skipped while building the counters above: it can get really
+ // slow on top of switches with thousands of cases. Instead, trigger
+ // simplification by adding zero to the last counter.
+ CaseCountSum = addCounters(CaseCountSum, Counter::getZero());
// If no explicit default case exists, create a branch region to represent
// the hidden branch, which will be added later by the CodeGen. This region
Index: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
===================================================================
--- llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -123,13 +123,15 @@
return C;
}
-Counter CounterExpressionBuilder::add(Counter LHS, Counter RHS) {
- return simplify(get(CounterExpression(CounterExpression::Add, LHS, RHS)));
+Counter CounterExpressionBuilder::add(Counter LHS, Counter RHS, bool Simplify) {
+ auto Cnt = get(CounterExpression(CounterExpression::Add, LHS, RHS));
+ return Simplify ? simplify(Cnt) : Cnt;
}
-Counter CounterExpressionBuilder::subtract(Counter LHS, Counter RHS) {
- return simplify(
- get(CounterExpression(CounterExpression::Subtract, LHS, RHS)));
+Counter CounterExpressionBuilder::subtract(Counter LHS, Counter RHS,
+ bool Simplify) {
+ auto Cnt = get(CounterExpression(CounterExpression::Subtract, LHS, RHS));
+ return Simplify ? simplify(Cnt) : Cnt;
}
void CounterMappingContext::dump(const Counter &C, raw_ostream &OS) const {
Index: llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
===================================================================
--- llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -195,11 +195,11 @@
ArrayRef<CounterExpression> getExpressions() const { return Expressions; }
/// Return a counter that represents the expression that adds LHS and RHS.
- Counter add(Counter LHS, Counter RHS);
+ Counter add(Counter LHS, Counter RHS, bool Simplify = true);
/// Return a counter that represents the expression that subtracts RHS from
/// LHS.
- Counter subtract(Counter LHS, Counter RHS);
+ Counter subtract(Counter LHS, Counter RHS, bool Simplify = true);
};
using LineColPair = std::pair<unsigned, unsigned>;
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===================================================================
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -550,17 +550,18 @@
Counter GapRegionCounter;
/// Return a counter for the subtraction of \c RHS from \c LHS
- Counter subtractCounters(Counter LHS, Counter RHS) {
- return Builder.subtract(LHS, RHS);
+ Counter subtractCounters(Counter LHS, Counter RHS, bool Simplify = true) {
+ return Builder.subtract(LHS, RHS, Simplify);
}
/// Return a counter for the sum of \c LHS and \c RHS.
- Counter addCounters(Counter LHS, Counter RHS) {
- return Builder.add(LHS, RHS);
+ Counter addCounters(Counter LHS, Counter RHS, bool Simplify = true) {
+ return Builder.add(LHS, RHS, Simplify);
}
- Counter addCounters(Counter C1, Counter C2, Counter C3) {
- return addCounters(addCounters(C1, C2), C3);
+ Counter addCounters(Counter C1, Counter C2, Counter C3,
+ bool Simplify = true) {
+ return addCounters(addCounters(C1, C2, Simplify), C3, Simplify);
}
/// Return the region counter for the given statement.
@@ -1317,11 +1318,16 @@
const SwitchCase *Case = S->getSwitchCaseList();
for (; Case; Case = Case->getNextSwitchCase()) {
HasDefaultCase = HasDefaultCase || isa<DefaultStmt>(Case);
- CaseCountSum = addCounters(CaseCountSum, getRegionCounter(Case));
+ CaseCountSum =
+ addCounters(CaseCountSum, getRegionCounter(Case), /*Simplify=*/false);
createSwitchCaseRegion(
Case, getRegionCounter(Case),
subtractCounters(ParentCount, getRegionCounter(Case)));
}
+ // Simplify is skipped while building the counters above: it can get really
+ // slow on top of switches with thousands of cases. Instead, trigger
+ // simplification by adding zero to the last counter.
+ CaseCountSum = addCounters(CaseCountSum, Counter::getZero());
// If no explicit default case exists, create a branch region to represent
// the hidden branch, which will be added later by the CodeGen. This region
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
