Refactor processing of BranchRegions associated with an MCDCDecisionRegion

llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

@@ -582,6 +582,27 @@ static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
   return MaxBitmapID + (SizeInBits / CHAR_BIT);
 }
 
+static void
+addMCDCBranches(unsigned FileID, const unsigned NumConds,
+                std::vector<const CounterMappingRegion *> &MCDCBranches,
+                const ArrayRef<CounterMappingRegion>::iterator &Begin,
+                const ArrayRef<CounterMappingRegion>::iterator &End) {
+  // Use the given iterator to scan to the end of the list of regions.
+  for (auto It = Begin; It != End; ++It)
+    if (It->FileID == FileID && MCDCBranches.size() < NumConds) {
+      if (It->Kind == CounterMappingRegion::MCDCBranchRegion)
+        // Gather BranchRegions associated within the given FileID until the
+        // NumConds limit is reached.
+        MCDCBranches.push_back(&*It);
+      else if (It->Kind == CounterMappingRegion::ExpansionRegion) {
+        // If an ExpansionRegion is encountered, recur to check that any
+        // BranchRegions associated with the ExpansionRegion are included.
+        assert(It->ExpandedFileID > It->FileID);
+        addMCDCBranches(It->ExpandedFileID, NumConds, MCDCBranches, It, End);
+      }
+    }
+}
+
 Error CoverageMapping::loadFunctionRecord(
     const CoverageMappingRecord &Record,
     IndexedInstrProfReader &ProfileReader) {
@@ -638,20 +659,56 @@ Error CoverageMapping::loadFunctionRecord(
       Record.MappingRegions[0].Count.isZero() && Counts[0] > 0)
     return Error::success();
 
-  unsigned NumConds = 0;
-  const CounterMappingRegion *MCDCDecision;
-  std::vector<const CounterMappingRegion *> MCDCBranches;
-
   FunctionRecord Function(OrigFuncName, Record.Filenames);
-  for (const auto &Region : Record.MappingRegions) {
+
+  const auto &RegionsBegin = Record.MappingRegions.begin();
+  const auto &RegionsEnd = Record.MappingRegions.end();
+  for (auto It = RegionsBegin; It != RegionsEnd; ++It) {
+    const auto &Region = *It;
+
     // If an MCDCDecisionRegion is seen, track the BranchRegions that follow
     // it according to Region.NumConditions.
     if (Region.Kind == CounterMappingRegion::MCDCDecisionRegion) {
-      assert(NumConds == 0);
-      MCDCDecision = &Region;
-      NumConds = Region.MCDCParams.NumConditions;
+      std::vector<const CounterMappingRegion *> MCDCBranches;
+      const unsigned NumConds = Region.MCDCParams.NumConditions;
+
+      // If a MCDCDecisionRegion was seen, use the current iterator to scan
+      // ahead to store the BranchRegions that correspond to it in a vector,
+      // according to the number of conditions recorded for the region (tracked
+      // by NumConds). Note that BranchRegions may be part of ExpansionRegions,
+      // which need to be followed recursively.
+      addMCDCBranches(It->FileID, NumConds, MCDCBranches, It, RegionsEnd);
+
+      // All of the corresponding BranchRegions ought to be accounted for.
+      assert(MCDCBranches.size() == NumConds);
+
+      // Evaluating the test vector bitmap for the decision region entails
+      // calculating precisely what bits are pertinent to this region alone.
+      // This is calculated based on the recorded offset into the global
+      // profile bitmap; the length is calculated based on the recorded
+      // number of conditions.
+      Expected<BitVector> ExecutedTestVectorBitmap =
+          Ctx.evaluateBitmap(&Region);
+      if (auto E = ExecutedTestVectorBitmap.takeError()) {
+        consumeError(std::move(E));
+        return Error::success();
+      }
+
+      // Since the bitmap identifies the executed test vectors for an MC/DC
+      // DecisionRegion, all of the information is now available to process.
+      // This is where the bulk of the MC/DC progressing takes place.
+      Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
+          Region, *ExecutedTestVectorBitmap, MCDCBranches);
+      if (auto E = Record.takeError()) {
+        consumeError(std::move(E));
+        return Error::success();
+      }
+
+      // Save the MC/DC Record so that it can be visualized later.
+      Function.pushMCDCRecord(*Record);
       continue;
     }
+
     Expected<int64_t> ExecutionCount = Ctx.evaluate(Region.Count);
     if (auto E = ExecutionCount.takeError()) {
       consumeError(std::move(E));
@@ -663,44 +720,6 @@ Error CoverageMapping::loadFunctionRecord(
       return Error::success();
     }
     Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount);
-
-    // If a MCDCDecisionRegion was seen, store the BranchRegions that
-    // correspond to it in a vector, according to the number of conditions
-    // recorded for the region (tracked by NumConds).
-    if (NumConds > 0 && Region.Kind == CounterMappingRegion::MCDCBranchRegion) {
-      MCDCBranches.push_back(&Region);
-
-      // As we move through all of the MCDCBranchRegions that follow the
-      // MCDCDecisionRegion, decrement NumConds to make sure we account for
-      // them all before we calculate the bitmap of executed test vectors.
-      if (--NumConds == 0) {
-        // Evaluating the test vector bitmap for the decision region entails
-        // calculating precisely what bits are pertinent to this region alone.
-        // This is calculated based on the recorded offset into the global
-        // profile bitmap; the length is calculated based on the recorded
-        // number of conditions.
-        Expected<BitVector> ExecutedTestVectorBitmap =
-            Ctx.evaluateBitmap(MCDCDecision);
-        if (auto E = ExecutedTestVectorBitmap.takeError()) {
-          consumeError(std::move(E));
-          return Error::success();
-        }
-
-        // Since the bitmap identifies the executed test vectors for an MC/DC
-        // DecisionRegion, all of the information is now available to process.
-        // This is where the bulk of the MC/DC progressing takes place.
-        Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
-            *MCDCDecision, *ExecutedTestVectorBitmap, MCDCBranches);
-        if (auto E = Record.takeError()) {
-          consumeError(std::move(E));
-          return Error::success();
-        }
-
-        // Save the MC/DC Record so that it can be visualized later.
-        Function.pushMCDCRecord(*Record);
-        MCDCBranches.clear();
-      }
-    }
   }
 
   // Don't create records for (filenames, function) pairs we've already seen.

llvm/test/tools/llvm-cov/Inputs/mcdc-macro.c

@@ -0,0 +1,20 @@
+#define C c
+#define D 1
+#define E (C != a) && (C > a)
+#define F E
+
+void __attribute__((noinline)) func1(void) { return; }
+
+void __attribute__((noinline)) func(int a, int b, int c) {
+  if (a && D && E || b)
+    func1();
+  if (b && D)
+    func1();
+  if (a && (b && C) || (D && F))
+    func1();
+}
+
+int main() {
+  func(2, 3, 3);
+  return 0;
+}

llvm/test/tools/llvm-cov/Inputs/mcdc-macro.proftext

@@ -0,0 +1,62 @@
+main
+# Func Hash:
+24
+# Num Counters:
+1
+# Counter Values:
+1
+
+foo
+# Func Hash:
+395201011017399473
+# Num Counters:
+22
+# Counter Values:
+1
+1
+0
+0
+1
+1
+1
+1
+1
+1
+1
+1
+1
+1
+0
+1
+1
+1
+0
+0
+0
+0
+# Num Bitmap Bytes:
+$13
+# Bitmap Byte Values:
+0x0
+0x0
+0x0
+0x20
+0x8
+0x0
+0x20
+0x0
+0x0
+0x0
+0x0
+0x0
+0x0
+
+
+bar
+# Func Hash:
+24
+# Num Counters:
+1
+# Counter Values:
+3
+

llvm/test/tools/llvm-cov/mcdc-macro.test

@@ -0,0 +1,92 @@
+// Test visualization of MC/DC constructs for branches in macro expansions.
+
+// RUN: llvm-profdata merge %S/Inputs/mcdc-macro.proftext -o %t.profdata
+// RUN: llvm-cov show --show-expansions --show-branches=count --show-mcdc %S/Inputs/mcdc-macro.o -instr-profile %t.profdata -path-equivalence=/tmp,%S/Inputs %S/Inputs/mcdc-macro.c | FileCheck %s
+
+// CHECK:  |  |  |  Branch (2:11): [Folded - Ignored]
+// CHECK:  |  |  |  Branch (3:11): [True: 0, False: 0]
+// CHECK:  |  |  |  Branch (3:23): [True: 0, False: 0]
+// CHECK:  |  Branch (9:7): [True: 0, False: 0]
+// CHECK-NEXT:  |  Branch (9:22): [True: 0, False: 0]
+// CHECK-NEXT:  ------------------
+// CHECK-NEXT:  |---> MC/DC Decision Region (9:7) to (9:23)
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |  Number of Conditions: 5
+// CHECK-NEXT:  |     Condition C1 --> (9:7)
+// CHECK-NEXT:  |     Condition C2 --> (2:11)
+// CHECK-NEXT:  |     Condition C3 --> (3:11)
+// CHECK-NEXT:  |     Condition C4 --> (3:23)
+// CHECK-NEXT:  |     Condition C5 --> (9:22)
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |  Executed MC/DC Test Vectors:
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |     None.
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |  C1-Pair: not covered
+// CHECK-NEXT:  |  C2-Pair: constant folded
+// CHECK-NEXT:  |  C3-Pair: not covered
+// CHECK-NEXT:  |  C4-Pair: not covered
+// CHECK-NEXT:  |  C5-Pair: not covered
+// CHECK-NEXT:  |  MC/DC Coverage for Decision: 0.00%
+// CHECK-NEXT:  |
+// CHECK-NEXT:  ------------------
+
+// CHECK:  |  |  |  Branch (2:11): [Folded - Ignored]
+// CHECK:  |  Branch (11:7): [True: 0, False: 0]
+// CHECK-NEXT:  ------------------
+// CHECK-NEXT:  |---> MC/DC Decision Region (11:7) to (11:13)
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |  Number of Conditions: 2
+// CHECK-NEXT:  |     Condition C1 --> (11:7)
+// CHECK-NEXT:  |     Condition C2 --> (2:11)
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |  Executed MC/DC Test Vectors:
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |     None.
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |  C1-Pair: not covered
+// CHECK-NEXT:  |  C2-Pair: constant folded
+// CHECK-NEXT:  |  MC/DC Coverage for Decision: 0.00%
+// CHECK-NEXT:  |
+// CHECK-NEXT:  ------------------
+
+// CHECK:  |  |  |  Branch (1:11): [True: 0, False: 0]
+// CHECK:  |  |  |  Branch (2:11): [Folded - Ignored]
+// CHECK:  |  |  |  |  |  Branch (3:11): [True: 0, False: 0]
+// CHECK:  |  |  |  |  |  Branch (3:23): [True: 0, False: 0]
+// CHECK:  |  Branch (13:7): [True: 0, False: 0]
+// CHECK-NEXT:  |  Branch (13:13): [True: 0, False: 0]
+// CHECK-NEXT:  ------------------
+// CHECK-NEXT:  |---> MC/DC Decision Region (13:7) to (13:32)
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |  Number of Conditions: 6
+// CHECK-NEXT:  |     Condition C1 --> (13:7)
+// CHECK-NEXT:  |     Condition C2 --> (13:13)
+// CHECK-NEXT:  |     Condition C3 --> (1:11)
+// CHECK-NEXT:  |     Condition C4 --> (2:11)
+// CHECK-NEXT:  |     Condition C5 --> (3:11)
+// CHECK-NEXT:  |     Condition C6 --> (3:23)
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |  Executed MC/DC Test Vectors:
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |     None.
+// CHECK-NEXT:  |
+// CHECK-NEXT:  |  C1-Pair: not covered
+// CHECK-NEXT:  |  C2-Pair: not covered
+// CHECK-NEXT:  |  C3-Pair: not covered
+// CHECK-NEXT:  |  C4-Pair: constant folded
+// CHECK-NEXT:  |  C5-Pair: not covered
+// CHECK-NEXT:  |  C6-Pair: not covered
+// CHECK-NEXT:  |  MC/DC Coverage for Decision: 0.00%
+// CHECK-NEXT:  |
+// CHECK-NEXT:  ------------------
+
+Instructions for regenerating the test:
+
+# cd %S/Inputs
+cp mcdc-macro.c /tmp
+
+clang -fcoverage-mcdc -fprofile-instr-generate -fcoverage-compilation-dir=. \
+    -fcoverage-mapping /tmp/mcdc-macro.c -o /tmp/mcdc-macro.o
+
+mv /tmp/mcdc-macro.o %S/Inputs