Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Coverage] Introduce "partial fold" on BranchRegion #112694

Merged
merged 2 commits into from
Oct 20, 2024

Conversation

chapuni
Copy link
Contributor

@chapuni chapuni commented Oct 17, 2024

Currently both True/False counts were folded. It lost the information, "It is True or False before folding." It prevented recalling branch counts in merging template instantiations.

In llvm-cov, a folded branch is shown as:

  • [True: n, Folded]
  • [Folded, False n]

In the case If n is zero, a branch is reported as "uncovered". This is distinguished from "folded" branch. When folded branches are merged, Folded may be dissolved.

In the coverage map, either Counter is Zero. Currently both were Zero.

Since "partial fold" has been introduced, either case in switch is omitted as Folded.

Each case: in switch is reported as [True: n, Folded], since False count doesn't show meaningful value.

When switch doesn't have default:, switch (Cond) is reported as [Folded, False: n], since True count was just the sum of case(s). switch with default can be considered as "the statement that doesn't have any False(s)".

Currently both True/False counts were folded. It lost the information,
"It is True or False before folding." It prevented recalling branch
counts in merging template instantiations.

In `llvm-cov`, a folded branch is shown as:

- `[True: n, Folded]`
- `[Folded, False n]`

In the case If `n` is zero, a branch is reported as "uncovered". This
is distinguished from "folded" branch. When folded branches are
merged, `Folded` may be dissolved.

In the coverage map, either `Counter` is `Zero`. Currently both were
`Zero`.

Since "partial fold" has been introduced, either case in `switch` is
omitted as `Folded`.

Each `case:` in `switch` is reported as `[True: n, Folded]`, since
`False` count doesn't show meaningful value.

When `switch` doesn't have `default:`, `switch (Cond)` is reported as
`[Folded, False: n]`, since `True` count was just the sum of
`case`(s). `switch` with `default` can be considered as "the statement
that doesn't have any `False`(s)".
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen PGO Profile Guided Optimizations labels Oct 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: NAKAMURA Takumi (chapuni)

Changes

Currently both True/False counts were folded. It lost the information, "It is True or False before folding." It prevented recalling branch counts in merging template instantiations.

In llvm-cov, a folded branch is shown as:

  • [True: n, Folded]
  • [Folded, False n]

In the case If n is zero, a branch is reported as "uncovered". This is distinguished from "folded" branch. When folded branches are merged, Folded may be dissolved.

In the coverage map, either Counter is Zero. Currently both were Zero.

Since "partial fold" has been introduced, either case in switch is omitted as Folded.

Each case: in switch is reported as [True: n, Folded], since False count doesn't show meaningful value.

When switch doesn't have default:, switch (Cond) is reported as [Folded, False: n], since True count was just the sum of case(s). switch with default can be considered as "the statement that doesn't have any False(s)".


Patch is 45.48 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/112694.diff

16 Files Affected:

  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+17-29)
  • (modified) clang/test/CoverageMapping/branch-constfolded.cpp (+20-20)
  • (modified) clang/test/CoverageMapping/if.cpp (+2-2)
  • (modified) clang/test/CoverageMapping/macro-expansion.c (+5-5)
  • (modified) clang/test/CoverageMapping/mcdc-scratch-space.c (+2-2)
  • (modified) clang/test/CoverageMapping/mcdc-system-headers.cpp (+2-2)
  • (modified) clang/test/CoverageMapping/switch.cpp (+32-32)
  • (modified) clang/test/CoverageMapping/switchmacro.c (+2-2)
  • (modified) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+8-7)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+1-1)
  • (modified) llvm/test/tools/llvm-cov/branch-c-general.test (+7-7)
  • (modified) llvm/tools/llvm-cov/CoverageExporterJson.cpp (+1-1)
  • (modified) llvm/tools/llvm-cov/CoverageExporterLcov.cpp (+1-1)
  • (modified) llvm/tools/llvm-cov/CoverageSummaryInfo.cpp (+12-12)
  • (modified) llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp (+30-21)
  • (modified) llvm/tools/llvm-cov/SourceCoverageViewText.cpp (+27-20)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 07015834bc84f3..a9a3eb91a6af27 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1098,12 +1098,6 @@ struct CounterCoverageMappingBuilder
     return ExitCount;
   }
 
-  /// Determine whether the given condition can be constant folded.
-  bool ConditionFoldsToBool(const Expr *Cond) {
-    Expr::EvalResult Result;
-    return (Cond->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext()));
-  }
-
   /// Create a Branch Region around an instrumentable condition for coverage
   /// and add it to the function's SourceRegions.  A branch region tracks a
   /// "True" counter and a "False" counter for boolean expressions that
@@ -1133,13 +1127,15 @@ struct CounterCoverageMappingBuilder
       // Alternatively, we can prevent any optimization done via
       // constant-folding by ensuring that ConstantFoldsToSimpleInteger() in
       // CodeGenFunction.c always returns false, but that is very heavy-handed.
-      if (ConditionFoldsToBool(C))
-        popRegions(pushRegion(Counter::getZero(), getStart(C), getEnd(C),
-                              Counter::getZero(), BranchParams));
-      else
-        // Otherwise, create a region with the True counter and False counter.
-        popRegions(pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt,
-                              BranchParams));
+      Expr::EvalResult Result;
+      if (C->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext())) {
+        if (Result.Val.getInt().getBoolValue())
+          FalseCnt = Counter::getZero();
+        else
+          TrueCnt = Counter::getZero();
+      }
+      popRegions(
+          pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt, BranchParams));
     }
   }
 
@@ -1153,12 +1149,12 @@ 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,
-                              Counter FalseCnt) {
+  void createSwitchCaseRegion(const SwitchCase *SC, Counter TrueCnt) {
     // 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.
-    popRegions(pushRegion(TrueCnt, getStart(SC), SC->getColonLoc(), FalseCnt));
+    popRegions(pushRegion(TrueCnt, getStart(SC), SC->getColonLoc(),
+                          Counter::getZero()));
   }
 
   /// Check whether a region with bounds \c StartLoc and \c EndLoc
@@ -1870,24 +1866,16 @@ struct CounterCoverageMappingBuilder
     const SwitchCase *Case = S->getSwitchCaseList();
     for (; Case; Case = Case->getNextSwitchCase()) {
       HasDefaultCase = HasDefaultCase || isa<DefaultStmt>(Case);
-      CaseCountSum =
-          addCounters(CaseCountSum, getRegionCounter(Case), /*Simplify=*/false);
-      createSwitchCaseRegion(
-          Case, getRegionCounter(Case),
-          subtractCounters(ParentCount, getRegionCounter(Case)));
+      auto CaseCount = getRegionCounter(Case);
+      CaseCountSum = addCounters(CaseCountSum, CaseCount, /*Simplify=*/false);
+      createSwitchCaseRegion(Case, CaseCount);
     }
-    // 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
     // will be associated with the switch statement's condition.
     if (!HasDefaultCase) {
-      Counter DefaultTrue = subtractCounters(ParentCount, CaseCountSum);
-      Counter DefaultFalse = subtractCounters(ParentCount, DefaultTrue);
-      createBranchRegion(S->getCond(), DefaultTrue, DefaultFalse);
+      Counter DefaultCount = subtractCounters(ParentCount, CaseCountSum);
+      createBranchRegion(S->getCond(), Counter::getZero(), DefaultCount);
     }
   }
 
diff --git a/clang/test/CoverageMapping/branch-constfolded.cpp b/clang/test/CoverageMapping/branch-constfolded.cpp
index 1e7e32808e8382..a2ac1c1eacd28f 100644
--- a/clang/test/CoverageMapping/branch-constfolded.cpp
+++ b/clang/test/CoverageMapping/branch-constfolded.cpp
@@ -5,94 +5,94 @@
 
 // CHECK-LABEL: _Z6fand_0b:
 bool fand_0(bool a) {      // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:20 = M:3, C:2
-  return false && a;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, 0
+  return false && a;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, (#0 - #1)
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:19 -> [[@LINE-1]]:20 = #2, (#1 - #2)
 
 // CHECK-LABEL: _Z6fand_1b:
 bool fand_1(bool a) {      // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:19 = M:3, C:2
   return a && true;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = #1, (#0 - #1)
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = #2, 0
 
 // CHECK-LABEL: _Z6fand_2bb:
 bool fand_2(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:25 = M:4, C:3
-  return false && a && b;  // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, 0
+  return false && a && b;  // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, (#0 - #3)
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:19 -> [[@LINE-1]]:20 = #4, (#3 - #4)
                            // CHECK: Branch,File 0, [[@LINE-2]]:24 -> [[@LINE-2]]:25 = #2, (#1 - #2)
 
 // CHECK-LABEL: _Z6fand_3bb:
 bool fand_3(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:24 = M:4, C:3
   return a && true && b;   // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = #3, (#0 - #3)
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = #4, 0
                            // CHECK: Branch,File 0, [[@LINE-2]]:23 -> [[@LINE-2]]:24 = #2, (#1 - #2)
 
 // CHECK-LABEL: _Z6fand_4bb:
 bool fand_4(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:25 = M:4, C:3
   return a && b && false;  // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = #3, (#0 - #3)
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:16 = #4, (#3 - #4)
-                           // CHECK: Branch,File 0, [[@LINE-2]]:20 -> [[@LINE-2]]:25 = 0, 0
+                           // CHECK: Branch,File 0, [[@LINE-2]]:20 -> [[@LINE-2]]:25 = 0, (#1 - #2)
 
 // CHECK-LABEL: _Z6fand_5b:
 bool fand_5(bool a) {      // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:23 = M:3, C:2
-  return false && true;    // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, 0
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:19 -> [[@LINE-1]]:23 = 0, 0
+  return false && true;    // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, (#0 - #1)
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:19 -> [[@LINE-1]]:23 = #2, 0
 
 // CHECK-LABEL: _Z6fand_6b:
 bool fand_6(bool a) {      // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:19 = M:3, C:2
-  return true && a;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = 0, 0
+  return true && a;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = #1, 0
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:18 -> [[@LINE-1]]:19 = #2, (#1 - #2)
 
 // CHECK-LABEL: _Z6fand_7b:
 bool fand_7(bool a) {      // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:20 = M:3, C:2
   return a && false;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = #1, (#0 - #1)
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, (#1 - #2)
 
 // CHECK-LABEL: _Z5for_0b:
 bool for_0(bool a) {       // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:19 = M:3, C:2
-  return true || a;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = 0, 0
+  return true || a;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = (#0 - #1), 0
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:18 -> [[@LINE-1]]:19 = (#1 - #2), #2
 
 // CHECK-LABEL: _Z5for_1b:
 bool for_1(bool a) {       // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:20 = M:3, C:2
   return a || false;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = (#0 - #1), #1
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, #2
 
 // CHECK-LABEL: _Z5for_2bb:
 bool for_2(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:24 = M:4, C:3
-  return true || a || b;   // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = 0, 0
+  return true || a || b;   // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = (#0 - #3), 0
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:18 -> [[@LINE-1]]:19 = (#3 - #4), #4
                            // CHECK: Branch,File 0, [[@LINE-2]]:23 -> [[@LINE-2]]:24 = (#1 - #2), #2
 
 // CHECK-LABEL: _Z5for_3bb:
 bool for_3(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:25 = M:4, C:3
   return a || false || b;  // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = (#0 - #3), #3
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, #4
                            // CHECK: Branch,File 0, [[@LINE-2]]:24 -> [[@LINE-2]]:25 = (#1 - #2), #2
 
 // CHECK-LABEL: _Z5for_4bb:
 bool for_4(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:24 = M:4, C:3
   return a || b || true;   // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = (#0 - #3), #3
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:16 = (#3 - #4), #4
-                           // CHECK: Branch,File 0, [[@LINE-2]]:20 -> [[@LINE-2]]:24 = 0, 0
+                           // CHECK: Branch,File 0, [[@LINE-2]]:20 -> [[@LINE-2]]:24 = (#1 - #2), 0
 
 // CHECK-LABEL: _Z5for_5b:
 bool for_5(bool a) {       // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:23 = M:3, C:2
-  return true || false;    // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = 0, 0
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:18 -> [[@LINE-1]]:23 = 0, 0
+  return true || false;    // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = (#0 - #1), 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:18 -> [[@LINE-1]]:23 = 0, #2
 
 // CHECK-LABEL: _Z5for_6b:
 bool for_6(bool a) {       // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:20 = M:3, C:2
-  return false || a;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, 0
+  return false || a;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, #1
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:19 -> [[@LINE-1]]:20 = (#1 - #2), #2
 
 // CHECK-LABEL: _Z5for_7b:
 bool for_7(bool a) {       // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:19 = M:3, C:2
   return a || true;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = (#0 - #1), #1
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = (#1 - #2), 0
 
 // CHECK-LABEL: _Z5for_8b:
 bool for_8(bool a) {       // MCDC: Decision,File 0, [[@LINE+3]]:7 -> [[@LINE+3]]:20 = M:3, C:2
-                           // CHECK: Branch,File 0, [[@LINE+2]]:7 -> [[@LINE+2]]:11 = 0, 0
-                           // CHECK: Branch,File 0, [[@LINE+1]]:15 -> [[@LINE+1]]:20 = 0, 0
+                           // CHECK: Branch,File 0, [[@LINE+2]]:7 -> [[@LINE+2]]:11 = #2, 0
+                           // CHECK: Branch,File 0, [[@LINE+1]]:15 -> [[@LINE+1]]:20 = 0, (#2 - #3)
   if (true && false)
       return true;
   else
diff --git a/clang/test/CoverageMapping/if.cpp b/clang/test/CoverageMapping/if.cpp
index 445cdfc20e2aff..b6fd525e930f90 100644
--- a/clang/test/CoverageMapping/if.cpp
+++ b/clang/test/CoverageMapping/if.cpp
@@ -14,7 +14,7 @@ struct S {
 // CHECK-LABEL: _Z3foov:
                                 // CHECK-NEXT: [[@LINE+3]]:12 -> [[@LINE+8]]:2 = #0
                                 // CHECK-NEXT: [[@LINE+3]]:15 -> [[@LINE+3]]:19 = #0
-                                // CHECK-NEXT: Branch,File 0, [[@LINE+2]]:15 -> [[@LINE+2]]:19 = 0, 0
+                                // CHECK-NEXT: Branch,File 0, [[@LINE+2]]:15 -> [[@LINE+2]]:19 = #2, 0
 void foo() {                    // CHECK-NEXT: Gap,File 0, [[@LINE+1]]:21 -> [[@LINE+1]]:22 = #2
   if (int j = true ? nop()      // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = #2
                    : nop();     // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = (#0 - #2)
@@ -168,7 +168,7 @@ int main() {                    // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 =
   // GH-45481
   S s;                    
   s.the_prop = 0? 1 : 2;        // CHECK-NEXT: File 0, [[@LINE]]:16 -> [[@LINE]]:17 = #0
-                                // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:16 -> [[@LINE-1]]:17 = 0, 0
+                                // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:16 -> [[@LINE-1]]:17 = 0, (#0 - #7)
                                 // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:18 -> [[@LINE-2]]:19 = #7
                                 // CHECK-NEXT: File 0, [[@LINE-3]]:19 -> [[@LINE-3]]:20 = #7
                                 // CHECK-NEXT: File 0, [[@LINE-4]]:23 -> [[@LINE-4]]:24 = (#0 - #7)
diff --git a/clang/test/CoverageMapping/macro-expansion.c b/clang/test/CoverageMapping/macro-expansion.c
index ad71fb15eda423..4cd2c934371931 100644
--- a/clang/test/CoverageMapping/macro-expansion.c
+++ b/clang/test/CoverageMapping/macro-expansion.c
@@ -4,29 +4,29 @@
 // CHECK:      File 1, [[@LINE+7]]:12 -> [[@LINE+7]]:38 = #0
 // CHECK-NEXT: File 1, [[@LINE+6]]:15 -> [[@LINE+6]]:28 = (#0 + #2)
 // CHECK-NEXT: File 1, [[@LINE+5]]:21 -> [[@LINE+5]]:22 = (#0 + #2)
-// CHECK: Branch,File 1, [[@LINE+4]]:21 -> [[@LINE+4]]:22 = 0, 0
+// CHECK: Branch,File 1, [[@LINE+4]]:21 -> [[@LINE+4]]:22 = 0, ((#0 + #2) - #3)
 // CHECK-NEXT: File 1, [[@LINE+3]]:24 -> [[@LINE+3]]:26 = #3
 // CHECK-NEXT: File 1, [[@LINE+2]]:36 -> [[@LINE+2]]:37 = (#0 + #2)
-// CHECK-NEXT: Branch,File 1, [[@LINE+1]]:36 -> [[@LINE+1]]:37 = 0, 0
+// CHECK-NEXT: Branch,File 1, [[@LINE+1]]:36 -> [[@LINE+1]]:37 = 0, #0
 #define M1 do { if (0) {} } while (0)
 // CHECK-NEXT: File 2, [[@LINE+12]]:15 -> [[@LINE+12]]:41 = #0
 // CHECK-NEXT: File 2, [[@LINE+11]]:18 -> [[@LINE+11]]:31 = (#0 + #4)
 // CHECK-NEXT: File 2, [[@LINE+10]]:24 -> [[@LINE+10]]:25 = (#0 + #4)
 // CHECK: File 2, [[@LINE+9]]:27 -> [[@LINE+9]]:29 = #5
 // CHECK-NEXT: File 2, [[@LINE+8]]:39 -> [[@LINE+8]]:40 = (#0 + #4)
-// CHECK-NEXT: Branch,File 2, [[@LINE+7]]:39 -> [[@LINE+7]]:40 = 0, 0
+// CHECK-NEXT: Branch,File 2, [[@LINE+7]]:39 -> [[@LINE+7]]:40 = 0, #0
 // CHECK-NEXT: File 3, [[@LINE+6]]:15 -> [[@LINE+6]]:41 = #0
 // CHECK-NEXT: File 3, [[@LINE+5]]:18 -> [[@LINE+5]]:31 = (#0 + #6)
 // CHECK-NEXT: File 3, [[@LINE+4]]:24 -> [[@LINE+4]]:25 = (#0 + #6)
 // CHECK: File 3, [[@LINE+3]]:27 -> [[@LINE+3]]:29 = #7
 // CHECK-NEXT: File 3, [[@LINE+2]]:39 -> [[@LINE+2]]:40 = (#0 + #6)
-// CHECK-NEXT: Branch,File 3, [[@LINE+1]]:39 -> [[@LINE+1]]:40 = 0, 0
+// CHECK-NEXT: Branch,File 3, [[@LINE+1]]:39 -> [[@LINE+1]]:40 = 0, #0
 #define M2(x) do { if (x) {} } while (0)
 // CHECK-NEXT: File 4, [[@LINE+5]]:15 -> [[@LINE+5]]:38 = #0
 // CHECK-NEXT: File 4, [[@LINE+4]]:18 -> [[@LINE+4]]:28 = (#0 + #8)
 // CHECK-NEXT: Expansion,File 4, [[@LINE+3]]:20 -> [[@LINE+3]]:22 = (#0 + #8)
 // CHECK-NEXT: File 4, [[@LINE+2]]:36 -> [[@LINE+2]]:37 = (#0 + #8)
-// CHECK-NEXT: Branch,File 4, [[@LINE+1]]:36 -> [[@LINE+1]]:37 = 0, 0
+// CHECK-NEXT: Branch,File 4, [[@LINE+1]]:36 -> [[@LINE+1]]:37 = 0, #0
 #define M3(x) do { M2(x); } while (0)
 // CHECK-NEXT: File 5, [[@LINE+4]]:15 -> [[@LINE+4]]:27 = #0
 // CHECK-NEXT: File 5, [[@LINE+3]]:16 -> [[@LINE+3]]:19 = #0
diff --git a/clang/test/CoverageMapping/mcdc-scratch-space.c b/clang/test/CoverageMapping/mcdc-scratch-space.c
index a263e9b688faed..60e456948a5182 100644
--- a/clang/test/CoverageMapping/mcdc-scratch-space.c
+++ b/clang/test/CoverageMapping/mcdc-scratch-space.c
@@ -3,7 +3,7 @@
 // CHECK: builtin_macro0:
 int builtin_macro0(int a) {
   // CHECK: Decision,File 0, [[@LINE+1]]:11 -> [[@LINE+2]]:15 = M:3, C:2
-  return (__LINE__ // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:11 = 0, 0 [1,2,0]
+  return (__LINE__ // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:11 = #1, 0 [1,2,0]
           && a); //   CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:15 = #2, (#1 - #2) [2,0,0]
 }
 
@@ -11,7 +11,7 @@ int builtin_macro0(int a) {
 int builtin_macro1(int a) {
   // CHECK: Decision,File 0, [[@LINE+1]]:11 -> [[@LINE+2]]:22 = M:3, C:2
   return (a // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:12 = (#0 - #1), #1 [1,0,2]
-          || __LINE__); // CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:14 = 0, 0 [2,0,0]
+          || __LINE__); // CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:14 = (#1 - #2), 0 [2,0,0]
 }
 
 #define PRE(x) pre_##x
diff --git a/clang/test/CoverageMapping/mcdc-system-headers.cpp b/clang/test/CoverageMapping/mcdc-system-headers.cpp
index ae26ed5fe469f2..cb1c8743c36e82 100644
--- a/clang/test/CoverageMapping/mcdc-system-headers.cpp
+++ b/clang/test/CoverageMapping/mcdc-system-headers.cpp
@@ -17,10 +17,10 @@
 int func0(int a) {
   // CHECK: Decision,File 0, [[@LINE+3]]:11 -> [[@LINE+3]]:21 = M:3, C:2
   // W_SYS: Expansion,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:16 = #0 (Expanded file = 1)
-  // X_SYS: Branch,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:11 = 0, 0 [1,2,0]
+  // X_SYS: Branch,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:11 = #1, 0 [1,2,0]
   return (CONST && a);
   // CHECK: Branch,File 0, [[@LINE-1]]:20 -> [[@LINE-1]]:21 = #2, (#1 - #2) [2,0,0]
-  // W_SYS: Branch,File 1, [[@LINE-16]]:15 -> [[@LINE-16]]:17 = 0, 0 [1,2,0]
+  // W_SYS: Branch,File 1, [[@LINE-16]]:15 -> [[@LINE-16]]:17 = #1, 0 [1,2,0]
 }
 
 // CHECK: _Z5func1ii:
diff --git a/clang/test/CoverageMapping/switch.cpp b/clang/test/CoverageMapping/switch.cpp
index b47c0e80099527..a1fee644faaf0e 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 - #2) - #3), (#2 + #3)
+  switch(i) {       // CHECK-NEXT: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = 0, ((#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 - #2)
+    return;         // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #2, 0
                     // 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 - #3)
+    break;          // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #3, 0
 
   }                 // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE+1]]:3 = #1
   int x = 0;        // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #1
@@ -18,24 +18,24 @@ int nop() { return 0; }
 
                     // CHECK: bar
 void bar(int i) {   // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+21]]:2 = #0
-  switch (i)        // CHECK-NEXT: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:12 = #0, 0
+  switch (i)        //...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2024

@llvm/pr-subscribers-pgo

Author: NAKAMURA Takumi (chapuni)

Changes

Currently both True/False counts were folded. It lost the information, "It is True or False before folding." It prevented recalling branch counts in merging template instantiations.

In llvm-cov, a folded branch is shown as:

  • [True: n, Folded]
  • [Folded, False n]

In the case If n is zero, a branch is reported as "uncovered". This is distinguished from "folded" branch. When folded branches are merged, Folded may be dissolved.

In the coverage map, either Counter is Zero. Currently both were Zero.

Since "partial fold" has been introduced, either case in switch is omitted as Folded.

Each case: in switch is reported as [True: n, Folded], since False count doesn't show meaningful value.

When switch doesn't have default:, switch (Cond) is reported as [Folded, False: n], since True count was just the sum of case(s). switch with default can be considered as "the statement that doesn't have any False(s)".


Patch is 45.48 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/112694.diff

16 Files Affected:

  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+17-29)
  • (modified) clang/test/CoverageMapping/branch-constfolded.cpp (+20-20)
  • (modified) clang/test/CoverageMapping/if.cpp (+2-2)
  • (modified) clang/test/CoverageMapping/macro-expansion.c (+5-5)
  • (modified) clang/test/CoverageMapping/mcdc-scratch-space.c (+2-2)
  • (modified) clang/test/CoverageMapping/mcdc-system-headers.cpp (+2-2)
  • (modified) clang/test/CoverageMapping/switch.cpp (+32-32)
  • (modified) clang/test/CoverageMapping/switchmacro.c (+2-2)
  • (modified) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+8-7)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+1-1)
  • (modified) llvm/test/tools/llvm-cov/branch-c-general.test (+7-7)
  • (modified) llvm/tools/llvm-cov/CoverageExporterJson.cpp (+1-1)
  • (modified) llvm/tools/llvm-cov/CoverageExporterLcov.cpp (+1-1)
  • (modified) llvm/tools/llvm-cov/CoverageSummaryInfo.cpp (+12-12)
  • (modified) llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp (+30-21)
  • (modified) llvm/tools/llvm-cov/SourceCoverageViewText.cpp (+27-20)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 07015834bc84f3..a9a3eb91a6af27 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1098,12 +1098,6 @@ struct CounterCoverageMappingBuilder
     return ExitCount;
   }
 
-  /// Determine whether the given condition can be constant folded.
-  bool ConditionFoldsToBool(const Expr *Cond) {
-    Expr::EvalResult Result;
-    return (Cond->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext()));
-  }
-
   /// Create a Branch Region around an instrumentable condition for coverage
   /// and add it to the function's SourceRegions.  A branch region tracks a
   /// "True" counter and a "False" counter for boolean expressions that
@@ -1133,13 +1127,15 @@ struct CounterCoverageMappingBuilder
       // Alternatively, we can prevent any optimization done via
       // constant-folding by ensuring that ConstantFoldsToSimpleInteger() in
       // CodeGenFunction.c always returns false, but that is very heavy-handed.
-      if (ConditionFoldsToBool(C))
-        popRegions(pushRegion(Counter::getZero(), getStart(C), getEnd(C),
-                              Counter::getZero(), BranchParams));
-      else
-        // Otherwise, create a region with the True counter and False counter.
-        popRegions(pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt,
-                              BranchParams));
+      Expr::EvalResult Result;
+      if (C->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext())) {
+        if (Result.Val.getInt().getBoolValue())
+          FalseCnt = Counter::getZero();
+        else
+          TrueCnt = Counter::getZero();
+      }
+      popRegions(
+          pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt, BranchParams));
     }
   }
 
@@ -1153,12 +1149,12 @@ 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,
-                              Counter FalseCnt) {
+  void createSwitchCaseRegion(const SwitchCase *SC, Counter TrueCnt) {
     // 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.
-    popRegions(pushRegion(TrueCnt, getStart(SC), SC->getColonLoc(), FalseCnt));
+    popRegions(pushRegion(TrueCnt, getStart(SC), SC->getColonLoc(),
+                          Counter::getZero()));
   }
 
   /// Check whether a region with bounds \c StartLoc and \c EndLoc
@@ -1870,24 +1866,16 @@ struct CounterCoverageMappingBuilder
     const SwitchCase *Case = S->getSwitchCaseList();
     for (; Case; Case = Case->getNextSwitchCase()) {
       HasDefaultCase = HasDefaultCase || isa<DefaultStmt>(Case);
-      CaseCountSum =
-          addCounters(CaseCountSum, getRegionCounter(Case), /*Simplify=*/false);
-      createSwitchCaseRegion(
-          Case, getRegionCounter(Case),
-          subtractCounters(ParentCount, getRegionCounter(Case)));
+      auto CaseCount = getRegionCounter(Case);
+      CaseCountSum = addCounters(CaseCountSum, CaseCount, /*Simplify=*/false);
+      createSwitchCaseRegion(Case, CaseCount);
     }
-    // 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
     // will be associated with the switch statement's condition.
     if (!HasDefaultCase) {
-      Counter DefaultTrue = subtractCounters(ParentCount, CaseCountSum);
-      Counter DefaultFalse = subtractCounters(ParentCount, DefaultTrue);
-      createBranchRegion(S->getCond(), DefaultTrue, DefaultFalse);
+      Counter DefaultCount = subtractCounters(ParentCount, CaseCountSum);
+      createBranchRegion(S->getCond(), Counter::getZero(), DefaultCount);
     }
   }
 
diff --git a/clang/test/CoverageMapping/branch-constfolded.cpp b/clang/test/CoverageMapping/branch-constfolded.cpp
index 1e7e32808e8382..a2ac1c1eacd28f 100644
--- a/clang/test/CoverageMapping/branch-constfolded.cpp
+++ b/clang/test/CoverageMapping/branch-constfolded.cpp
@@ -5,94 +5,94 @@
 
 // CHECK-LABEL: _Z6fand_0b:
 bool fand_0(bool a) {      // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:20 = M:3, C:2
-  return false && a;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, 0
+  return false && a;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, (#0 - #1)
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:19 -> [[@LINE-1]]:20 = #2, (#1 - #2)
 
 // CHECK-LABEL: _Z6fand_1b:
 bool fand_1(bool a) {      // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:19 = M:3, C:2
   return a && true;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = #1, (#0 - #1)
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = #2, 0
 
 // CHECK-LABEL: _Z6fand_2bb:
 bool fand_2(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:25 = M:4, C:3
-  return false && a && b;  // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, 0
+  return false && a && b;  // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, (#0 - #3)
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:19 -> [[@LINE-1]]:20 = #4, (#3 - #4)
                            // CHECK: Branch,File 0, [[@LINE-2]]:24 -> [[@LINE-2]]:25 = #2, (#1 - #2)
 
 // CHECK-LABEL: _Z6fand_3bb:
 bool fand_3(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:24 = M:4, C:3
   return a && true && b;   // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = #3, (#0 - #3)
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = #4, 0
                            // CHECK: Branch,File 0, [[@LINE-2]]:23 -> [[@LINE-2]]:24 = #2, (#1 - #2)
 
 // CHECK-LABEL: _Z6fand_4bb:
 bool fand_4(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:25 = M:4, C:3
   return a && b && false;  // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = #3, (#0 - #3)
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:16 = #4, (#3 - #4)
-                           // CHECK: Branch,File 0, [[@LINE-2]]:20 -> [[@LINE-2]]:25 = 0, 0
+                           // CHECK: Branch,File 0, [[@LINE-2]]:20 -> [[@LINE-2]]:25 = 0, (#1 - #2)
 
 // CHECK-LABEL: _Z6fand_5b:
 bool fand_5(bool a) {      // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:23 = M:3, C:2
-  return false && true;    // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, 0
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:19 -> [[@LINE-1]]:23 = 0, 0
+  return false && true;    // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, (#0 - #1)
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:19 -> [[@LINE-1]]:23 = #2, 0
 
 // CHECK-LABEL: _Z6fand_6b:
 bool fand_6(bool a) {      // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:19 = M:3, C:2
-  return true && a;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = 0, 0
+  return true && a;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = #1, 0
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:18 -> [[@LINE-1]]:19 = #2, (#1 - #2)
 
 // CHECK-LABEL: _Z6fand_7b:
 bool fand_7(bool a) {      // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:20 = M:3, C:2
   return a && false;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = #1, (#0 - #1)
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, (#1 - #2)
 
 // CHECK-LABEL: _Z5for_0b:
 bool for_0(bool a) {       // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:19 = M:3, C:2
-  return true || a;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = 0, 0
+  return true || a;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = (#0 - #1), 0
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:18 -> [[@LINE-1]]:19 = (#1 - #2), #2
 
 // CHECK-LABEL: _Z5for_1b:
 bool for_1(bool a) {       // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:20 = M:3, C:2
   return a || false;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = (#0 - #1), #1
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, #2
 
 // CHECK-LABEL: _Z5for_2bb:
 bool for_2(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:24 = M:4, C:3
-  return true || a || b;   // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = 0, 0
+  return true || a || b;   // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = (#0 - #3), 0
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:18 -> [[@LINE-1]]:19 = (#3 - #4), #4
                            // CHECK: Branch,File 0, [[@LINE-2]]:23 -> [[@LINE-2]]:24 = (#1 - #2), #2
 
 // CHECK-LABEL: _Z5for_3bb:
 bool for_3(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:25 = M:4, C:3
   return a || false || b;  // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = (#0 - #3), #3
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:20 = 0, #4
                            // CHECK: Branch,File 0, [[@LINE-2]]:24 -> [[@LINE-2]]:25 = (#1 - #2), #2
 
 // CHECK-LABEL: _Z5for_4bb:
 bool for_4(bool a, bool b) {// MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:24 = M:4, C:3
   return a || b || true;   // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = (#0 - #3), #3
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:16 = (#3 - #4), #4
-                           // CHECK: Branch,File 0, [[@LINE-2]]:20 -> [[@LINE-2]]:24 = 0, 0
+                           // CHECK: Branch,File 0, [[@LINE-2]]:20 -> [[@LINE-2]]:24 = (#1 - #2), 0
 
 // CHECK-LABEL: _Z5for_5b:
 bool for_5(bool a) {       // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:23 = M:3, C:2
-  return true || false;    // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = 0, 0
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:18 -> [[@LINE-1]]:23 = 0, 0
+  return true || false;    // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:14 = (#0 - #1), 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:18 -> [[@LINE-1]]:23 = 0, #2
 
 // CHECK-LABEL: _Z5for_6b:
 bool for_6(bool a) {       // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:20 = M:3, C:2
-  return false || a;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, 0
+  return false || a;       // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:15 = 0, #1
 }                          // CHECK: Branch,File 0, [[@LINE-1]]:19 -> [[@LINE-1]]:20 = (#1 - #2), #2
 
 // CHECK-LABEL: _Z5for_7b:
 bool for_7(bool a) {       // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:19 = M:3, C:2
   return a || true;        // CHECK: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = (#0 - #1), #1
-}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = 0, 0
+}                          // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = (#1 - #2), 0
 
 // CHECK-LABEL: _Z5for_8b:
 bool for_8(bool a) {       // MCDC: Decision,File 0, [[@LINE+3]]:7 -> [[@LINE+3]]:20 = M:3, C:2
-                           // CHECK: Branch,File 0, [[@LINE+2]]:7 -> [[@LINE+2]]:11 = 0, 0
-                           // CHECK: Branch,File 0, [[@LINE+1]]:15 -> [[@LINE+1]]:20 = 0, 0
+                           // CHECK: Branch,File 0, [[@LINE+2]]:7 -> [[@LINE+2]]:11 = #2, 0
+                           // CHECK: Branch,File 0, [[@LINE+1]]:15 -> [[@LINE+1]]:20 = 0, (#2 - #3)
   if (true && false)
       return true;
   else
diff --git a/clang/test/CoverageMapping/if.cpp b/clang/test/CoverageMapping/if.cpp
index 445cdfc20e2aff..b6fd525e930f90 100644
--- a/clang/test/CoverageMapping/if.cpp
+++ b/clang/test/CoverageMapping/if.cpp
@@ -14,7 +14,7 @@ struct S {
 // CHECK-LABEL: _Z3foov:
                                 // CHECK-NEXT: [[@LINE+3]]:12 -> [[@LINE+8]]:2 = #0
                                 // CHECK-NEXT: [[@LINE+3]]:15 -> [[@LINE+3]]:19 = #0
-                                // CHECK-NEXT: Branch,File 0, [[@LINE+2]]:15 -> [[@LINE+2]]:19 = 0, 0
+                                // CHECK-NEXT: Branch,File 0, [[@LINE+2]]:15 -> [[@LINE+2]]:19 = #2, 0
 void foo() {                    // CHECK-NEXT: Gap,File 0, [[@LINE+1]]:21 -> [[@LINE+1]]:22 = #2
   if (int j = true ? nop()      // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = #2
                    : nop();     // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = (#0 - #2)
@@ -168,7 +168,7 @@ int main() {                    // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 =
   // GH-45481
   S s;                    
   s.the_prop = 0? 1 : 2;        // CHECK-NEXT: File 0, [[@LINE]]:16 -> [[@LINE]]:17 = #0
-                                // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:16 -> [[@LINE-1]]:17 = 0, 0
+                                // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:16 -> [[@LINE-1]]:17 = 0, (#0 - #7)
                                 // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:18 -> [[@LINE-2]]:19 = #7
                                 // CHECK-NEXT: File 0, [[@LINE-3]]:19 -> [[@LINE-3]]:20 = #7
                                 // CHECK-NEXT: File 0, [[@LINE-4]]:23 -> [[@LINE-4]]:24 = (#0 - #7)
diff --git a/clang/test/CoverageMapping/macro-expansion.c b/clang/test/CoverageMapping/macro-expansion.c
index ad71fb15eda423..4cd2c934371931 100644
--- a/clang/test/CoverageMapping/macro-expansion.c
+++ b/clang/test/CoverageMapping/macro-expansion.c
@@ -4,29 +4,29 @@
 // CHECK:      File 1, [[@LINE+7]]:12 -> [[@LINE+7]]:38 = #0
 // CHECK-NEXT: File 1, [[@LINE+6]]:15 -> [[@LINE+6]]:28 = (#0 + #2)
 // CHECK-NEXT: File 1, [[@LINE+5]]:21 -> [[@LINE+5]]:22 = (#0 + #2)
-// CHECK: Branch,File 1, [[@LINE+4]]:21 -> [[@LINE+4]]:22 = 0, 0
+// CHECK: Branch,File 1, [[@LINE+4]]:21 -> [[@LINE+4]]:22 = 0, ((#0 + #2) - #3)
 // CHECK-NEXT: File 1, [[@LINE+3]]:24 -> [[@LINE+3]]:26 = #3
 // CHECK-NEXT: File 1, [[@LINE+2]]:36 -> [[@LINE+2]]:37 = (#0 + #2)
-// CHECK-NEXT: Branch,File 1, [[@LINE+1]]:36 -> [[@LINE+1]]:37 = 0, 0
+// CHECK-NEXT: Branch,File 1, [[@LINE+1]]:36 -> [[@LINE+1]]:37 = 0, #0
 #define M1 do { if (0) {} } while (0)
 // CHECK-NEXT: File 2, [[@LINE+12]]:15 -> [[@LINE+12]]:41 = #0
 // CHECK-NEXT: File 2, [[@LINE+11]]:18 -> [[@LINE+11]]:31 = (#0 + #4)
 // CHECK-NEXT: File 2, [[@LINE+10]]:24 -> [[@LINE+10]]:25 = (#0 + #4)
 // CHECK: File 2, [[@LINE+9]]:27 -> [[@LINE+9]]:29 = #5
 // CHECK-NEXT: File 2, [[@LINE+8]]:39 -> [[@LINE+8]]:40 = (#0 + #4)
-// CHECK-NEXT: Branch,File 2, [[@LINE+7]]:39 -> [[@LINE+7]]:40 = 0, 0
+// CHECK-NEXT: Branch,File 2, [[@LINE+7]]:39 -> [[@LINE+7]]:40 = 0, #0
 // CHECK-NEXT: File 3, [[@LINE+6]]:15 -> [[@LINE+6]]:41 = #0
 // CHECK-NEXT: File 3, [[@LINE+5]]:18 -> [[@LINE+5]]:31 = (#0 + #6)
 // CHECK-NEXT: File 3, [[@LINE+4]]:24 -> [[@LINE+4]]:25 = (#0 + #6)
 // CHECK: File 3, [[@LINE+3]]:27 -> [[@LINE+3]]:29 = #7
 // CHECK-NEXT: File 3, [[@LINE+2]]:39 -> [[@LINE+2]]:40 = (#0 + #6)
-// CHECK-NEXT: Branch,File 3, [[@LINE+1]]:39 -> [[@LINE+1]]:40 = 0, 0
+// CHECK-NEXT: Branch,File 3, [[@LINE+1]]:39 -> [[@LINE+1]]:40 = 0, #0
 #define M2(x) do { if (x) {} } while (0)
 // CHECK-NEXT: File 4, [[@LINE+5]]:15 -> [[@LINE+5]]:38 = #0
 // CHECK-NEXT: File 4, [[@LINE+4]]:18 -> [[@LINE+4]]:28 = (#0 + #8)
 // CHECK-NEXT: Expansion,File 4, [[@LINE+3]]:20 -> [[@LINE+3]]:22 = (#0 + #8)
 // CHECK-NEXT: File 4, [[@LINE+2]]:36 -> [[@LINE+2]]:37 = (#0 + #8)
-// CHECK-NEXT: Branch,File 4, [[@LINE+1]]:36 -> [[@LINE+1]]:37 = 0, 0
+// CHECK-NEXT: Branch,File 4, [[@LINE+1]]:36 -> [[@LINE+1]]:37 = 0, #0
 #define M3(x) do { M2(x); } while (0)
 // CHECK-NEXT: File 5, [[@LINE+4]]:15 -> [[@LINE+4]]:27 = #0
 // CHECK-NEXT: File 5, [[@LINE+3]]:16 -> [[@LINE+3]]:19 = #0
diff --git a/clang/test/CoverageMapping/mcdc-scratch-space.c b/clang/test/CoverageMapping/mcdc-scratch-space.c
index a263e9b688faed..60e456948a5182 100644
--- a/clang/test/CoverageMapping/mcdc-scratch-space.c
+++ b/clang/test/CoverageMapping/mcdc-scratch-space.c
@@ -3,7 +3,7 @@
 // CHECK: builtin_macro0:
 int builtin_macro0(int a) {
   // CHECK: Decision,File 0, [[@LINE+1]]:11 -> [[@LINE+2]]:15 = M:3, C:2
-  return (__LINE__ // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:11 = 0, 0 [1,2,0]
+  return (__LINE__ // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:11 = #1, 0 [1,2,0]
           && a); //   CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:15 = #2, (#1 - #2) [2,0,0]
 }
 
@@ -11,7 +11,7 @@ int builtin_macro0(int a) {
 int builtin_macro1(int a) {
   // CHECK: Decision,File 0, [[@LINE+1]]:11 -> [[@LINE+2]]:22 = M:3, C:2
   return (a // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:12 = (#0 - #1), #1 [1,0,2]
-          || __LINE__); // CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:14 = 0, 0 [2,0,0]
+          || __LINE__); // CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:14 = (#1 - #2), 0 [2,0,0]
 }
 
 #define PRE(x) pre_##x
diff --git a/clang/test/CoverageMapping/mcdc-system-headers.cpp b/clang/test/CoverageMapping/mcdc-system-headers.cpp
index ae26ed5fe469f2..cb1c8743c36e82 100644
--- a/clang/test/CoverageMapping/mcdc-system-headers.cpp
+++ b/clang/test/CoverageMapping/mcdc-system-headers.cpp
@@ -17,10 +17,10 @@
 int func0(int a) {
   // CHECK: Decision,File 0, [[@LINE+3]]:11 -> [[@LINE+3]]:21 = M:3, C:2
   // W_SYS: Expansion,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:16 = #0 (Expanded file = 1)
-  // X_SYS: Branch,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:11 = 0, 0 [1,2,0]
+  // X_SYS: Branch,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:11 = #1, 0 [1,2,0]
   return (CONST && a);
   // CHECK: Branch,File 0, [[@LINE-1]]:20 -> [[@LINE-1]]:21 = #2, (#1 - #2) [2,0,0]
-  // W_SYS: Branch,File 1, [[@LINE-16]]:15 -> [[@LINE-16]]:17 = 0, 0 [1,2,0]
+  // W_SYS: Branch,File 1, [[@LINE-16]]:15 -> [[@LINE-16]]:17 = #1, 0 [1,2,0]
 }
 
 // CHECK: _Z5func1ii:
diff --git a/clang/test/CoverageMapping/switch.cpp b/clang/test/CoverageMapping/switch.cpp
index b47c0e80099527..a1fee644faaf0e 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 - #2) - #3), (#2 + #3)
+  switch(i) {       // CHECK-NEXT: Branch,File 0, [[@LINE]]:10 -> [[@LINE]]:11 = 0, ((#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 - #2)
+    return;         // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #2, 0
                     // 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 - #3)
+    break;          // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = #3, 0
 
   }                 // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE+1]]:3 = #1
   int x = 0;        // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #1
@@ -18,24 +18,24 @@ int nop() { return 0; }
 
                     // CHECK: bar
 void bar(int i) {   // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+21]]:2 = #0
-  switch (i)        // CHECK-NEXT: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:12 = #0, 0
+  switch (i)        //...
[truncated]

// constant-folded branch.
if (Region.Count.isZero() && Region.FalseCount.isZero())
CountedBranchRegions.back().Folded = true;
CountedBranchRegions.back().TrueFolded = Region.Count.isZero();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename Region.Count to Region.TrueCount?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CounterMappingRegion::Count is used in non-Branch regions. (then, FalseCount may be N/A)

Similar cases can be seen in llvm::CountedRegion and Clang's SourceMappingRegion. So, I think it'd be our common sense for the region to have FalseCount optionally.

In contrast, Folded is specific to Branch region. So, I think TrueFolded would be less intrusive.

@@ -125,7 +125,7 @@ json::Array renderRegions(ArrayRef<coverage::CountedRegion> Regions) {
json::Array renderBranchRegions(ArrayRef<coverage::CountedRegion> Regions) {
json::Array RegionArray;
for (const auto &Region : Regions)
if (!Region.Folded)
if (!Region.TrueFolded || !Region.FalseFolded)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace with a helper function? e.g. Region.isFolded()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it would be less valuable, since it looks like CountedRegion is rather naked. It's clear for me. (Personally I prefer !(TrueFolded && FalseFolded) than DeMorganized)

That said, I can introduce isBothFolded(). I hope we wouldn't provide also isTrueFolded() and isFalseFolded().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we have to revisit how to show folding. Let me feed this to future tasks.

if (!BR.TrueFolded) {
// "True" Condition Branches.
++NumBranches;
if (BR.ExecutionCount > 0)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename ExecutionCount to TrueExecutionCount?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above, CountedRegion::ExecutionCount is used generally.

if (BR.ExecutionCount > 0)
++CoveredBranches;
}
if (!BR.FalseFolded) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be a lambda or static helper function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is enough short with expanded impl.

Copy link
Contributor Author

@chapuni chapuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for comments!

// constant-folded branch.
if (Region.Count.isZero() && Region.FalseCount.isZero())
CountedBranchRegions.back().Folded = true;
CountedBranchRegions.back().TrueFolded = Region.Count.isZero();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CounterMappingRegion::Count is used in non-Branch regions. (then, FalseCount may be N/A)

Similar cases can be seen in llvm::CountedRegion and Clang's SourceMappingRegion. So, I think it'd be our common sense for the region to have FalseCount optionally.

In contrast, Folded is specific to Branch region. So, I think TrueFolded would be less intrusive.

@@ -125,7 +125,7 @@ json::Array renderRegions(ArrayRef<coverage::CountedRegion> Regions) {
json::Array renderBranchRegions(ArrayRef<coverage::CountedRegion> Regions) {
json::Array RegionArray;
for (const auto &Region : Regions)
if (!Region.Folded)
if (!Region.TrueFolded || !Region.FalseFolded)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it would be less valuable, since it looks like CountedRegion is rather naked. It's clear for me. (Personally I prefer !(TrueFolded && FalseFolded) than DeMorganized)

That said, I can introduce isBothFolded(). I hope we wouldn't provide also isTrueFolded() and isFalseFolded().

if (!BR.TrueFolded) {
// "True" Condition Branches.
++NumBranches;
if (BR.ExecutionCount > 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above, CountedRegion::ExecutionCount is used generally.

@hanickadot
Copy link
Contributor

Do you have example before/after of the change? It's a bit hard to imagine.

@chapuni
Copy link
Contributor Author

chapuni commented Oct 18, 2024

@hanickadot I didn't think there'd be anything to imagine.

That said, I will arrange examples later. I still wonder how we could show "partial fold" more clearly and make it more consistent.

@hanickadot
Copy link
Contributor

I have no additional list of thing Jessica pointed out, LGTM

Expr::EvalResult Result;
if (C->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext())) {
if (Result.Val.getInt().getBoolValue())
FalseCnt = Counter::getZero();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I got this started by using Counter::getZero() to represent folded conditions. This decision overloaded the meaning a bit. We may want to consider using a more meaningful region flag or something with room to extend. Though that will impact the region encoding.

@evodius96
Copy link
Contributor

I think this is a meaningful enhancement to branch coverage. I don't have much to add to what's been said. LGTM. Thanks!

@chapuni chapuni merged commit 4a011ac into main Oct 20, 2024
5 of 8 checks passed
@chapuni chapuni deleted the users/chapuni/cov/single/partial branch October 20, 2024 03:30
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 20, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-amdgpu-runtime running on omp-vega20-0 while building clang,llvm at step 7 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/8580

Here is the relevant piece of the build log for the reference
Step 7 (Add check check-offload) failure: test (failure)
******************** TEST 'libomptarget :: amdgcn-amd-amdhsa :: sanitizer/kernel_crash_async.c' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 2
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp    -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib  -fopenmp-targets=amdgcn-amd-amdhsa -O3 /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa -O3 /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a
# note: command had no output on stdout or stderr
# RUN: at line 3
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/not --crash env -u LLVM_DISABLE_SYMBOLIZATION OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES=1 /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp 2>&1 | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c --check-prefixes=TRACE
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/not --crash env -u LLVM_DISABLE_SYMBOLIZATION OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES=1 /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp
# note: command had no output on stdout or stderr
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c --check-prefixes=TRACE
# note: command had no output on stdout or stderr
# RUN: at line 4
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/not --crash /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp 2>&1 | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c --check-prefixes=CHECK
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/not --crash /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp
# note: command had no output on stdout or stderr
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c --check-prefixes=CHECK
# note: command had no output on stdout or stderr
# RUN: at line 5
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp    -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib  -fopenmp-targets=amdgcn-amd-amdhsa -O3 /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a -g
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa -O3 /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a -g
# note: command had no output on stdout or stderr
# RUN: at line 6
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/not --crash env -u LLVM_DISABLE_SYMBOLIZATION OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES=1 /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp 2>&1 | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c --check-prefixes=TRACE
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/not --crash env -u LLVM_DISABLE_SYMBOLIZATION OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES=1 /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp
# note: command had no output on stdout or stderr
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c --check-prefixes=TRACE
# note: command had no output on stdout or stderr
# RUN: at line 7
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/not --crash /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp 2>&1 | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c --check-prefixes=CHECK
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/not --crash /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/kernel_crash_async.c.tmp
# note: command had no output on stdout or stderr
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c --check-prefixes=CHECK
# .---command stderr------------
# | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c:39:11: error: CHECK: expected string not found in input
# | // CHECK: Kernel {{[0-9]}}: {{.*}} (__omp_offloading_{{.*}}_main_l29)
# |           ^
# | <stdin>:1:1: note: scanning from here
# | Display only launched kernel:
# | ^
# | <stdin>:2:23: note: possible intended match here
# | Kernel 'omp target in main @ 29 (__omp_offloading_802_b38838e_main_l29)'
# |                       ^
# | 
# | Input file: <stdin>
# | Check file: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/sanitizer/kernel_crash_async.c
# | 
...

EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024
Currently both True/False counts were folded. It lost the information,
"It is True or False before folding." It prevented recalling branch
counts in merging template instantiations.

In `llvm-cov`, a folded branch is shown as:

- `[True: n, Folded]`
- `[Folded, False n]`

In the case If `n` is zero, a branch is reported as "uncovered". This is
distinguished from "folded" branch. When folded branches are merged,
`Folded` may be dissolved.

In the coverage map, either `Counter` is `Zero`. Currently both were
`Zero`.

Since "partial fold" has been introduced, either case in `switch` is
omitted as `Folded`.

Each `case:` in `switch` is reported as `[True: n, Folded]`, since
`False` count doesn't show meaningful value.

When `switch` doesn't have `default:`, `switch (Cond)` is reported as
`[Folded, False: n]`, since `True` count was just the sum of `case`(s).
`switch` with `default` can be considered as "the statement that doesn't
have any `False`(s)".
@Lambdaris
Copy link

Lambdaris commented Oct 23, 2024

This patch also helps to distinguish constants from rust, in which constant may have only one counter hard coded to Zero, thank you!

Besides, I found that with this change we should also count false counter id when calculating max CounterID. Otherwise some cases would break with "unexpected" (actually expected, suppose the max counter id is a FalseCounter of a constant false condition) counter id from profile data. I does so at #94137 and it would be also great if some one picked it and opened a smaller pr.

@chapuni
Copy link
Contributor Author

chapuni commented Oct 23, 2024

@Lambdaris I have #112730 as well.

@chapuni chapuni mentioned this pull request Nov 13, 2024
chapuni added a commit that referenced this pull request Dec 18, 2024
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.
chapuni added a commit that referenced this pull request Dec 18, 2024
…112694 (#120418)

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants