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][MC/DC] Show uncoverable and unreachable conditions #94137

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Lambdaris
Copy link

@Lambdaris Lambdaris commented Jun 2, 2024

To close #93560 .

Changes

  1. Clang only sets the counter in folded branch to Zero. And llvm-cov shows a branch as folded as long as either of its counters is Zero.
  2. Add two additional results, uncoverable and unreachable to MCDC conditions. uncoverable means the condition can not effect value of the decision, while unreachable means the condition is always short-circuited. Since both the two kinds of conditions is by no means covered, they could be excluded from MCDC and informed to users. The scheme to identify such conditions is described at this comment.
  3. Add an option --mcdc-exclude to control which kinds of conditions should be counted in coverage. By default constant conditions are excluded.

Copy link

github-actions bot commented Jun 2, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen PGO Profile Guided Optimizations labels Jun 2, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 2, 2024

@llvm/pr-subscribers-llvm-binary-utilities
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-pgo

Author: Lambdaris (Lambdaris)

Changes

To close #93560 .

Changes

  1. Clang only sets the counter in folded branch to Zero. And llvm-cov shows a branch as folded as long as either of its counters is Zero.
  2. Add two additional results, uncoverable and unreachable to MCDC conditions. uncoverable means the condition can not effect value of the decision, while unreachable means the condition is always short-circuited. Since both the two kinds of conditions is by no means covered, they are excluded from MCDC and informed to users. The scheme to identify such conditions is described at this comment.

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

18 Files Affected:

  • (modified) clang/docs/SourceBasedCodeCoverage.rst (+12)
  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+19-8)
  • (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) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+50-21)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+103-13)
  • (modified) llvm/test/tools/llvm-cov/Inputs/mcdc-const-folding.o ()
  • (modified) llvm/test/tools/llvm-cov/Inputs/mcdc-const.o ()
  • (modified) llvm/test/tools/llvm-cov/Inputs/mcdc-macro.o ()
  • (modified) llvm/test/tools/llvm-cov/branch-c-general.test (+6-6)
  • (modified) llvm/test/tools/llvm-cov/mcdc-const.test (+47-47)
  • (modified) llvm/tools/llvm-cov/CoverageReport.cpp (+4-1)
  • (modified) llvm/tools/llvm-cov/CoverageSummaryInfo.cpp (+1-1)
  • (modified) llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp (+6-1)
  • (modified) llvm/tools/llvm-cov/SourceCoverageViewText.cpp (+10-4)
diff --git a/clang/docs/SourceBasedCodeCoverage.rst b/clang/docs/SourceBasedCodeCoverage.rst
index cee706289284d..f875187c80608 100644
--- a/clang/docs/SourceBasedCodeCoverage.rst
+++ b/clang/docs/SourceBasedCodeCoverage.rst
@@ -496,6 +496,18 @@ starts a new boolean expression that is separated from the other conditions by
 the operator ``func()``.  When this is encountered, a warning will be generated
 and the boolean expression will not be instrumented.
 
+Similar as branch coverage, MCDC also is not tracked for constant folded conditions.
+Moreover it's even not tracked for conditions that can not effect outcomes of decisions
+due to other constants. These conditions will be shown as ``uncoverable`` or 
+``unreachable``, determined by whether they can be visited. For instance, in 
+``a || true || b``, value of the decision is always ``true`` by reason of the constant
+condition. Thus ``a`` can not lead the decision to ``false`` even though it could branch, 
+while ``b`` is always short-circuited. Hence ``a`` is shown as ``uncoverable``
+and ``b`` is marked as ``unreachable``. Statistics of MCDC does not count constant 
+conditions which do not vary or such conditions which make no difference on value of 
+decisions. If a decision is proved to no branch theoretically, it shows mark ``Folded``
+rather than coverage percent.
+
 Switch statements
 -----------------
 
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 6ce2d32dd292e..37e6543795558 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1084,9 +1084,13 @@ struct CounterCoverageMappingBuilder
   }
 
   /// Determine whether the given condition can be constant folded.
-  bool ConditionFoldsToBool(const Expr *Cond) {
+  bool ConditionFoldsToBool(const Expr *Cond, bool &ResultBool) {
     Expr::EvalResult Result;
-    return (Cond->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext()));
+    if (Cond->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext())) {
+      ResultBool = Result.Val.getInt().getBoolValue();
+      return true;
+    }
+    return false;
   }
 
   /// Create a Branch Region around an instrumentable condition for coverage
@@ -1113,15 +1117,22 @@ struct CounterCoverageMappingBuilder
         BranchParams = mcdc::BranchParameters{ID, Conds};
 
       // If a condition can fold to true or false, the corresponding branch
-      // will be removed.  Create a region with both counters hard-coded to
-      // zero. This allows us to visualize them in a special way.
+      // will be removed. Create a region with the relative counter hard-coded
+      // to zero. This allows us to visualize them in a special way.
       // 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
+      bool ConstantBool = false;
+      if (ConditionFoldsToBool(C, ConstantBool)) {
+        if (ConstantBool) {
+          popRegions(pushRegion(TrueCnt, getStart(C), getEnd(C),
+                                Counter::getZero(), BranchParams));
+        } else {
+          popRegions(pushRegion(Counter::getZero(), getStart(C), getEnd(C),
+                                FalseCnt, BranchParams));
+        }
+
+      } else
         // Otherwise, create a region with the True counter and False counter.
         popRegions(pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt,
                               BranchParams));
diff --git a/clang/test/CoverageMapping/branch-constfolded.cpp b/clang/test/CoverageMapping/branch-constfolded.cpp
index c8755d5d752b6..1e1639ba796df 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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:0, 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 445cdfc20e2af..b6fd525e930f9 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 ad71fb15eda42..4cd2c93437193 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 2b5b12d9dcad6..1b22bb7e421b6 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:0, 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:0, 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 4dfbb17c2bba8..7a867be5c9e6c 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:0, 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/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index da03104045249..157ac83d69ee6 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -384,6 +384,13 @@ struct MCDCRecord {
   /// are effectively ignored.
   enum CondState { MCDC_DontCare = -1, MCDC_False = 0, MCDC_True = 1 };
 
+  enum CondResult {
+    MCDC_Normal,
+    MCDC_Constant,
+    MCDC_Uncoverable,
+    MCDC_Unreachable
+  };
+
   /// Emulate SmallVector<CondState> with a pair of BitVector.
   ///
   ///          True  False DontCare (Impossible)
@@ -442,22 +449,23 @@ struct MCDCRecord {
   using TVPairMap = llvm::DenseMap<unsigned, TVRowPair>;
   using CondIDMap = llvm::DenseMap<unsigned, unsigned>;
   using LineColPairMap = llvm::DenseMap<unsigned, LineColPair>;
+  using ResultVector = llvm::SmallVector<CondResult>;
 
 private:
   CounterMappingRegion Region;
   TestVectors TV;
   TVPairMap IndependencePairs;
-  BoolVector Folded;
+  ResultVector CondResults;
   CondIDMap PosToID;
   LineColPairMap CondLoc;
 
 public:
   MCDCRecord(const CounterMappingRegion &Region, TestVectors &&TV,
-             TVPairMap &&IndependencePairs, BoolVector &&Folded,
+             TVPairMap &&IndependencePairs, ResultVector &&CondResults,
              CondIDMap &&PosToID, LineColPairMap &&CondLoc)
       : Region(Region), TV(std::move(TV)),
         IndependencePairs(std::move(IndependencePairs)),
-        Folded(std::move(Folded)), PosToID(std::move(PosToID)),
+        CondResults(std::move(CondResults)), PosToID(std::move(PosToID)),
         CondLoc(std::move(CondLoc)){};
 
   CounterMappingRegion getDecisionRegion() const { return Region; }
@@ -465,7 +473,12 @@ struct MCDCRecord {
     return Region.getDecisionParams().NumConditions;
   }
   unsigned getNumTestVectors() const { return TV.size(); }
-  bool isCondFolded(unsigned Condition) const { return Folded[Condition]; }
+  bool isCondCoverable(unsigned Condition) const {
+    return getCondResult(Condition) == CondResult::MCDC_Normal;
+  }
+  CondResult getCondResult(unsign...
[truncated]

@Lambdaris
Copy link
Author

Lambdaris commented Jul 13, 2024

Add an option "--mcdc-exclude" to control which conditions can be excluded from coverage. By default it's "constant".

@evodius96
Copy link
Contributor

I think this is a useful option, thanks. So the default preserves existing behavior? I think that's perfect and amenable to change in the future of needed.

@Lambdaris
Copy link
Author

Lambdaris commented Jul 16, 2024

Renam the test to mcdc-exclude and add a test to check --mcdc-exclude=none

So the default preserves existing behavior?

Yeah, existing behavior is equivalent to --mcdc-exclude=constant as default.

@Lambdaris
Copy link
Author

Lambdaris commented Aug 21, 2024

I have no authority to request reviewers or commit this patch. So let's @ someone, @chapuni @ornata @hanickadot

@evodius96
Copy link
Contributor

I have no authority to request reviewers or commit this patch. So let's @ someone, @chapuni @ornata @hanickadot

I had some comments in the pending state that I just now noticed :( sorry about that.

@Lambdaris
Copy link
Author

Lambdaris commented Aug 23, 2024

I have no authority to request reviewers or commit this patch. So let's @ someone, @chapuni @ornata @hanickadot

I had some comments in the pending state that I just now noticed :( sorry about that.

😨 Terribly sorry I was not aware that pending views are invisible to others. I should have written normal comments.

@chapuni
Copy link
Contributor

chapuni commented Feb 12, 2025

What's the current status? I think marking extended semantics (to Regions) will still be the issue. Could we enhance "Gap"?

@Lambdaris
Copy link
Author

What's the current status?

I think it's done and ready for review.

I think marking extended semantics (to Regions) will still be the issue. Could we enhance "Gap"?

Could you clarify the issue and how we enhance "gap" a bit more? Now the major work of this patch is in MCDCRecordProcessor::processMCDCRecord, marking some mcdc branch regions as we expected.

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 llvm:binary-utilities PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Coverage][MC/DC] Constant folding might make conditions never covered
4 participants