[llvm-branch-commits] [llvm] llvm-cov: [MCDC] Merge and recalculate independence pairs on template instantiations. (PR #121196)

via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Fri Dec 27 01:50:05 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-pgo

Author: NAKAMURA Takumi (chapuni)

<details>
<summary>Changes</summary>

Independence pairs are recalculated after merging `TestVectors`.

This introduces `MCDCRecord::BitmayByCond`. It makes `ExecTestVectors` reassociated with Bitmap index. Or it would require importing whole `TestVectors`, or recalculating `TestVectors` with covmap and profdata.

Depends on: #<!-- -->121194, #<!-- -->121195

---
Full diff: https://github.com/llvm/llvm-project/pull/121196.diff


4 Files Affected:

- (modified) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+11-5) 
- (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+48-9) 
- (modified) llvm/test/tools/llvm-cov/Inputs/mcdc-templates-merge.cpp (+2-2) 
- (modified) llvm/test/tools/llvm-cov/mcdc-templates-merge.test (+1-1) 


``````````diff
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index cfaee395655295..5d7d555a10bd3a 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -471,6 +471,7 @@ struct MCDCRecord {
     }
   };
 
+  using BitmapByCondTy = std::array<BitVector, 2>;
   using TestVectors = llvm::SmallVector<std::pair<TestVector, CondState>>;
   using BoolVector = std::array<BitVector, 2>;
   using TVRowPair = std::pair<unsigned, unsigned>;
@@ -481,6 +482,7 @@ struct MCDCRecord {
 private:
   CounterMappingRegion Region;
   TestVectors TV;
+  BitmapByCondTy BitmapByCond;
   std::optional<TVPairMap> IndependencePairs;
   BoolVector Folded;
   CondIDMap PosToID;
@@ -488,8 +490,10 @@ struct MCDCRecord {
 
 public:
   MCDCRecord(const CounterMappingRegion &Region, TestVectors &&TV,
-             BoolVector &&Folded, CondIDMap &&PosToID, LineColPairMap &&CondLoc)
-      : Region(Region), TV(std::move(TV)), Folded(std::move(Folded)),
+             BitmapByCondTy &&BitmapByCond, BoolVector &&Folded,
+             CondIDMap &&PosToID, LineColPairMap &&CondLoc)
+      : Region(Region), TV(std::move(TV)),
+        BitmapByCond(std::move(BitmapByCond)), Folded(std::move(Folded)),
         PosToID(std::move(PosToID)), CondLoc(std::move(CondLoc)) {
     findIndependencePairs();
   }
@@ -497,8 +501,9 @@ struct MCDCRecord {
   inline LineColPair viewLoc() const { return Region.endLoc(); }
 
   bool isMergeable(const MCDCRecord &RHS) const {
-    return (this->viewLoc() == RHS.viewLoc() && this->PosToID == RHS.PosToID &&
-            this->CondLoc == RHS.CondLoc);
+    return (this->viewLoc() == RHS.viewLoc() &&
+            this->BitmapByCond[false].size() == RHS.BitmapByCond[true].size() &&
+            this->PosToID == RHS.PosToID && this->CondLoc == RHS.CondLoc);
   }
 
   // This may invalidate IndependencePairs
@@ -577,7 +582,8 @@ struct MCDCRecord {
     auto [Covered, Folded] = getCoveredCount();
     auto NumTVs = getNumTestVectors();
     switch (Strategy) {
-    case MergeStrategy::Merge:
+    default:
+      llvm_unreachable("Not supported");
     case MergeStrategy::Any:
       return {
           Covered, // The largest covered number
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 76aa008886291e..8dd354f5122253 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -260,17 +260,55 @@ void CountedRegion::merge(const CountedRegion &RHS, MergeStrategy Strategy) {
 }
 
 void MCDCRecord::merge(MCDCRecord &&RHS, MergeStrategy Strategy) {
+  assert(this->TV.size() ==
+         this->BitmapByCond[false].count() + this->BitmapByCond[true].count());
+  assert(RHS.TV.size() ==
+         RHS.BitmapByCond[false].count() + RHS.BitmapByCond[true].count());
   assert(this->PosToID == RHS.PosToID);
   assert(this->CondLoc == RHS.CondLoc);
 
   switch (Strategy) {
   case MergeStrategy::Merge:
+    break;
   case MergeStrategy::Any:
   case MergeStrategy::All:
     if (this->getMergeRank(Strategy) < RHS.getMergeRank(Strategy))
       *this = std::move(RHS);
     return;
   }
+
+  std::array<TestVectors, 2> LHSTV;
+  auto LHSI = this->TV.begin();
+  auto RHSI = RHS.TV.begin();
+  bool Merged = false;
+  for (auto MCDCCond : {MCDCRecord::MCDC_False, MCDCRecord::MCDC_True}) {
+    auto &LHSBitmap = this->BitmapByCond[MCDCCond];
+    auto &RHSBitmap = RHS.BitmapByCond[MCDCCond];
+    for (unsigned I = 0, E = LHSBitmap.size(); I != E; ++I) {
+      if (LHSBitmap[I]) {
+        if (RHSBitmap[I])
+          ++RHSI;
+        LHSTV[LHSI->second].push_back(std::move(*LHSI++));
+      } else if (RHSBitmap[I]) {
+        LHSTV[RHSI->second].push_back(std::move(*RHSI++));
+        LHSBitmap[I] = true;
+        Merged = true;
+      }
+    }
+
+    this->Folded[MCDCCond] &= RHS.Folded[MCDCCond];
+  }
+
+  if (Merged)
+    IndependencePairs.reset();
+
+  assert(LHSI == this->TV.end());
+  assert(RHSI == RHS.TV.end());
+  this->TV = std::move(LHSTV[false]);
+  this->TV.append(std::make_move_iterator(LHSTV[true].begin()),
+                  std::make_move_iterator(LHSTV[true].end()));
+  assert(this->TV.size() ==
+         this->BitmapByCond[false].count() + this->BitmapByCond[true].count());
 }
 
 // Find an independence pair for each condition:
@@ -284,13 +322,7 @@ void MCDCRecord::findIndependencePairs() {
   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); })
-
-  );
+  unsigned TVTrueIdx = BitmapByCond[false].count();
   for (unsigned I = TVTrueIdx; I < NumTVs; ++I) {
     const auto &[A, ACond] = TV[I];
     assert(ACond == MCDCRecord::MCDC_True);
@@ -435,6 +467,7 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
   /// with a bit value of '1' indicates that the corresponding Test Vector
   /// identified by that index was executed.
   const BitVector &Bitmap;
+  MCDCRecord::BitmapByCondTy BitmapByCond;
 
   /// Decision Region to which the ExecutedTestVectorBitmap applies.
   const CounterMappingRegion &Region;
@@ -487,6 +520,7 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
                       ArrayRef<const CounterMappingRegion *> Branches,
                       bool IsVersion11)
       : NextIDsBuilder(Branches), TVIdxBuilder(this->NextIDs), Bitmap(Bitmap),
+        BitmapByCond{{BitVector(NumTestVectors), BitVector(NumTestVectors)}},
         Region(Region), DecisionParams(Region.getDecisionParams()),
         Branches(Branches), NumConditions(DecisionParams.NumConditions),
         Folded{{BitVector(NumConditions), BitVector(NumConditions)}},
@@ -518,6 +552,8 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
                       : DecisionParams.BitmapIdx - NumTestVectors + NextTVIdx])
         continue;
 
+      assert(!BitmapByCond[MCDCCond][NextTVIdx]);
+      BitmapByCond[MCDCCond][NextTVIdx] = true;
       ExecVectorIdxs.emplace_back(MCDCCond, NextTVIdx, ExecVectors.size());
 
       // Copy the completed test vector to the vector of testvectors.
@@ -547,6 +583,8 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
     for (const auto &IdxTuple : ExecVectorIdxs)
       NewTestVectors.push_back(std::move(ExecVectors[IdxTuple.Ord]));
     ExecVectors = std::move(NewTestVectors);
+
+    assert(!BitmapByCond[false].anyCommon(BitmapByCond[true]));
   }
 
 public:
@@ -585,8 +623,9 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
     findExecutedTestVectors();
 
     // Record Test vectors, executed vectors, and independence pairs.
-    return MCDCRecord(Region, std::move(ExecVectors), std::move(Folded),
-                      std::move(PosToID), std::move(CondLoc));
+    return MCDCRecord(Region, std::move(ExecVectors), std::move(BitmapByCond),
+                      std::move(Folded), std::move(PosToID),
+                      std::move(CondLoc));
   }
 };
 
diff --git a/llvm/test/tools/llvm-cov/Inputs/mcdc-templates-merge.cpp b/llvm/test/tools/llvm-cov/Inputs/mcdc-templates-merge.cpp
index 09c2e0980cca85..46d1260d122747 100644
--- a/llvm/test/tools/llvm-cov/Inputs/mcdc-templates-merge.cpp
+++ b/llvm/test/tools/llvm-cov/Inputs/mcdc-templates-merge.cpp
@@ -8,7 +8,7 @@ bool ab(Ty a, Ty b) {
 // ANY:   [[@LINE-3]]| 2| return
 // ALL:   [[@LINE-4]]| 0| return
 
-// MERGE: MC/DC Coverage for Decision{{[:]}}  50.00%
+// MERGE: MC/DC Coverage for Decision{{[:]}} 100.00%
 // ANY:   MC/DC Coverage for Decision{{[:]}}  50.00%
 // ALL:   MC/DC Coverage for Decision{{[:]}}   0.00%
 
@@ -28,7 +28,7 @@ bool Cab(bool a, bool b) {
 // ANY:   [[@LINE-3]]| 2| return
 // ALL:   [[@LINE-4]]| 2| return
 
-// MERGE:  MC/DC Coverage for Decision{{[:]}}  50.00%
+// MERGE:  MC/DC Coverage for Decision{{[:]}} 100.00%
 // ANY:    MC/DC Coverage for Decision{{[:]}}  50.00%
 // ALL:    MC/DC Coverage for Decision{{[:]}}   0.00%
 
diff --git a/llvm/test/tools/llvm-cov/mcdc-templates-merge.test b/llvm/test/tools/llvm-cov/mcdc-templates-merge.test
index 21c5458edfa846..d4369dc2db2930 100644
--- a/llvm/test/tools/llvm-cov/mcdc-templates-merge.test
+++ b/llvm/test/tools/llvm-cov/mcdc-templates-merge.test
@@ -34,7 +34,7 @@ ANY:        11  1  90.91%
 ALL:        12  7  41.67%
 
 # MC/DC Conditions
-MERGE:       4  2  50.00%
+MERGE:       5  0 100.00%
 ANY:         4  2  50.00%
 ALL:         4  4   0.00%
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/121196


More information about the llvm-branch-commits mailing list