[clang] [llvm] [MC/DC] Refactor: Introduce `ConditionIDs` as `std::array<2>` (PR #81221)

NAKAMURA Takumi via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 14 05:49:34 PST 2024


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

>From 66bb6cc3fd339360c16c6a98ce08f34978f665e0 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Fri, 9 Feb 2024 07:56:51 +0900
Subject: [PATCH 1/4] [MC/DC] Refactor: Introduce `ConditionIDs` as
 `std::array<2>`

Its 0th element corresponds to `FalseID` and 1st to `TrueID`.

CoverageMappingGen.cpp: `DecisionIDPair` is replaced with `ConditionIDs`
---
 clang/lib/CodeGen/CoverageMappingGen.cpp      | 50 +++++++------------
 .../ProfileData/Coverage/CoverageMapping.h    |  9 ++--
 .../ProfileData/Coverage/CoverageMapping.cpp  | 31 ++++++------
 .../Coverage/CoverageMappingReader.cpp        |  7 +--
 .../Coverage/CoverageMappingWriter.cpp        |  4 +-
 .../ProfileData/CoverageMappingTest.cpp       | 24 ++++-----
 6 files changed, 56 insertions(+), 69 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0c43317642bca4..54559af47e63dc 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -95,7 +95,6 @@ void CoverageSourceInfo::updateNextTokLoc(SourceLocation Loc) {
 }
 
 namespace {
-using MCDCConditionID = CounterMappingRegion::MCDCConditionID;
 using MCDCParameters = CounterMappingRegion::MCDCParameters;
 
 /// A region of source code that can be mapped to a counter.
@@ -586,11 +585,6 @@ struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder {
 /// creation.
 struct MCDCCoverageBuilder {
 
-  struct DecisionIDPair {
-    MCDCConditionID TrueID = 0;
-    MCDCConditionID FalseID = 0;
-  };
-
   /// The AST walk recursively visits nested logical-AND or logical-OR binary
   /// operator nodes and then visits their LHS and RHS children nodes.  As this
   /// happens, the algorithm will assign IDs to each operator's LHS and RHS side
@@ -681,14 +675,14 @@ struct MCDCCoverageBuilder {
 private:
   CodeGenModule &CGM;
 
-  llvm::SmallVector<DecisionIDPair> DecisionStack;
+  llvm::SmallVector<MCDCConditionIDs> DecisionStack;
   llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDs;
   llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap;
   MCDCConditionID NextID = 1;
   bool NotMapped = false;
 
   /// Represent a sentinel value of [0,0] for the bottom of DecisionStack.
-  static constexpr DecisionIDPair DecisionStackSentinel{0, 0};
+  static constexpr MCDCConditionIDs DecisionStackSentinel{0, 0};
 
   /// Is this a logical-AND operation?
   bool isLAnd(const BinaryOperator *E) const {
@@ -727,7 +721,7 @@ struct MCDCCoverageBuilder {
   }
 
   /// Return the LHS Decision ([0,0] if not set).
-  const DecisionIDPair &back() const { return DecisionStack.back(); }
+  const MCDCConditionIDs &back() const { return DecisionStack.back(); }
 
   /// Push the binary operator statement to track the nest level and assign IDs
   /// to the operator's LHS and RHS.  The RHS may be a larger subtree that is
@@ -744,7 +738,7 @@ struct MCDCCoverageBuilder {
     if (NotMapped)
       return;
 
-    const DecisionIDPair &ParentDecision = DecisionStack.back();
+    const MCDCConditionIDs &ParentDecision = DecisionStack.back();
 
     // If the operator itself has an assigned ID, this means it represents a
     // larger subtree.  In this case, assign that ID to its LHS node.  Its RHS
@@ -760,18 +754,18 @@ struct MCDCCoverageBuilder {
 
     // Push the LHS decision IDs onto the DecisionStack.
     if (isLAnd(E))
-      DecisionStack.push_back({RHSid, ParentDecision.FalseID});
+      DecisionStack.push_back({ParentDecision[0], RHSid});
     else
-      DecisionStack.push_back({ParentDecision.TrueID, RHSid});
+      DecisionStack.push_back({RHSid, ParentDecision[1]});
   }
 
   /// Pop and return the LHS Decision ([0,0] if not set).
-  DecisionIDPair pop() {
+  MCDCConditionIDs pop() {
     if (!CGM.getCodeGenOpts().MCDCCoverage || NotMapped)
       return DecisionStack.front();
 
     assert(DecisionStack.size() > 1);
-    DecisionIDPair D = DecisionStack.back();
+    MCDCConditionIDs D = DecisionStack.back();
     DecisionStack.pop_back();
     return D;
   }
@@ -865,8 +859,7 @@ struct CounterCoverageMappingBuilder
                     std::optional<SourceLocation> StartLoc = std::nullopt,
                     std::optional<SourceLocation> EndLoc = std::nullopt,
                     std::optional<Counter> FalseCount = std::nullopt,
-                    MCDCConditionID ID = 0, MCDCConditionID TrueID = 0,
-                    MCDCConditionID FalseID = 0) {
+                    MCDCConditionID ID = 0, MCDCConditionIDs Conds = {}) {
 
     if (StartLoc && !FalseCount) {
       MostRecentLocation = *StartLoc;
@@ -885,8 +878,7 @@ struct CounterCoverageMappingBuilder
       StartLoc = std::nullopt;
     if (EndLoc && EndLoc->isInvalid())
       EndLoc = std::nullopt;
-    RegionStack.emplace_back(Count, FalseCount,
-                             MCDCParameters{0, 0, ID, TrueID, FalseID},
+    RegionStack.emplace_back(Count, FalseCount, MCDCParameters{0, 0, ID, Conds},
                              StartLoc, EndLoc);
 
     return RegionStack.size() - 1;
@@ -1024,15 +1016,12 @@ struct CounterCoverageMappingBuilder
     return (Cond->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext()));
   }
 
-  using MCDCDecisionIDPair = MCDCCoverageBuilder::DecisionIDPair;
-
   /// Create a Branch Region around an instrumentable condition for coverage
   /// and add it to the function's SourceRegions.  A branch region tracks a
   /// "True" counter and a "False" counter for boolean expressions that
   /// result in the generation of a branch.
-  void
-  createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt,
-                     const MCDCDecisionIDPair &IDPair = MCDCDecisionIDPair()) {
+  void createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt,
+                          const MCDCConditionIDs &Conds = {}) {
     // Check for NULL conditions.
     if (!C)
       return;
@@ -1043,8 +1032,6 @@ struct CounterCoverageMappingBuilder
     // code other than the Condition.
     if (CodeGenFunction::isInstrumentedCondition(C)) {
       MCDCConditionID ID = MCDCBuilder.getCondID(C);
-      MCDCConditionID TrueID = IDPair.TrueID;
-      MCDCConditionID FalseID = IDPair.FalseID;
 
       // If a condition can fold to true or false, the corresponding branch
       // will be removed.  Create a region with both counters hard-coded to
@@ -1054,11 +1041,11 @@ struct CounterCoverageMappingBuilder
       // CodeGenFunction.c always returns false, but that is very heavy-handed.
       if (ConditionFoldsToBool(C))
         popRegions(pushRegion(Counter::getZero(), getStart(C), getEnd(C),
-                              Counter::getZero(), ID, TrueID, FalseID));
+                              Counter::getZero(), ID, Conds));
       else
         // Otherwise, create a region with the True counter and False counter.
-        popRegions(pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt, ID,
-                              TrueID, FalseID));
+        popRegions(
+            pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt, ID, Conds));
     }
   }
 
@@ -1152,8 +1139,7 @@ struct CounterCoverageMappingBuilder
             SourceRegions.emplace_back(
                 I.getCounter(), I.getFalseCounter(),
                 MCDCParameters{0, 0, I.getMCDCParams().ID,
-                               I.getMCDCParams().TrueID,
-                               I.getMCDCParams().FalseID},
+                               I.getMCDCParams().Conds},
                 Loc, getEndOfFileOrMacro(Loc), I.isBranch());
           else
             SourceRegions.emplace_back(I.getCounter(), Loc,
@@ -2134,8 +2120,8 @@ static void dump(llvm::raw_ostream &OS, StringRef FunctionName,
     }
 
     if (R.Kind == CounterMappingRegion::MCDCBranchRegion) {
-      OS << " [" << R.MCDCParams.ID << "," << R.MCDCParams.TrueID;
-      OS << "," << R.MCDCParams.FalseID << "] ";
+      OS << " [" << R.MCDCParams.ID << "," << R.MCDCParams.Conds[1];
+      OS << "," << R.MCDCParams.Conds[0] << "] ";
     }
 
     if (R.Kind == CounterMappingRegion::ExpansionRegion)
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 88ec60c7aa33c6..d07baa8e71a29f 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -30,6 +30,7 @@
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
+#include <array>
 #include <cassert>
 #include <cstdint>
 #include <iterator>
@@ -37,7 +38,6 @@
 #include <sstream>
 #include <string>
 #include <system_error>
-#include <tuple>
 #include <utility>
 #include <vector>
 
@@ -217,6 +217,9 @@ class CounterExpressionBuilder {
 
 using LineColPair = std::pair<unsigned, unsigned>;
 
+using MCDCConditionID = unsigned int;
+using MCDCConditionIDs = std::array<MCDCConditionID, 2>;
+
 /// A Counter mapping region associates a source range with a specific counter.
 struct CounterMappingRegion {
   enum RegionKind {
@@ -249,7 +252,6 @@ struct CounterMappingRegion {
     MCDCBranchRegion
   };
 
-  using MCDCConditionID = unsigned int;
   struct MCDCParameters {
     /// Byte Index of Bitmap Coverage Object for a Decision Region.
     unsigned BitmapIdx = 0;
@@ -259,7 +261,8 @@ struct CounterMappingRegion {
 
     /// IDs used to represent a branch region and other branch regions
     /// evaluated based on True and False branches.
-    MCDCConditionID ID = 0, TrueID = 0, FalseID = 0;
+    MCDCConditionID ID = 0;
+    MCDCConditionIDs Conds;
   };
 
   /// Primary Counter that is also used for Branch Regions (TrueCount).
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index eb0996e33b70dc..66152685af831b 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -245,7 +245,7 @@ class MCDCRecordProcessor {
   unsigned BitmapIdx;
 
   /// Mapping of a condition ID to its corresponding branch region.
-  llvm::DenseMap<unsigned, const CounterMappingRegion *> Map;
+  llvm::DenseMap<unsigned, MCDCConditionIDs> CondsMap;
 
   /// Vector used to track whether a condition is constant folded.
   MCDCRecord::BoolVector Folded;
@@ -285,20 +285,17 @@ class MCDCRecordProcessor {
   // the truth table.
   void buildTestVector(MCDCRecord::TestVector &TV, unsigned ID,
                        unsigned Index) {
-    const CounterMappingRegion *Branch = Map[ID];
-
-    TV[ID - 1] = MCDCRecord::MCDC_False;
-    if (Branch->MCDCParams.FalseID > 0)
-      buildTestVector(TV, Branch->MCDCParams.FalseID, Index);
-    else
-      recordTestVector(TV, Index, MCDCRecord::MCDC_False);
-
-    Index |= 1 << (ID - 1);
-    TV[ID - 1] = MCDCRecord::MCDC_True;
-    if (Branch->MCDCParams.TrueID > 0)
-      buildTestVector(TV, Branch->MCDCParams.TrueID, Index);
-    else
-      recordTestVector(TV, Index, MCDCRecord::MCDC_True);
+    const auto &Conds = CondsMap[ID];
+
+    for (unsigned I = 0; I < 2; ++I) {
+      auto MCDCCond = (I ? MCDCRecord::MCDC_True : MCDCRecord::MCDC_False);
+      Index |= I << (ID - 1);
+      TV[ID - 1] = MCDCCond;
+      if (Conds[I] > 0)
+        buildTestVector(TV, Conds[I], Index);
+      else
+        recordTestVector(TV, Index, MCDCCond);
+    }
 
     // Reset back to DontCare.
     TV[ID - 1] = MCDCRecord::MCDC_DontCare;
@@ -371,7 +368,7 @@ class MCDCRecordProcessor {
     // - Record whether the condition is constant folded so that we exclude it
     //   from being measured.
     for (const auto *B : Branches) {
-      Map[B->MCDCParams.ID] = B;
+      CondsMap[B->MCDCParams.ID] = B->MCDCParams.Conds;
       PosToID[I] = B->MCDCParams.ID - 1;
       CondLoc[I] = B->startLoc();
       Folded[I++] = (B->Count.isZero() && B->FalseCount.isZero());
@@ -524,7 +521,7 @@ class MCDCDecisionRecorder {
 
     /// IDs that are stored in MCDCBranches
     /// Complete when all IDs (1 to NumConditions) are met.
-    DenseSet<CounterMappingRegion::MCDCConditionID> ConditionIDs;
+    DenseSet<MCDCConditionID> ConditionIDs;
 
     /// Set of IDs of Expansion(s) that are relevant to DecisionRegion
     /// and its children (via expansions).
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
index ac8e6b56379f21..351c16397da8f0 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -372,9 +372,10 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray(
     auto CMR = CounterMappingRegion(
         C, C2,
         CounterMappingRegion::MCDCParameters{
-            static_cast<unsigned>(BIDX), static_cast<unsigned>(NC),
-            static_cast<unsigned>(ID), static_cast<unsigned>(TID),
-            static_cast<unsigned>(FID)},
+            static_cast<unsigned>(BIDX),
+            static_cast<unsigned>(NC),
+            static_cast<unsigned>(ID),
+            {static_cast<unsigned>(FID), static_cast<unsigned>(TID)}},
         InferredFileID, ExpandedFileID, LineStart, ColumnStart,
         LineStart + NumLines, ColumnEnd, Kind);
     if (CMR.startLoc() > CMR.endLoc())
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
index 27727f216b0513..5a5d546de8bbb7 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
@@ -252,8 +252,8 @@ void CoverageMappingWriter::write(raw_ostream &OS) {
       writeCounter(MinExpressions, Count, OS);
       writeCounter(MinExpressions, FalseCount, OS);
       encodeULEB128(unsigned(I->MCDCParams.ID), OS);
-      encodeULEB128(unsigned(I->MCDCParams.TrueID), OS);
-      encodeULEB128(unsigned(I->MCDCParams.FalseID), OS);
+      encodeULEB128(unsigned(I->MCDCParams.Conds[1]), OS);
+      encodeULEB128(unsigned(I->MCDCParams.Conds[0]), OS);
       break;
     case CounterMappingRegion::MCDCDecisionRegion:
       encodeULEB128(unsigned(I->Kind)
diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
index 2849781a9dc43b..c26c096fdf9f36 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -197,18 +197,18 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
     auto &Regions = InputFunctions.back().Regions;
     unsigned FileID = getFileIndexForFunction(File);
     Regions.push_back(CounterMappingRegion::makeDecisionRegion(
-        CounterMappingRegion::MCDCParameters{Mask, NC}, FileID, LS, CS, LE,
-        CE));
+        CounterMappingRegion::MCDCParameters{Mask, NC, 0, {}}, FileID, LS, CS,
+        LE, CE));
   }
 
-  void addMCDCBranchCMR(Counter C1, Counter C2, unsigned ID, unsigned TrueID,
-                        unsigned FalseID, StringRef File, unsigned LS,
+  void addMCDCBranchCMR(Counter C1, Counter C2, unsigned ID,
+                        MCDCConditionIDs Conds, StringRef File, unsigned LS,
                         unsigned CS, unsigned LE, unsigned CE) {
     auto &Regions = InputFunctions.back().Regions;
     unsigned FileID = getFileIndexForFunction(File);
     Regions.push_back(CounterMappingRegion::makeBranchRegion(
-        C1, C2, CounterMappingRegion::MCDCParameters{0, 0, ID, TrueID, FalseID},
-        FileID, LS, CS, LE, CE));
+        C1, C2, CounterMappingRegion::MCDCParameters{0, 0, ID, Conds}, FileID,
+        LS, CS, LE, CE));
   }
 
   void addExpansionCMR(StringRef File, StringRef ExpandedFile, unsigned LS,
@@ -874,9 +874,9 @@ TEST_P(CoverageMappingTest, non_code_region_bitmask) {
   addCMR(Counter::getCounter(3), "file", 1, 1, 5, 5);
 
   addMCDCDecisionCMR(0, 2, "file", 7, 1, 7, 6);
-  addMCDCBranchCMR(Counter::getCounter(0), Counter::getCounter(1), 1, 2, 0,
+  addMCDCBranchCMR(Counter::getCounter(0), Counter::getCounter(1), 1, {0, 2},
                    "file", 7, 2, 7, 3);
-  addMCDCBranchCMR(Counter::getCounter(2), Counter::getCounter(3), 2, 0, 0,
+  addMCDCBranchCMR(Counter::getCounter(2), Counter::getCounter(3), 2, {0, 0},
                    "file", 7, 4, 7, 5);
 
   EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
@@ -902,11 +902,11 @@ TEST_P(CoverageMappingTest, decision_before_expansion) {
   addExpansionCMR("foo", "B", 4, 19, 4, 20);
   addCMR(Counter::getCounter(0), "A", 1, 14, 1, 17);
   addCMR(Counter::getCounter(0), "A", 1, 14, 1, 17);
-  addMCDCBranchCMR(Counter::getCounter(0), Counter::getCounter(1), 1, 2, 0, "A",
-                   1, 14, 1, 17);
+  addMCDCBranchCMR(Counter::getCounter(0), Counter::getCounter(1), 1, {0, 2},
+                   "A", 1, 14, 1, 17);
   addCMR(Counter::getCounter(1), "B", 1, 14, 1, 17);
-  addMCDCBranchCMR(Counter::getCounter(1), Counter::getCounter(2), 2, 0, 0, "B",
-                   1, 14, 1, 17);
+  addMCDCBranchCMR(Counter::getCounter(1), Counter::getCounter(2), 2, {0, 0},
+                   "B", 1, 14, 1, 17);
 
   // InputFunctionCoverageData::Regions is rewritten after the write.
   auto InputRegions = InputFunctions.back().Regions;

>From 1e0ac44979dffa6e82a07b56dfad18b7f1a8fc61 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sun, 11 Feb 2024 00:11:09 +0900
Subject: [PATCH 2/4] Expand `recordTestVector()`

---
 .../ProfileData/Coverage/CoverageMapping.cpp  | 29 +++++++++----------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 66152685af831b..66be042c4a716e 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -267,19 +267,6 @@ class MCDCRecordProcessor {
         Folded(NumConditions, false), IndependencePairs(NumConditions) {}
 
 private:
-  void recordTestVector(MCDCRecord::TestVector &TV, unsigned Index,
-                        MCDCRecord::CondState Result) {
-    if (!Bitmap[BitmapIdx + Index])
-      return;
-
-    // Copy the completed test vector to the vector of testvectors.
-    ExecVectors.push_back(TV);
-
-    // The final value (T,F) is equal to the last non-dontcare state on the
-    // path (in a short-circuiting system).
-    ExecVectors.back().push_back(Result);
-  }
-
   // Walk the binary decision diagram and try assigning both false and true to
   // each node. When a terminal node (ID == 0) is reached, fill in the value in
   // the truth table.
@@ -291,10 +278,20 @@ class MCDCRecordProcessor {
       auto MCDCCond = (I ? MCDCRecord::MCDC_True : MCDCRecord::MCDC_False);
       Index |= I << (ID - 1);
       TV[ID - 1] = MCDCCond;
-      if (Conds[I] > 0)
+      if (Conds[I] > 0) {
         buildTestVector(TV, Conds[I], Index);
-      else
-        recordTestVector(TV, Index, MCDCCond);
+        continue;
+      }
+
+      if (!Bitmap[BitmapIdx + Index])
+        continue;
+
+      // Copy the completed test vector to the vector of testvectors.
+      ExecVectors.push_back(TV);
+
+      // The final value (T,F) is equal to the last non-dontcare state on the
+      // path (in a short-circuiting system).
+      ExecVectors.back().push_back(MCDCCond);
     }
 
     // Reset back to DontCare.

>From a47ef780e2b61ebd30045d48ad511889d12627eb Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sat, 10 Feb 2024 23:44:51 +0900
Subject: [PATCH 3/4] Symbolize. (Maybe funny)

---
 clang/lib/CodeGen/CoverageMappingGen.cpp           |  8 ++++----
 llvm/lib/ProfileData/Coverage/CoverageMapping.cpp  | 14 ++++++++------
 .../ProfileData/Coverage/CoverageMappingWriter.cpp |  4 ++--
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 54559af47e63dc..b53f3f6182e733 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -754,9 +754,9 @@ struct MCDCCoverageBuilder {
 
     // Push the LHS decision IDs onto the DecisionStack.
     if (isLAnd(E))
-      DecisionStack.push_back({ParentDecision[0], RHSid});
+      DecisionStack.push_back({ParentDecision[false], RHSid});
     else
-      DecisionStack.push_back({RHSid, ParentDecision[1]});
+      DecisionStack.push_back({RHSid, ParentDecision[true]});
   }
 
   /// Pop and return the LHS Decision ([0,0] if not set).
@@ -2120,8 +2120,8 @@ static void dump(llvm::raw_ostream &OS, StringRef FunctionName,
     }
 
     if (R.Kind == CounterMappingRegion::MCDCBranchRegion) {
-      OS << " [" << R.MCDCParams.ID << "," << R.MCDCParams.Conds[1];
-      OS << "," << R.MCDCParams.Conds[0] << "] ";
+      OS << " [" << R.MCDCParams.ID << "," << R.MCDCParams.Conds[true];
+      OS << "," << R.MCDCParams.Conds[false] << "] ";
     }
 
     if (R.Kind == CounterMappingRegion::ExpansionRegion)
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 66be042c4a716e..b4a83a6ed9fc07 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -272,14 +272,16 @@ class MCDCRecordProcessor {
   // the truth table.
   void buildTestVector(MCDCRecord::TestVector &TV, unsigned ID,
                        unsigned Index) {
-    const auto &Conds = CondsMap[ID];
+    assert((Index & (1 << (ID - 1))) == 0);
 
-    for (unsigned I = 0; I < 2; ++I) {
-      auto MCDCCond = (I ? MCDCRecord::MCDC_True : MCDCRecord::MCDC_False);
-      Index |= I << (ID - 1);
+    for (auto MCDCCond : {MCDCRecord::MCDC_False, MCDCRecord::MCDC_True}) {
+      static_assert(MCDCRecord::MCDC_False == 0);
+      static_assert(MCDCRecord::MCDC_True == 1);
+      Index |= MCDCCond << (ID - 1);
       TV[ID - 1] = MCDCCond;
-      if (Conds[I] > 0) {
-        buildTestVector(TV, Conds[I], Index);
+      auto NextID = CondsMap[ID][MCDCCond];
+      if (NextID > 0) {
+        buildTestVector(TV, NextID, Index);
         continue;
       }
 
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
index 5a5d546de8bbb7..f67cfc8306e8b1 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
@@ -252,8 +252,8 @@ void CoverageMappingWriter::write(raw_ostream &OS) {
       writeCounter(MinExpressions, Count, OS);
       writeCounter(MinExpressions, FalseCount, OS);
       encodeULEB128(unsigned(I->MCDCParams.ID), OS);
-      encodeULEB128(unsigned(I->MCDCParams.Conds[1]), OS);
-      encodeULEB128(unsigned(I->MCDCParams.Conds[0]), OS);
+      encodeULEB128(unsigned(I->MCDCParams.Conds[true]), OS);
+      encodeULEB128(unsigned(I->MCDCParams.Conds[false]), OS);
       break;
     case CounterMappingRegion::MCDCDecisionRegion:
       encodeULEB128(unsigned(I->Kind)

>From 3756c7a81bba7c7c4306c2c8451336f910d98673 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Wed, 14 Feb 2024 22:46:09 +0900
Subject: [PATCH 4/4] ids fix

---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 82e1d875016ff0..93c3c31e71fa83 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -684,6 +684,7 @@ struct MCDCCoverageBuilder {
   CodeGenModule &CGM;
 
   llvm::SmallVector<mcdc::ConditionIDs> DecisionStack;
+  MCDC::State &MCDCState;
   llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDs;
   mcdc::ConditionID NextID = 1;
   bool NotMapped = false;



More information about the cfe-commits mailing list