[llvm-branch-commits] [llvm] [Coverage] Sort `MCDCRecord::ExecVectors` order by Bitmap index (PR #121195)

NAKAMURA Takumi via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Jan 6 16:27:14 PST 2025


https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/121195

>From 36b4aaf0248c85c3e3cf621f75b26e2734efafba Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Fri, 27 Dec 2024 15:31:05 +0900
Subject: [PATCH 1/4] [Coverage] Make `MCDCRecord::Folded` as `[false/true]`
 with BitVector. NFC.

---
 .../llvm/ProfileData/Coverage/CoverageMapping.h       |  6 ++++--
 llvm/lib/ProfileData/Coverage/CoverageMapping.cpp     | 11 ++++++-----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 42da188fef34ee..5caba07f8ad43a 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -442,7 +442,7 @@ struct MCDCRecord {
   };
 
   using TestVectors = llvm::SmallVector<std::pair<TestVector, CondState>>;
-  using BoolVector = llvm::SmallVector<bool>;
+  using BoolVector = std::array<BitVector, 2>;
   using TVRowPair = std::pair<unsigned, unsigned>;
   using TVPairMap = llvm::DenseMap<unsigned, TVRowPair>;
   using CondIDMap = llvm::DenseMap<unsigned, unsigned>;
@@ -470,7 +470,9 @@ struct MCDCRecord {
     return Region.getDecisionParams().NumConditions;
   }
   unsigned getNumTestVectors() const { return TV.size(); }
-  bool isCondFolded(unsigned Condition) const { return Folded[Condition]; }
+  bool isCondFolded(unsigned Condition) const {
+    return Folded[false][Condition] || Folded[true][Condition];
+  }
 
   /// Return the evaluation of a condition (indicated by Condition) in an
   /// executed test vector (indicated by TestVectorIndex), which will be True,
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 87d8bb1bbb79c7..2eba1667306d15 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -392,8 +392,9 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
       : NextIDsBuilder(Branches), TVIdxBuilder(this->NextIDs), Bitmap(Bitmap),
         Region(Region), DecisionParams(Region.getDecisionParams()),
         Branches(Branches), NumConditions(DecisionParams.NumConditions),
-        Folded(NumConditions, false), IndependencePairs(NumConditions),
-        ExecVectors(ExecVectorsByCond[false]), IsVersion11(IsVersion11) {}
+        Folded{{BitVector(NumConditions), BitVector(NumConditions)}},
+        IndependencePairs(NumConditions), ExecVectors(ExecVectorsByCond[false]),
+        IsVersion11(IsVersion11) {}
 
 private:
   // Walk the binary decision diagram and try assigning both false and true to
@@ -485,7 +486,6 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
   /// location is also tracked, as well as whether it is constant folded (in
   /// which case it is excuded from the metric).
   MCDCRecord processMCDCRecord() {
-    unsigned I = 0;
     MCDCRecord::CondIDMap PosToID;
     MCDCRecord::LineColPairMap CondLoc;
 
@@ -499,11 +499,12 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
     //   visualize where the condition is.
     // - Record whether the condition is constant folded so that we exclude it
     //   from being measured.
-    for (const auto *B : Branches) {
+    for (auto [I, B] : enumerate(Branches)) {
       const auto &BranchParams = B->getBranchParams();
       PosToID[I] = BranchParams.ID;
       CondLoc[I] = B->startLoc();
-      Folded[I++] = (B->Count.isZero() || B->FalseCount.isZero());
+      Folded[false][I] = B->FalseCount.isZero();
+      Folded[true][I] = B->Count.isZero();
     }
 
     // Using Profile Bitmap from runtime, mark the executed test vectors.

>From 978070d6aca594a49dc2ef0558e1e28e883ac1b0 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Fri, 27 Dec 2024 15:34:16 +0900
Subject: [PATCH 2/4] [Coverage] MCDC: Move `findIndependencePairs` into
 `MCDCRecord`

---
 .../ProfileData/Coverage/CoverageMapping.h    | 27 +++++---
 .../ProfileData/Coverage/CoverageMapping.cpp  | 67 ++++++++++---------
 2 files changed, 51 insertions(+), 43 deletions(-)

diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 42da188fef34ee..4f28f5da6cf81f 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -35,6 +35,7 @@
 #include <cstdint>
 #include <iterator>
 #include <memory>
+#include <optional>
 #include <sstream>
 #include <string>
 #include <system_error>
@@ -451,19 +452,22 @@ struct MCDCRecord {
 private:
   CounterMappingRegion Region;
   TestVectors TV;
-  TVPairMap IndependencePairs;
+  std::optional<TVPairMap> IndependencePairs;
   BoolVector Folded;
   CondIDMap PosToID;
   LineColPairMap CondLoc;
 
 public:
   MCDCRecord(const CounterMappingRegion &Region, TestVectors &&TV,
-             TVPairMap &&IndependencePairs, BoolVector &&Folded,
-             CondIDMap &&PosToID, LineColPairMap &&CondLoc)
-      : Region(Region), TV(std::move(TV)),
-        IndependencePairs(std::move(IndependencePairs)),
-        Folded(std::move(Folded)), PosToID(std::move(PosToID)),
-        CondLoc(std::move(CondLoc)){};
+             BoolVector &&Folded, CondIDMap &&PosToID, LineColPairMap &&CondLoc)
+      : Region(Region), TV(std::move(TV)), Folded(std::move(Folded)),
+        PosToID(std::move(PosToID)), CondLoc(std::move(CondLoc)) {
+    findIndependencePairs();
+  }
+
+  // Compare executed test vectors against each other to find an independence
+  // pairs for each condition.  This processing takes the most time.
+  void findIndependencePairs();
 
   const CounterMappingRegion &getDecisionRegion() const { return Region; }
   unsigned getNumConditions() const {
@@ -494,10 +498,10 @@ struct MCDCRecord {
   /// TestVectors requires a translation from a ordinal position to actual
   /// condition ID. This is done via PosToID[].
   bool isConditionIndependencePairCovered(unsigned Condition) const {
+    assert(IndependencePairs);
     auto It = PosToID.find(Condition);
-    if (It != PosToID.end())
-      return IndependencePairs.contains(It->second);
-    llvm_unreachable("Condition ID without an Ordinal mapping");
+    assert(It != PosToID.end() && "Condition ID without an Ordinal mapping");
+    return IndependencePairs->contains(It->second);
   }
 
   /// Return the Independence Pair that covers the given condition. Because
@@ -507,7 +511,8 @@ struct MCDCRecord {
   /// via PosToID[].
   TVRowPair getConditionIndependencePair(unsigned Condition) {
     assert(isConditionIndependencePairCovered(Condition));
-    return IndependencePairs[PosToID[Condition]];
+    assert(IndependencePairs);
+    return (*IndependencePairs)[PosToID[Condition]];
   }
 
   float getPercentCovered() const {
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 87d8bb1bbb79c7..2c7a77bac09328 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -221,6 +221,40 @@ Expected<int64_t> CounterMappingContext::evaluate(const Counter &C) const {
   return LastPoppedValue;
 }
 
+// Find an independence pair for each condition:
+// - The condition is true in one test and false in the other.
+// - The decision outcome is true one test and false in the other.
+// - All other conditions' values must be equal or marked as "don't care".
+void MCDCRecord::findIndependencePairs() {
+  if (IndependencePairs)
+    return;
+
+  IndependencePairs.emplace();
+
+  unsigned NumTVs = TV.size();
+  // Will be replaced to shorter expr.
+  unsigned TVTrueIdx = std::distance(
+      TV.begin(),
+      std::find_if(TV.begin(), TV.end(),
+                   [&](auto I) { return (I.second == MCDCRecord::MCDC_True); })
+
+  );
+  for (unsigned I = TVTrueIdx; I < NumTVs; ++I) {
+    const auto &[A, ACond] = TV[I];
+    assert(ACond == MCDCRecord::MCDC_True);
+    for (unsigned J = 0; J < TVTrueIdx; ++J) {
+      const auto &[B, BCond] = TV[J];
+      assert(BCond == MCDCRecord::MCDC_False);
+      // If the two vectors differ in exactly one condition, ignoring DontCare
+      // conditions, we have found an independence pair.
+      auto AB = A.getDifferences(B);
+      if (AB.count() == 1)
+        IndependencePairs->insert(
+            {AB.find_first(), std::make_pair(J + 1, I + 1)});
+    }
+  }
+}
+
 mcdc::TVIdxBuilder::TVIdxBuilder(const SmallVectorImpl<ConditionIDs> &NextIDs,
                                  int Offset)
     : Indices(NextIDs.size()) {
@@ -375,9 +409,6 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
   /// ExecutedTestVectorBitmap.
   MCDCRecord::TestVectors &ExecVectors;
 
-  /// Number of False items in ExecVectors
-  unsigned NumExecVectorsF;
-
 #ifndef NDEBUG
   DenseSet<unsigned> TVIdxs;
 #endif
@@ -446,34 +477,11 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
     // Fill ExecVectors order by False items and True items.
     // ExecVectors is the alias of ExecVectorsByCond[false], so
     // Append ExecVectorsByCond[true] on it.
-    NumExecVectorsF = ExecVectors.size();
     auto &ExecVectorsT = ExecVectorsByCond[true];
     ExecVectors.append(std::make_move_iterator(ExecVectorsT.begin()),
                        std::make_move_iterator(ExecVectorsT.end()));
   }
 
-  // Find an independence pair for each condition:
-  // - The condition is true in one test and false in the other.
-  // - The decision outcome is true one test and false in the other.
-  // - All other conditions' values must be equal or marked as "don't care".
-  void findIndependencePairs() {
-    unsigned NumTVs = ExecVectors.size();
-    for (unsigned I = NumExecVectorsF; I < NumTVs; ++I) {
-      const auto &[A, ACond] = ExecVectors[I];
-      assert(ACond == MCDCRecord::MCDC_True);
-      for (unsigned J = 0; J < NumExecVectorsF; ++J) {
-        const auto &[B, BCond] = ExecVectors[J];
-        assert(BCond == MCDCRecord::MCDC_False);
-        // If the two vectors differ in exactly one condition, ignoring DontCare
-        // conditions, we have found an independence pair.
-        auto AB = A.getDifferences(B);
-        if (AB.count() == 1)
-          IndependencePairs.insert(
-              {AB.find_first(), std::make_pair(J + 1, I + 1)});
-      }
-    }
-  }
-
 public:
   /// Process the MC/DC Record in order to produce a result for a boolean
   /// expression. This process includes tracking the conditions that comprise
@@ -509,13 +517,8 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
     // Using Profile Bitmap from runtime, mark the executed test vectors.
     findExecutedTestVectors();
 
-    // Compare executed test vectors against each other to find an independence
-    // pairs for each condition.  This processing takes the most time.
-    findIndependencePairs();
-
     // Record Test vectors, executed vectors, and independence pairs.
-    return MCDCRecord(Region, std::move(ExecVectors),
-                      std::move(IndependencePairs), std::move(Folded),
+    return MCDCRecord(Region, std::move(ExecVectors), std::move(Folded),
                       std::move(PosToID), std::move(CondLoc));
   }
 };

>From 3abe2ac361822bf6513e5f3ef9fead37842af862 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Fri, 27 Dec 2024 15:40:47 +0900
Subject: [PATCH 3/4] [Coverage] Sort `MCDCRecord::ExecVectors` order by Bitmap
 index

---
 .../ProfileData/Coverage/CoverageMapping.cpp  | 40 +++++++++++++------
 llvm/test/tools/llvm-cov/mcdc-const.test      | 32 +++++++--------
 llvm/test/tools/llvm-cov/mcdc-general.test    |  8 ++--
 3 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 3bbee70be0fe93..53445b1030c9fd 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -401,13 +401,27 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
   /// Mapping of calculated MC/DC Independence Pairs for each condition.
   MCDCRecord::TVPairMap IndependencePairs;
 
-  /// Storage for ExecVectors
-  /// ExecVectors is the alias of its 0th element.
-  std::array<MCDCRecord::TestVectors, 2> ExecVectorsByCond;
+  /// Helper for sorting ExecVectors.
+  struct TVIdxTuple {
+    MCDCRecord::CondState MCDCCond; /// True/False
+    unsigned BIdx;                  /// Bitmap Index
+    unsigned Ord;                   /// Last position on ExecVectors
+
+    TVIdxTuple(MCDCRecord::CondState MCDCCond, unsigned BIdx, unsigned Ord)
+        : MCDCCond(MCDCCond), BIdx(BIdx), Ord(Ord) {}
+
+    bool operator<(const TVIdxTuple &RHS) const {
+      return (std::tie(this->MCDCCond, this->BIdx, this->Ord) <
+              std::tie(RHS.MCDCCond, RHS.BIdx, RHS.Ord));
+    }
+  };
+
+  // Indices for sorted TestVectors;
+  std::vector<TVIdxTuple> ExecVectorIdxs;
 
   /// Actual executed Test Vectors for the boolean expression, based on
   /// ExecutedTestVectorBitmap.
-  MCDCRecord::TestVectors &ExecVectors;
+  MCDCRecord::TestVectors ExecVectors;
 
 #ifndef NDEBUG
   DenseSet<unsigned> TVIdxs;
@@ -424,8 +438,7 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
         Region(Region), DecisionParams(Region.getDecisionParams()),
         Branches(Branches), NumConditions(DecisionParams.NumConditions),
         Folded{{BitVector(NumConditions), BitVector(NumConditions)}},
-        IndependencePairs(NumConditions), ExecVectors(ExecVectorsByCond[false]),
-        IsVersion11(IsVersion11) {}
+        IndependencePairs(NumConditions), IsVersion11(IsVersion11) {}
 
 private:
   // Walk the binary decision diagram and try assigning both false and true to
@@ -453,10 +466,12 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
                       : DecisionParams.BitmapIdx - NumTestVectors + NextTVIdx])
         continue;
 
+      ExecVectorIdxs.emplace_back(MCDCCond, NextTVIdx, ExecVectors.size());
+
       // Copy the completed test vector to the vector of testvectors.
       // The final value (T,F) is equal to the last non-dontcare state on the
       // path (in a short-circuiting system).
-      ExecVectorsByCond[MCDCCond].push_back({TV, MCDCCond});
+      ExecVectors.push_back({TV, MCDCCond});
     }
 
     // Reset back to DontCare.
@@ -475,12 +490,11 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
     assert(TVIdxs.size() == unsigned(NumTestVectors) &&
            "TVIdxs wasn't fulfilled");
 
-    // Fill ExecVectors order by False items and True items.
-    // ExecVectors is the alias of ExecVectorsByCond[false], so
-    // Append ExecVectorsByCond[true] on it.
-    auto &ExecVectorsT = ExecVectorsByCond[true];
-    ExecVectors.append(std::make_move_iterator(ExecVectorsT.begin()),
-                       std::make_move_iterator(ExecVectorsT.end()));
+    llvm::sort(ExecVectorIdxs);
+    MCDCRecord::TestVectors NewTestVectors;
+    for (const auto &IdxTuple : ExecVectorIdxs)
+      NewTestVectors.push_back(std::move(ExecVectors[IdxTuple.Ord]));
+    ExecVectors = std::move(NewTestVectors);
   }
 
 public:
diff --git a/llvm/test/tools/llvm-cov/mcdc-const.test b/llvm/test/tools/llvm-cov/mcdc-const.test
index 5424625cf6a6b5..76eb7cf706d737 100644
--- a/llvm/test/tools/llvm-cov/mcdc-const.test
+++ b/llvm/test/tools/llvm-cov/mcdc-const.test
@@ -61,8 +61,8 @@
 //      CHECKFULLCASE: |  C1-Pair: constant folded
 // CHECKFULLCASE-NEXT: |  C2-Pair: not covered
 //      CHECKFULLCASE: |  MC/DC Coverage for Decision: 0.00%
-//      CHECKFULLCASE: |  1 { F,  C  = T      }
-// CHECKFULLCASE-NEXT: |  2 { T,  C  = T      }
+//      CHECKFULLCASE: |  1 { T,  C  = T      }
+// CHECKFULLCASE-NEXT: |  2 { F,  C  = T      }
 //      CHECKFULLCASE: |  C1-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C2-Pair: constant folded
 //      CHECKFULLCASE: |  MC/DC Coverage for Decision: 0.00%
@@ -106,20 +106,20 @@
 // CHECKFULLCASE-NEXT: |  C2-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C3-Pair: not covered
 //      CHECKFULLCASE: |  MC/DC Coverage for Decision: 0.00%
-//      CHECKFULLCASE: |  1 { F,  C,  -  = T      }
-// CHECKFULLCASE-NEXT: |  2 { T,  C,  -  = T      }
+//      CHECKFULLCASE: |  1 { T,  C,  -  = T      }
+// CHECKFULLCASE-NEXT: |  2 { F,  C,  -  = T      }
 //      CHECKFULLCASE: |  C1-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C2-Pair: constant folded
 // CHECKFULLCASE-NEXT: |  C3-Pair: not covered
 //      CHECKFULLCASE: |  MC/DC Coverage for Decision: 0.00%
-//      CHECKFULLCASE: |  1 { C,  F,  T  = T      }
-// CHECKFULLCASE-NEXT: |  2 { C,  T,  -  = T      }
+//      CHECKFULLCASE: |  1 { C,  T,  -  = T      }
+// CHECKFULLCASE-NEXT: |  2 { C,  F,  T  = T      }
 //      CHECKFULLCASE: |  C1-Pair: constant folded
 // CHECKFULLCASE-NEXT: |  C2-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C3-Pair: not covered
 //      CHECKFULLCASE: |  MC/DC Coverage for Decision: 0.00%
-//      CHECKFULLCASE: |  1 { F,  C,  T  = T      }
-// CHECKFULLCASE-NEXT: |  2 { T,  C,  -  = T      }
+//      CHECKFULLCASE: |  1 { T,  C,  -  = T      }
+// CHECKFULLCASE-NEXT: |  2 { F,  C,  T  = T      }
 //      CHECKFULLCASE: |  C1-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C2-Pair: constant folded
 // CHECKFULLCASE-NEXT: |  C3-Pair: not covered
@@ -151,26 +151,26 @@
 // CHECKFULLCASE-NEXT: |  C2-Pair: constant folded
 // CHECKFULLCASE-NEXT: |  C3-Pair: covered: (2,3)
 //      CHECKFULLCASE: |  MC/DC Coverage for Decision: 100.00%
-//      CHECKFULLCASE: |  1 { F,  T,  C  = T      }
-// CHECKFULLCASE-NEXT: |  2 { T,  -,  C  = T      }
+//      CHECKFULLCASE: |  1 { T,  -,  C  = T      }
+// CHECKFULLCASE-NEXT: |  2 { F,  T,  C  = T      }
 //      CHECKFULLCASE: |  C1-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C2-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C3-Pair: constant folded
 //      CHECKFULLCASE: |  MC/DC Coverage for Decision: 0.00%
-//      CHECKFULLCASE: |  1 { F,  C,  -  = T      }
-// CHECKFULLCASE-NEXT: |  2 { T,  C,  -  = T      }
+//      CHECKFULLCASE: |  1 { T,  C,  -  = T      }
+// CHECKFULLCASE-NEXT: |  2 { F,  C,  -  = T      }
 //      CHECKFULLCASE: |  C1-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C2-Pair: constant folded
 // CHECKFULLCASE-NEXT: |  C3-Pair: not covered
 //      CHECKFULLCASE: |  MC/DC Coverage for Decision: 0.00%
-//      CHECKFULLCASE: |  1 { F,  T,  C  = T      }
-// CHECKFULLCASE-NEXT: |  2 { T,  -,  C  = T      }
+//      CHECKFULLCASE: |  1 { T,  -,  C  = T      }
+// CHECKFULLCASE-NEXT: |  2 { F,  T,  C  = T      }
 //      CHECKFULLCASE: |  C1-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C2-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C3-Pair: constant folded
 //      CHECKFULLCASE: |  MC/DC Coverage for Decision: 0.00%
-//      CHECKFULLCASE: |  1 { F,  C,  T  = T      }
-// CHECKFULLCASE-NEXT: |  2 { T,  C,  -  = T      }
+//      CHECKFULLCASE: |  1 { T,  C,  -  = T      }
+// CHECKFULLCASE-NEXT: |  2 { F,  C,  T  = T      }
 //      CHECKFULLCASE: |  C1-Pair: not covered
 // CHECKFULLCASE-NEXT: |  C2-Pair: constant folded
 // CHECKFULLCASE-NEXT: |  C3-Pair: not covered
diff --git a/llvm/test/tools/llvm-cov/mcdc-general.test b/llvm/test/tools/llvm-cov/mcdc-general.test
index c1e95cb2bd92ac..1835af9a4c6b5c 100644
--- a/llvm/test/tools/llvm-cov/mcdc-general.test
+++ b/llvm/test/tools/llvm-cov/mcdc-general.test
@@ -19,15 +19,15 @@
 // CHECK-NEXT:  |
 // CHECK-NEXT:  |     C1, C2, C3, C4    Result
 // CHECK-NEXT:  |  1 { F,  -,  F,  -  = F      }
-// CHECK-NEXT:  |  2 { F,  -,  T,  F  = F      }
-// CHECK-NEXT:  |  3 { T,  F,  F,  -  = F      }
+// CHECK-NEXT:  |  2 { T,  F,  F,  -  = F      }
+// CHECK-NEXT:  |  3 { F,  -,  T,  F  = F      }
 // CHECK-NEXT:  |  4 { T,  F,  T,  F  = F      }
 // CHECK-NEXT:  |  5 { T,  F,  T,  T  = T      }
 // CHECK-NEXT:  |  6 { T,  T,  -,  -  = T      }
 // CHECK-NEXT:  |
 // CHECK-NEXT:  |  C1-Pair: covered: (1,6)
-// CHECK-NEXT:  |  C2-Pair: covered: (3,6)
-// CHECK-NEXT:  |  C3-Pair: covered: (3,5)
+// CHECK-NEXT:  |  C2-Pair: covered: (2,6)
+// CHECK-NEXT:  |  C3-Pair: covered: (2,5)
 // CHECK-NEXT:  |  C4-Pair: covered: (4,5)
 // CHECK-NEXT:  |  MC/DC Coverage for Decision: 100.00%
 // CHECK-NEXT:  |

>From f86c537e2c9224cddf10d0b05333e35bdc169361 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Fri, 27 Dec 2024 15:42:07 +0900
Subject: [PATCH 4/4] llvm-cov: Refactor CoverageSummaryInfo. NFC.

---
 llvm/tools/llvm-cov/CoverageSummaryInfo.cpp | 51 +++++++++++----------
 llvm/tools/llvm-cov/CoverageSummaryInfo.h   | 45 +++++++++---------
 2 files changed, 50 insertions(+), 46 deletions(-)

diff --git a/llvm/tools/llvm-cov/CoverageSummaryInfo.cpp b/llvm/tools/llvm-cov/CoverageSummaryInfo.cpp
index ad7561d3dc62c0..5c002a694f66ae 100644
--- a/llvm/tools/llvm-cov/CoverageSummaryInfo.cpp
+++ b/llvm/tools/llvm-cov/CoverageSummaryInfo.cpp
@@ -16,8 +16,9 @@
 using namespace llvm;
 using namespace coverage;
 
-static void sumBranches(size_t &NumBranches, size_t &CoveredBranches,
-                        const ArrayRef<CountedRegion> &Branches) {
+static auto sumBranches(const ArrayRef<CountedRegion> &Branches) {
+  size_t NumBranches = 0;
+  size_t CoveredBranches = 0;
   for (const auto &BR : Branches) {
     if (!BR.TrueFolded) {
       // "True" Condition Branches.
@@ -32,20 +33,22 @@ static void sumBranches(size_t &NumBranches, size_t &CoveredBranches,
         ++CoveredBranches;
     }
   }
+  return BranchCoverageInfo(CoveredBranches, NumBranches);
 }
 
-static void sumBranchExpansions(size_t &NumBranches, size_t &CoveredBranches,
-                                const CoverageMapping &CM,
-                                ArrayRef<ExpansionRecord> Expansions) {
+static BranchCoverageInfo
+sumBranchExpansions(const CoverageMapping &CM,
+                    ArrayRef<ExpansionRecord> Expansions) {
+  BranchCoverageInfo BranchCoverage;
   for (const auto &Expansion : Expansions) {
     auto CE = CM.getCoverageForExpansion(Expansion);
-    sumBranches(NumBranches, CoveredBranches, CE.getBranches());
-    sumBranchExpansions(NumBranches, CoveredBranches, CM, CE.getExpansions());
+    BranchCoverage += sumBranches(CE.getBranches());
+    BranchCoverage += sumBranchExpansions(CM, CE.getExpansions());
   }
+  return BranchCoverage;
 }
 
-static std::pair<size_t, size_t>
-sumMCDCPairs(const ArrayRef<MCDCRecord> &Records) {
+auto sumMCDCPairs(const ArrayRef<MCDCRecord> &Records) {
   size_t NumPairs = 0, CoveredPairs = 0;
   for (const auto &Record : Records) {
     const auto NumConditions = Record.getNumConditions();
@@ -56,7 +59,7 @@ sumMCDCPairs(const ArrayRef<MCDCRecord> &Records) {
         ++CoveredPairs;
     }
   }
-  return {NumPairs, CoveredPairs};
+  return MCDCCoverageInfo(CoveredPairs, NumPairs);
 }
 
 static std::pair<RegionCoverageInfo, LineCoverageInfo>
@@ -85,24 +88,27 @@ sumRegions(ArrayRef<CountedRegion> CodeRegions, const CoverageData &CD) {
           LineCoverageInfo(CoveredLines, NumLines)};
 }
 
+CoverageDataSummary::CoverageDataSummary(const CoverageData &CD,
+                                         ArrayRef<CountedRegion> CodeRegions) {
+  std::tie(RegionCoverage, LineCoverage) = sumRegions(CodeRegions, CD);
+  BranchCoverage = sumBranches(CD.getBranches());
+  MCDCCoverage = sumMCDCPairs(CD.getMCDCRecords());
+}
+
 FunctionCoverageSummary
 FunctionCoverageSummary::get(const CoverageMapping &CM,
                              const coverage::FunctionRecord &Function) {
   CoverageData CD = CM.getCoverageForFunction(Function);
-  auto [RegionCoverage, LineCoverage] = sumRegions(Function.CountedRegions, CD);
 
-  // Compute the branch coverage, including branches from expansions.
-  size_t NumBranches = 0, CoveredBranches = 0;
-  sumBranches(NumBranches, CoveredBranches, CD.getBranches());
-  sumBranchExpansions(NumBranches, CoveredBranches, CM, CD.getExpansions());
+  auto Summary =
+      FunctionCoverageSummary(Function.Name, Function.ExecutionCount);
 
-  size_t NumPairs = 0, CoveredPairs = 0;
-  std::tie(NumPairs, CoveredPairs) = sumMCDCPairs(CD.getMCDCRecords());
+  Summary += CoverageDataSummary(CD, Function.CountedRegions);
 
-  return FunctionCoverageSummary(
-      Function.Name, Function.ExecutionCount, RegionCoverage, LineCoverage,
-      BranchCoverageInfo(CoveredBranches, NumBranches),
-      MCDCCoverageInfo(CoveredPairs, NumPairs));
+  // Compute the branch coverage, including branches from expansions.
+  Summary.BranchCoverage += sumBranchExpansions(CM, CD.getExpansions());
+
+  return Summary;
 }
 
 FunctionCoverageSummary
@@ -117,8 +123,7 @@ FunctionCoverageSummary::get(const InstantiationGroup &Group,
        << Group.getColumn();
   }
 
-  FunctionCoverageSummary Summary(Name);
-  Summary.ExecutionCount = Group.getTotalExecutionCount();
+  FunctionCoverageSummary Summary(Name, Group.getTotalExecutionCount());
   Summary.RegionCoverage = Summaries[0].RegionCoverage;
   Summary.LineCoverage = Summaries[0].LineCoverage;
   Summary.BranchCoverage = Summaries[0].BranchCoverage;
diff --git a/llvm/tools/llvm-cov/CoverageSummaryInfo.h b/llvm/tools/llvm-cov/CoverageSummaryInfo.h
index 64c2c8406cf3eb..d9210676c41bf3 100644
--- a/llvm/tools/llvm-cov/CoverageSummaryInfo.h
+++ b/llvm/tools/llvm-cov/CoverageSummaryInfo.h
@@ -223,26 +223,32 @@ class FunctionCoverageInfo {
   }
 };
 
-/// A summary of function's code coverage.
-struct FunctionCoverageSummary {
-  std::string Name;
-  uint64_t ExecutionCount;
+struct CoverageDataSummary {
   RegionCoverageInfo RegionCoverage;
   LineCoverageInfo LineCoverage;
   BranchCoverageInfo BranchCoverage;
   MCDCCoverageInfo MCDCCoverage;
 
-  FunctionCoverageSummary(const std::string &Name)
-      : Name(Name), ExecutionCount(0) {}
+  CoverageDataSummary() = default;
+  CoverageDataSummary(const coverage::CoverageData &CD,
+                      ArrayRef<coverage::CountedRegion> CodeRegions);
 
-  FunctionCoverageSummary(const std::string &Name, uint64_t ExecutionCount,
-                          const RegionCoverageInfo &RegionCoverage,
-                          const LineCoverageInfo &LineCoverage,
-                          const BranchCoverageInfo &BranchCoverage,
-                          const MCDCCoverageInfo &MCDCCoverage)
-      : Name(Name), ExecutionCount(ExecutionCount),
-        RegionCoverage(RegionCoverage), LineCoverage(LineCoverage),
-        BranchCoverage(BranchCoverage), MCDCCoverage(MCDCCoverage) {}
+  auto &operator+=(const CoverageDataSummary &RHS) {
+    RegionCoverage += RHS.RegionCoverage;
+    LineCoverage += RHS.LineCoverage;
+    BranchCoverage += RHS.BranchCoverage;
+    MCDCCoverage += RHS.MCDCCoverage;
+    return *this;
+  }
+};
+
+/// A summary of function's code coverage.
+struct FunctionCoverageSummary : CoverageDataSummary {
+  std::string Name;
+  uint64_t ExecutionCount;
+
+  FunctionCoverageSummary(const std::string &Name, uint64_t ExecutionCount = 0)
+      : Name(Name), ExecutionCount(ExecutionCount) {}
 
   /// Compute the code coverage summary for the given function coverage
   /// mapping record.
@@ -257,12 +263,8 @@ struct FunctionCoverageSummary {
 };
 
 /// A summary of file's code coverage.
-struct FileCoverageSummary {
+struct FileCoverageSummary : CoverageDataSummary {
   StringRef Name;
-  RegionCoverageInfo RegionCoverage;
-  LineCoverageInfo LineCoverage;
-  BranchCoverageInfo BranchCoverage;
-  MCDCCoverageInfo MCDCCoverage;
   FunctionCoverageInfo FunctionCoverage;
   FunctionCoverageInfo InstantiationCoverage;
 
@@ -270,11 +272,8 @@ struct FileCoverageSummary {
   FileCoverageSummary(StringRef Name) : Name(Name) {}
 
   FileCoverageSummary &operator+=(const FileCoverageSummary &RHS) {
-    RegionCoverage += RHS.RegionCoverage;
-    LineCoverage += RHS.LineCoverage;
+    *static_cast<CoverageDataSummary *>(this) += RHS;
     FunctionCoverage += RHS.FunctionCoverage;
-    BranchCoverage += RHS.BranchCoverage;
-    MCDCCoverage += RHS.MCDCCoverage;
     InstantiationCoverage += RHS.InstantiationCoverage;
     return *this;
   }



More information about the llvm-branch-commits mailing list