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] Sort MCDCRecord::ExecVectors order by Bitmap index #121195

Open
wants to merge 1 commit into
base: users/chapuni/cov/merge/mcdcsort-base
Choose a base branch
from

Conversation

chapuni
Copy link
Contributor

@chapuni chapuni commented Dec 27, 2024

This makes easier to merge MCDCRecords in later stages.

Depends on: #120842, #110966, #121188, #121190

@llvmbot llvmbot added the PGO Profile Guided Optimizations label Dec 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 27, 2024

@llvm/pr-subscribers-pgo

Author: NAKAMURA Takumi (chapuni)

Changes

This makes easier to merge MCDCRecords in later stages.

Depends on: #120842, #110966, #121188, #121190


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

3 Files Affected:

  • (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+27-13)
  • (modified) llvm/test/tools/llvm-cov/mcdc-const.test (+16-16)
  • (modified) llvm/test/tools/llvm-cov/mcdc-general.test (+4-4)
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:  |

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants