[Coverage] Resurrect Branch:FalseCnt in SwitchStmt that was pruned in #112694 #120418
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I missed that FalseCnt for each Case was used to calculate percentage in the SwitchStmt. At the moment I resurrect them. In `!HasDefaultCase`, the pair of Counters shall be `[CaseCountSum, FalseCnt]`. (Reversal of before #112694) I think it can be considered as the False count on SwitchStmt. FalseCnt shall be folded (same as current impl) in the coming SingleByteCoverage changes, since percentage would not make sense.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: NAKAMURA Takumi (chapuni) ChangesI missed that FalseCnt for each Case was used to calculate percentage in the SwitchStmt. At the moment I resurrect them. In FalseCnt shall be folded (same as current impl) in the coming SingleByteCoverage changes, since percentage would not make sense. Full diff: https://github.com/llvm/llvm-project/pull/120418.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 7248abe480cba8..cda218eac34af8 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1146,12 +1146,15 @@ struct CounterCoverageMappingBuilder
/// Create a Branch Region around a SwitchCase for code coverage
/// and add it to the function's SourceRegions.
- void createSwitchCaseRegion(const SwitchCase *SC, Counter TrueCnt) {
+ /// Returns Counter that corresponds to SC.
+ Counter createSwitchCaseRegion(const SwitchCase *SC, Counter ParentCount) {
// Push region onto RegionStack but immediately pop it (which adds it to
// the function's SourceRegions) because it doesn't apply to any other
// source other than the SwitchCase.
+ Counter TrueCnt = getRegionCounter(SC);
popRegions(pushRegion(TrueCnt, getStart(SC), SC->getColonLoc(),
- Counter::getZero()));
+ subtractCounters(ParentCount, TrueCnt)));
+ return TrueCnt;
}
/// Check whether a region with bounds \c StartLoc and \c EndLoc
@@ -1863,16 +1866,22 @@ struct CounterCoverageMappingBuilder
const SwitchCase *Case = S->getSwitchCaseList();
for (; Case; Case = Case->getNextSwitchCase()) {
HasDefaultCase = HasDefaultCase || isa<DefaultStmt>(Case);
- auto CaseCount = getRegionCounter(Case);
+ auto CaseCount = createSwitchCaseRegion(Case, ParentCount);
CaseCountSum = addCounters(CaseCountSum, CaseCount, /*Simplify=*/false);
- createSwitchCaseRegion(Case, CaseCount);
}
// 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
// will be associated with the switch statement's condition.
if (!HasDefaultCase) {
- Counter DefaultCount = subtractCounters(ParentCount, CaseCountSum);
- createBranchRegion(S->getCond(), Counter::getZero(), DefaultCount);
+ // 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(), /*Simplify=*/true);
+
+ // This is considered as the False count on SwitchStmt.
+ Counter SwitchFalse = subtractCounters(ParentCount, CaseCountSum);
+ createBranchRegion(S->getCond(), CaseCountSum, SwitchFalse);
}
}
diff --git a/clang/test/CoverageMapping/switch.cpp b/clang/test/CoverageMapping/switch.cpp
index a1fee644faaf0e..db4cddbc6b9415 100644
--- a/clang/test/CoverageMapping/switch.cpp
+++ b/clang/test/CoverageMapping/switch.cpp
@@ -2,13 +2,13 @@
// CHECK: foo
void foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+11]]:2 = #0
- switch(i) { // CHECK-NEXT: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = 0, ((#0 - #2) - #3)
+ switch(i) { // CHECK-NEXT: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = (#2 + #3), ((#0 - #2) - #3)
// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+5]]:10 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:11 = #2
- return; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #2, 0
+ return; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #2, (#0 - #2)
// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:12 -> [[@LINE+1]]:3 = 0
case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #3
- break; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #3, 0
+ break; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #3, (#0 - #3)
} // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE+1]]:3 = #1
int x = 0; // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #1
@@ -28,14 +28,14 @@ void bar(int i) { // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+21]]:2 = #0
nop(); // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:11 -> [[@LINE-1]]:12 = 0, #2
// CHECK-NEXT: File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:10 = 0
switch (i) // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+11]]:2 = #3
- case 1: // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:11 -> [[@LINE-1]]:12 = 0, (#3 - #5)
+ case 1: // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:11 -> [[@LINE-1]]:12 = #5, (#3 - #5)
// CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE+1]]:10 = #5
- nop(); // CHECK-NEXT: Branch,File 0, [[@LINE-2]]:3 -> [[@LINE-2]]:9 = #5, 0
+ nop(); // CHECK-NEXT: Branch,File 0, [[@LINE-2]]:3 -> [[@LINE-2]]:9 = #5, (#3 - #5)
// CHECK-NEXT: File 0, [[@LINE+1]]:3 -> [[@LINE+7]]:2 = #4
- switch (i) { // CHECK-NEXT: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:12 = 0, (#4 - #7)
+ switch (i) { // CHECK-NEXT: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:12 = #7, (#4 - #7)
nop(); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+2]]:10 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #7
- nop(); // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #7, 0
+ nop(); // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #7, (#4 - #7)
}
nop(); // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #6
}
@@ -53,35 +53,35 @@ int main() { // CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+39]]:2 = #0
int i = 0;
switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+8]]:10 = 0
case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #2
- i = 1; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #2, 0
+ i = 1; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #2, (#0 - #2)
break; // CHECK-NEXT: Gap,File 0, [[@LINE]]:11 -> [[@LINE+1]]:3 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #3
- i = 2; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #3, 0
+ i = 2; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #3, (#0 - #3)
break; // CHECK-NEXT: Gap,File 0, [[@LINE]]:11 -> [[@LINE+1]]:3 = 0
default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #4
- break; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = #4, 0
+ break; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = #4, (#0 - #4)
} // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE+1]]:3 = #1
switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+27]]:2 = #1
case 0: // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+7]]:10 = 0
i = 1; // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE+1]]:10 = #6
- break; // CHECK-NEXT: Branch,File 0, [[@LINE-2]]:3 -> [[@LINE-2]]:9 = #6, 0
+ break; // CHECK-NEXT: Branch,File 0, [[@LINE-2]]:3 -> [[@LINE-2]]:9 = #6, (#1 - #6)
// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:11 -> [[@LINE+1]]:3 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = #7
- i = 2; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #7, 0
+ i = 2; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #7, (#1 - #7)
default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = (#7 + #8)
- break; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = #8, 0
+ break; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = #8, (#1 - #8)
} // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE+2]]:3 = #5
// CHECK-NEXT: File 0, [[@LINE+1]]:3 -> [[@LINE+17]]:2 = #5
- switch(i) { // CHECK-NEXT: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = 0, ((((#5 - #10) - #11) - #12) - #13)
+ switch(i) { // CHECK-NEXT: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = (((#10 + #11) + #12) + #13), ((((#5 - #10) - #11) - #12) - #13)
// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+8]]:11 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+7]]:11 = #10
- // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #10, 0
+ // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #10, (#5 - #10)
case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+5]]:11 = (#10 + #11)
- i = 11; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #11, 0
+ i = 11; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #11, (#5 - #11)
case 3: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:11 = ((#10 + #11) + #12)
- // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #12, 0
+ // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #12, (#5 - #12)
case 4: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:11 = (((#10 + #11) + #12) + #13)
- i = 99; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #13, 0
+ i = 99; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #13, (#5 - #13)
}
foo(1); // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:11 = #9
@@ -95,10 +95,10 @@ int pr44011(int i) { // CHECK-NEXT: File 0, [[@LINE]]:20 -> {{.*}}:2 = #0
switch (i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+6]]:13 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 = #2
- return 0; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #2, 0
+ return 0; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #2, (#0 - #2)
// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:3 = 0
default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 = #3
- return 1; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = #3, 0
+ return 1; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = #3, (#0 - #3)
}
} // A region for counter #1 is missing due to the missing return.
@@ -106,17 +106,17 @@ int pr44011(int i) { // CHECK-NEXT: File 0, [[@LINE]]:20 -> {{.*}}:2 = #0
// FIXME: End location for "case 1" shouldn't point at the end of the switch.
// CHECK: fallthrough
int fallthrough(int i) { // CHECK-NEXT: File 0, [[@LINE]]:24 -> [[@LINE+14]]:2 = #0
- // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:11 = 0, ((((#0 - #2) - #3) - #4) - #5)
+ // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:11 = (((#2 + #3) + #4) + #5), ((((#0 - #2) - #3) - #4) - #5)
switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+10]]:10 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+9]]:10 = #2
- i = 23; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #2, 0
+ i = 23; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #2, (#0 - #2)
case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = (#2 + #3)
- i = 11; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #3, 0
+ i = 11; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #3, (#0 - #3)
break; // CHECK-NEXT: Gap,File 0, [[@LINE]]:11 -> [[@LINE+1]]:3 = 0
case 3: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+4]]:10 = #4
- // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #4, 0
+ // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #4, (#0 - #4)
case 4: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = (#4 + #5)
- i = 99; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #5, 0
+ i = 99; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #5, (#0 - #5)
break;
}
}
@@ -126,12 +126,12 @@ void abort(void) __attribute((noreturn));
int noret(int x) { // CHECK-NEXT: File 0, [[@LINE]]:18 -> [[@LINE+11]]:2
switch (x) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+8]]:14 = 0
default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:12
- abort(); // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = #2, 0
+ abort(); // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = #2, (#0 - #2)
// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+1]]:3 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13
- return 5; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #3, 0
+ return 5; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #3, (#0 - #3)
// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:3 = 0
case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:14
- return 10; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #4, 0
+ return 10; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #4, (#0 - #4)
}
}
diff --git a/clang/test/CoverageMapping/switchmacro.c b/clang/test/CoverageMapping/switchmacro.c
index 0696e7490cdf99..4c98cc7d9403a4 100644
--- a/clang/test/CoverageMapping/switchmacro.c
+++ b/clang/test/CoverageMapping/switchmacro.c
@@ -6,7 +6,7 @@
int foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:16 -> {{[0-9]+}}:2 = #0
switch (i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> {{[0-9]+}}:11 = 0
default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> {{[0-9]+}}:11 = #2
- // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = #2, 0
+ // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = #2, (#0 - #2)
if (i == 1) // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:15 = #2
// CHECK-NEXT: Branch,File 0, [[@LINE-1]]:9 -> [[@LINE-1]]:15 = #3, (#2 - #3)
return 0; // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:15 = #3
@@ -15,7 +15,7 @@ int foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:16 -> {{[0-9]+}}:2 = #0
// CHECK-NEXT: File 0, [[@LINE+1]]:8 -> {{[0-9]+}}:11 = (#2 - #3)
FOO(1);
case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:13 = ((#2 + #4) - #3)
- // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #4, 0
+ // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #4, (#0 - #4)
return 2; // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+4]]:3 = 0
// CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:3 -> [[@LINE+2]]:6 = 0
|
evodius96
approved these changes
Dec 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I missed that FalseCnt for each Case was used to calculate percentage in the SwitchStmt. At the moment I resurrect them.
In
!HasDefaultCase, the pair of Counters shall be[CaseCountSum, FalseCnt]. (Reversal of before #112694)I think it can be considered as the False count on SwitchStmt.
FalseCnt shall be folded (same as current impl) in the coming SingleByteCoverage changes, since percentage would not make sense.