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

[MC/DC] Refactor: Introduce MCDCTypes.h for coverage::mcdc #81459

Merged
merged 3 commits into from
Feb 13, 2024

Conversation

chapuni
Copy link
Contributor

@chapuni chapuni commented Feb 12, 2024

They can be also used in clang.
Introduce the lightweight header instead of CoverageMapping.h.

This includes for now:

  • mcdc::ConditionID
  • mcdc::Parameters

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen PGO Profile Guided Optimizations labels Feb 12, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 12, 2024

@llvm/pr-subscribers-pgo

Author: NAKAMURA Takumi (chapuni)

Changes

They can be also used in clang.
Introduce the lightweight header instead of CoverageMapping.h.


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

6 Files Affected:

  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+26-27)
  • (modified) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+11-22)
  • (added) llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h (+36)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+1-1)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp (+3-4)
  • (modified) llvm/unittests/ProfileData/CoverageMappingTest.cpp (+3-4)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0c43317642bca4..888725dd2fc734 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -95,8 +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.
 class SourceMappingRegion {
@@ -107,7 +105,7 @@ class SourceMappingRegion {
   std::optional<Counter> FalseCount;
 
   /// Parameters used for Modified Condition/Decision Coverage
-  MCDCParameters MCDCParams;
+  mcdc::Parameters MCDCParams;
 
   /// The region's starting location.
   std::optional<SourceLocation> LocStart;
@@ -131,7 +129,7 @@ class SourceMappingRegion {
         SkippedRegion(false) {}
 
   SourceMappingRegion(Counter Count, std::optional<Counter> FalseCount,
-                      MCDCParameters MCDCParams,
+                      mcdc::Parameters MCDCParams,
                       std::optional<SourceLocation> LocStart,
                       std::optional<SourceLocation> LocEnd,
                       bool GapRegion = false)
@@ -139,7 +137,7 @@ class SourceMappingRegion {
         LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion),
         SkippedRegion(false) {}
 
-  SourceMappingRegion(MCDCParameters MCDCParams,
+  SourceMappingRegion(mcdc::Parameters MCDCParams,
                       std::optional<SourceLocation> LocStart,
                       std::optional<SourceLocation> LocEnd)
       : MCDCParams(MCDCParams), LocStart(LocStart), LocEnd(LocEnd),
@@ -187,7 +185,7 @@ class SourceMappingRegion {
 
   bool isMCDCDecision() const { return MCDCParams.NumConditions != 0; }
 
-  const MCDCParameters &getMCDCParams() const { return MCDCParams; }
+  const mcdc::Parameters &getMCDCParams() const { return MCDCParams; }
 };
 
 /// Spelling locations for the start and end of a source region.
@@ -587,8 +585,8 @@ struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder {
 struct MCDCCoverageBuilder {
 
   struct DecisionIDPair {
-    MCDCConditionID TrueID = 0;
-    MCDCConditionID FalseID = 0;
+    mcdc::ConditionID TrueID = 0;
+    mcdc::ConditionID FalseID = 0;
   };
 
   /// The AST walk recursively visits nested logical-AND or logical-OR binary
@@ -682,9 +680,9 @@ struct MCDCCoverageBuilder {
   CodeGenModule &CGM;
 
   llvm::SmallVector<DecisionIDPair> DecisionStack;
-  llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDs;
+  llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDs;
   llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap;
-  MCDCConditionID NextID = 1;
+  mcdc::ConditionID NextID = 1;
   bool NotMapped = false;
 
   /// Represent a sentinel value of [0,0] for the bottom of DecisionStack.
@@ -696,9 +694,10 @@ struct MCDCCoverageBuilder {
   }
 
 public:
-  MCDCCoverageBuilder(CodeGenModule &CGM,
-                      llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDMap,
-                      llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap)
+  MCDCCoverageBuilder(
+      CodeGenModule &CGM,
+      llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDMap,
+      llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap)
       : CGM(CGM), DecisionStack(1, DecisionStackSentinel), CondIDs(CondIDMap),
         MCDCBitmapMap(MCDCBitmapMap) {}
 
@@ -713,12 +712,12 @@ struct MCDCCoverageBuilder {
   bool isBuilding() const { return (NextID > 1); }
 
   /// Set the given condition's ID.
-  void setCondID(const Expr *Cond, MCDCConditionID ID) {
+  void setCondID(const Expr *Cond, mcdc::ConditionID ID) {
     CondIDs[CodeGenFunction::stripCond(Cond)] = ID;
   }
 
   /// Return the ID of a given condition.
-  MCDCConditionID getCondID(const Expr *Cond) const {
+  mcdc::ConditionID getCondID(const Expr *Cond) const {
     auto I = CondIDs.find(CodeGenFunction::stripCond(Cond));
     if (I == CondIDs.end())
       return 0;
@@ -755,7 +754,7 @@ struct MCDCCoverageBuilder {
       setCondID(E->getLHS(), NextID++);
 
     // Assign a ID+1 for the RHS.
-    MCDCConditionID RHSid = NextID++;
+    mcdc::ConditionID RHSid = NextID++;
     setCondID(E->getRHS(), RHSid);
 
     // Push the LHS decision IDs onto the DecisionStack.
@@ -865,8 +864,8 @@ 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) {
+                    mcdc::ConditionID ID = 0, mcdc::ConditionID TrueID = 0,
+                    mcdc::ConditionID FalseID = 0) {
 
     if (StartLoc && !FalseCount) {
       MostRecentLocation = *StartLoc;
@@ -886,7 +885,7 @@ struct CounterCoverageMappingBuilder
     if (EndLoc && EndLoc->isInvalid())
       EndLoc = std::nullopt;
     RegionStack.emplace_back(Count, FalseCount,
-                             MCDCParameters{0, 0, ID, TrueID, FalseID},
+                             mcdc::Parameters{0, 0, ID, TrueID, FalseID},
                              StartLoc, EndLoc);
 
     return RegionStack.size() - 1;
@@ -896,7 +895,7 @@ struct CounterCoverageMappingBuilder
                     std::optional<SourceLocation> StartLoc = std::nullopt,
                     std::optional<SourceLocation> EndLoc = std::nullopt) {
 
-    RegionStack.emplace_back(MCDCParameters{BitmapIdx, Conditions}, StartLoc,
+    RegionStack.emplace_back(mcdc::Parameters{BitmapIdx, Conditions}, StartLoc,
                              EndLoc);
 
     return RegionStack.size() - 1;
@@ -1042,9 +1041,9 @@ struct CounterCoverageMappingBuilder
     // function's SourceRegions) because it doesn't apply to any other source
     // code other than the Condition.
     if (CodeGenFunction::isInstrumentedCondition(C)) {
-      MCDCConditionID ID = MCDCBuilder.getCondID(C);
-      MCDCConditionID TrueID = IDPair.TrueID;
-      MCDCConditionID FalseID = IDPair.FalseID;
+      mcdc::ConditionID ID = MCDCBuilder.getCondID(C);
+      mcdc::ConditionID TrueID = IDPair.TrueID;
+      mcdc::ConditionID 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
@@ -1151,9 +1150,9 @@ struct CounterCoverageMappingBuilder
           if (I.isBranch())
             SourceRegions.emplace_back(
                 I.getCounter(), I.getFalseCounter(),
-                MCDCParameters{0, 0, I.getMCDCParams().ID,
-                               I.getMCDCParams().TrueID,
-                               I.getMCDCParams().FalseID},
+                mcdc::Parameters{0, 0, I.getMCDCParams().ID,
+                                 I.getMCDCParams().TrueID,
+                                 I.getMCDCParams().FalseID},
                 Loc, getEndOfFileOrMacro(Loc), I.isBranch());
           else
             SourceRegions.emplace_back(I.getCounter(), Loc,
@@ -1338,7 +1337,7 @@ struct CounterCoverageMappingBuilder
       CoverageMappingModuleGen &CVM,
       llvm::DenseMap<const Stmt *, unsigned> &CounterMap,
       llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap,
-      llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDMap,
+      llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDMap,
       SourceManager &SM, const LangOptions &LangOpts)
       : CoverageMappingBuilder(CVM, SM, LangOpts), CounterMap(CounterMap),
         MCDCBitmapMap(MCDCBitmapMap),
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 88ec60c7aa33c6..2869ae8fe861cc 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -23,6 +23,7 @@
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Object/BuildID.h"
+#include "llvm/ProfileData/Coverage/MCDCTypes.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Alignment.h"
 #include "llvm/Support/Compiler.h"
@@ -249,19 +250,6 @@ struct CounterMappingRegion {
     MCDCBranchRegion
   };
 
-  using MCDCConditionID = unsigned int;
-  struct MCDCParameters {
-    /// Byte Index of Bitmap Coverage Object for a Decision Region.
-    unsigned BitmapIdx = 0;
-
-    /// Number of Conditions used for a Decision Region.
-    unsigned NumConditions = 0;
-
-    /// 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;
-  };
-
   /// Primary Counter that is also used for Branch Regions (TrueCount).
   Counter Count;
 
@@ -269,7 +257,7 @@ struct CounterMappingRegion {
   Counter FalseCount;
 
   /// Parameters used for Modified Condition/Decision Coverage
-  MCDCParameters MCDCParams;
+  mcdc::Parameters MCDCParams;
 
   unsigned FileID = 0;
   unsigned ExpandedFileID = 0;
@@ -285,7 +273,7 @@ struct CounterMappingRegion {
         ColumnEnd(ColumnEnd), Kind(Kind) {}
 
   CounterMappingRegion(Counter Count, Counter FalseCount,
-                       MCDCParameters MCDCParams, unsigned FileID,
+                       mcdc::Parameters MCDCParams, unsigned FileID,
                        unsigned ExpandedFileID, unsigned LineStart,
                        unsigned ColumnStart, unsigned LineEnd,
                        unsigned ColumnEnd, RegionKind Kind)
@@ -294,7 +282,7 @@ struct CounterMappingRegion {
         ColumnStart(ColumnStart), LineEnd(LineEnd), ColumnEnd(ColumnEnd),
         Kind(Kind) {}
 
-  CounterMappingRegion(MCDCParameters MCDCParams, unsigned FileID,
+  CounterMappingRegion(mcdc::Parameters MCDCParams, unsigned FileID,
                        unsigned LineStart, unsigned ColumnStart,
                        unsigned LineEnd, unsigned ColumnEnd, RegionKind Kind)
       : MCDCParams(MCDCParams), FileID(FileID), LineStart(LineStart),
@@ -334,15 +322,16 @@ struct CounterMappingRegion {
   makeBranchRegion(Counter Count, Counter FalseCount, unsigned FileID,
                    unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
                    unsigned ColumnEnd) {
-    return CounterMappingRegion(Count, FalseCount, MCDCParameters(), FileID, 0,
-                                LineStart, ColumnStart, LineEnd, ColumnEnd,
+    return CounterMappingRegion(Count, FalseCount, mcdc::Parameters(), FileID,
+                                0, LineStart, ColumnStart, LineEnd, ColumnEnd,
                                 BranchRegion);
   }
 
   static CounterMappingRegion
-  makeBranchRegion(Counter Count, Counter FalseCount, MCDCParameters MCDCParams,
-                   unsigned FileID, unsigned LineStart, unsigned ColumnStart,
-                   unsigned LineEnd, unsigned ColumnEnd) {
+  makeBranchRegion(Counter Count, Counter FalseCount,
+                   mcdc::Parameters MCDCParams, unsigned FileID,
+                   unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
+                   unsigned ColumnEnd) {
     return CounterMappingRegion(Count, FalseCount, MCDCParams, FileID, 0,
                                 LineStart, ColumnStart, LineEnd, ColumnEnd,
                                 MCDCParams.ID == 0 ? BranchRegion
@@ -350,7 +339,7 @@ struct CounterMappingRegion {
   }
 
   static CounterMappingRegion
-  makeDecisionRegion(MCDCParameters MCDCParams, unsigned FileID,
+  makeDecisionRegion(mcdc::Parameters MCDCParams, unsigned FileID,
                      unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
                      unsigned ColumnEnd) {
     return CounterMappingRegion(MCDCParams, FileID, LineStart, ColumnStart,
diff --git a/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
new file mode 100644
index 00000000000000..0d3a79c4674eb2
--- /dev/null
+++ b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
@@ -0,0 +1,36 @@
+//===- CoverageMapping.h - Code coverage mapping support --------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Types related to MC/DC Coverage.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
+#define LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
+
+namespace llvm::coverage::mcdc {
+
+/// The ID for MCDCBranch.
+using ConditionID = unsigned int;
+
+/// MC/DC-specifig parameters
+struct Parameters {
+  /// Byte Index of Bitmap Coverage Object for a Decision Region.
+  unsigned BitmapIdx = 0;
+
+  /// Number of Conditions used for a Decision Region.
+  unsigned NumConditions = 0;
+
+  /// IDs used to represent a branch region and other branch regions
+  /// evaluated based on True and False branches.
+  ConditionID ID = 0, TrueID = 0, FalseID = 0;
+};
+
+} // namespace llvm::coverage::mcdc
+
+#endif // LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index eb0996e33b70dc..517a81d5740147 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -524,7 +524,7 @@ class MCDCDecisionRecorder {
 
     /// IDs that are stored in MCDCBranches
     /// Complete when all IDs (1 to NumConditions) are met.
-    DenseSet<CounterMappingRegion::MCDCConditionID> ConditionIDs;
+    DenseSet<mcdc::ConditionID> 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..fc6014c1f7be70 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -371,10 +371,9 @@ 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)},
+        mcdc::Parameters{static_cast<unsigned>(BIDX), static_cast<unsigned>(NC),
+                         static_cast<unsigned>(ID), static_cast<unsigned>(TID),
+                         static_cast<unsigned>(FID)},
         InferredFileID, ExpandedFileID, LineStart, ColumnStart,
         LineStart + NumLines, ColumnEnd, Kind);
     if (CMR.startLoc() > CMR.endLoc())
diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
index 2849781a9dc43b..d60312b68237bb 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -197,8 +197,7 @@ 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));
+        mcdc::Parameters{Mask, NC}, FileID, LS, CS, LE, CE));
   }
 
   void addMCDCBranchCMR(Counter C1, Counter C2, unsigned ID, unsigned TrueID,
@@ -207,8 +206,8 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
     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, mcdc::Parameters{0, 0, ID, TrueID, FalseID}, FileID, LS, CS, LE,
+        CE));
   }
 
   void addExpansionCMR(StringRef File, StringRef ExpandedFile, unsigned LS,

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 12, 2024

@llvm/pr-subscribers-clang

Author: NAKAMURA Takumi (chapuni)

Changes

They can be also used in clang.
Introduce the lightweight header instead of CoverageMapping.h.


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

6 Files Affected:

  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+26-27)
  • (modified) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+11-22)
  • (added) llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h (+36)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+1-1)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp (+3-4)
  • (modified) llvm/unittests/ProfileData/CoverageMappingTest.cpp (+3-4)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0c43317642bca4..888725dd2fc734 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -95,8 +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.
 class SourceMappingRegion {
@@ -107,7 +105,7 @@ class SourceMappingRegion {
   std::optional<Counter> FalseCount;
 
   /// Parameters used for Modified Condition/Decision Coverage
-  MCDCParameters MCDCParams;
+  mcdc::Parameters MCDCParams;
 
   /// The region's starting location.
   std::optional<SourceLocation> LocStart;
@@ -131,7 +129,7 @@ class SourceMappingRegion {
         SkippedRegion(false) {}
 
   SourceMappingRegion(Counter Count, std::optional<Counter> FalseCount,
-                      MCDCParameters MCDCParams,
+                      mcdc::Parameters MCDCParams,
                       std::optional<SourceLocation> LocStart,
                       std::optional<SourceLocation> LocEnd,
                       bool GapRegion = false)
@@ -139,7 +137,7 @@ class SourceMappingRegion {
         LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion),
         SkippedRegion(false) {}
 
-  SourceMappingRegion(MCDCParameters MCDCParams,
+  SourceMappingRegion(mcdc::Parameters MCDCParams,
                       std::optional<SourceLocation> LocStart,
                       std::optional<SourceLocation> LocEnd)
       : MCDCParams(MCDCParams), LocStart(LocStart), LocEnd(LocEnd),
@@ -187,7 +185,7 @@ class SourceMappingRegion {
 
   bool isMCDCDecision() const { return MCDCParams.NumConditions != 0; }
 
-  const MCDCParameters &getMCDCParams() const { return MCDCParams; }
+  const mcdc::Parameters &getMCDCParams() const { return MCDCParams; }
 };
 
 /// Spelling locations for the start and end of a source region.
@@ -587,8 +585,8 @@ struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder {
 struct MCDCCoverageBuilder {
 
   struct DecisionIDPair {
-    MCDCConditionID TrueID = 0;
-    MCDCConditionID FalseID = 0;
+    mcdc::ConditionID TrueID = 0;
+    mcdc::ConditionID FalseID = 0;
   };
 
   /// The AST walk recursively visits nested logical-AND or logical-OR binary
@@ -682,9 +680,9 @@ struct MCDCCoverageBuilder {
   CodeGenModule &CGM;
 
   llvm::SmallVector<DecisionIDPair> DecisionStack;
-  llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDs;
+  llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDs;
   llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap;
-  MCDCConditionID NextID = 1;
+  mcdc::ConditionID NextID = 1;
   bool NotMapped = false;
 
   /// Represent a sentinel value of [0,0] for the bottom of DecisionStack.
@@ -696,9 +694,10 @@ struct MCDCCoverageBuilder {
   }
 
 public:
-  MCDCCoverageBuilder(CodeGenModule &CGM,
-                      llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDMap,
-                      llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap)
+  MCDCCoverageBuilder(
+      CodeGenModule &CGM,
+      llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDMap,
+      llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap)
       : CGM(CGM), DecisionStack(1, DecisionStackSentinel), CondIDs(CondIDMap),
         MCDCBitmapMap(MCDCBitmapMap) {}
 
@@ -713,12 +712,12 @@ struct MCDCCoverageBuilder {
   bool isBuilding() const { return (NextID > 1); }
 
   /// Set the given condition's ID.
-  void setCondID(const Expr *Cond, MCDCConditionID ID) {
+  void setCondID(const Expr *Cond, mcdc::ConditionID ID) {
     CondIDs[CodeGenFunction::stripCond(Cond)] = ID;
   }
 
   /// Return the ID of a given condition.
-  MCDCConditionID getCondID(const Expr *Cond) const {
+  mcdc::ConditionID getCondID(const Expr *Cond) const {
     auto I = CondIDs.find(CodeGenFunction::stripCond(Cond));
     if (I == CondIDs.end())
       return 0;
@@ -755,7 +754,7 @@ struct MCDCCoverageBuilder {
       setCondID(E->getLHS(), NextID++);
 
     // Assign a ID+1 for the RHS.
-    MCDCConditionID RHSid = NextID++;
+    mcdc::ConditionID RHSid = NextID++;
     setCondID(E->getRHS(), RHSid);
 
     // Push the LHS decision IDs onto the DecisionStack.
@@ -865,8 +864,8 @@ 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) {
+                    mcdc::ConditionID ID = 0, mcdc::ConditionID TrueID = 0,
+                    mcdc::ConditionID FalseID = 0) {
 
     if (StartLoc && !FalseCount) {
       MostRecentLocation = *StartLoc;
@@ -886,7 +885,7 @@ struct CounterCoverageMappingBuilder
     if (EndLoc && EndLoc->isInvalid())
       EndLoc = std::nullopt;
     RegionStack.emplace_back(Count, FalseCount,
-                             MCDCParameters{0, 0, ID, TrueID, FalseID},
+                             mcdc::Parameters{0, 0, ID, TrueID, FalseID},
                              StartLoc, EndLoc);
 
     return RegionStack.size() - 1;
@@ -896,7 +895,7 @@ struct CounterCoverageMappingBuilder
                     std::optional<SourceLocation> StartLoc = std::nullopt,
                     std::optional<SourceLocation> EndLoc = std::nullopt) {
 
-    RegionStack.emplace_back(MCDCParameters{BitmapIdx, Conditions}, StartLoc,
+    RegionStack.emplace_back(mcdc::Parameters{BitmapIdx, Conditions}, StartLoc,
                              EndLoc);
 
     return RegionStack.size() - 1;
@@ -1042,9 +1041,9 @@ struct CounterCoverageMappingBuilder
     // function's SourceRegions) because it doesn't apply to any other source
     // code other than the Condition.
     if (CodeGenFunction::isInstrumentedCondition(C)) {
-      MCDCConditionID ID = MCDCBuilder.getCondID(C);
-      MCDCConditionID TrueID = IDPair.TrueID;
-      MCDCConditionID FalseID = IDPair.FalseID;
+      mcdc::ConditionID ID = MCDCBuilder.getCondID(C);
+      mcdc::ConditionID TrueID = IDPair.TrueID;
+      mcdc::ConditionID 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
@@ -1151,9 +1150,9 @@ struct CounterCoverageMappingBuilder
           if (I.isBranch())
             SourceRegions.emplace_back(
                 I.getCounter(), I.getFalseCounter(),
-                MCDCParameters{0, 0, I.getMCDCParams().ID,
-                               I.getMCDCParams().TrueID,
-                               I.getMCDCParams().FalseID},
+                mcdc::Parameters{0, 0, I.getMCDCParams().ID,
+                                 I.getMCDCParams().TrueID,
+                                 I.getMCDCParams().FalseID},
                 Loc, getEndOfFileOrMacro(Loc), I.isBranch());
           else
             SourceRegions.emplace_back(I.getCounter(), Loc,
@@ -1338,7 +1337,7 @@ struct CounterCoverageMappingBuilder
       CoverageMappingModuleGen &CVM,
       llvm::DenseMap<const Stmt *, unsigned> &CounterMap,
       llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap,
-      llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDMap,
+      llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDMap,
       SourceManager &SM, const LangOptions &LangOpts)
       : CoverageMappingBuilder(CVM, SM, LangOpts), CounterMap(CounterMap),
         MCDCBitmapMap(MCDCBitmapMap),
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 88ec60c7aa33c6..2869ae8fe861cc 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -23,6 +23,7 @@
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Object/BuildID.h"
+#include "llvm/ProfileData/Coverage/MCDCTypes.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Alignment.h"
 #include "llvm/Support/Compiler.h"
@@ -249,19 +250,6 @@ struct CounterMappingRegion {
     MCDCBranchRegion
   };
 
-  using MCDCConditionID = unsigned int;
-  struct MCDCParameters {
-    /// Byte Index of Bitmap Coverage Object for a Decision Region.
-    unsigned BitmapIdx = 0;
-
-    /// Number of Conditions used for a Decision Region.
-    unsigned NumConditions = 0;
-
-    /// 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;
-  };
-
   /// Primary Counter that is also used for Branch Regions (TrueCount).
   Counter Count;
 
@@ -269,7 +257,7 @@ struct CounterMappingRegion {
   Counter FalseCount;
 
   /// Parameters used for Modified Condition/Decision Coverage
-  MCDCParameters MCDCParams;
+  mcdc::Parameters MCDCParams;
 
   unsigned FileID = 0;
   unsigned ExpandedFileID = 0;
@@ -285,7 +273,7 @@ struct CounterMappingRegion {
         ColumnEnd(ColumnEnd), Kind(Kind) {}
 
   CounterMappingRegion(Counter Count, Counter FalseCount,
-                       MCDCParameters MCDCParams, unsigned FileID,
+                       mcdc::Parameters MCDCParams, unsigned FileID,
                        unsigned ExpandedFileID, unsigned LineStart,
                        unsigned ColumnStart, unsigned LineEnd,
                        unsigned ColumnEnd, RegionKind Kind)
@@ -294,7 +282,7 @@ struct CounterMappingRegion {
         ColumnStart(ColumnStart), LineEnd(LineEnd), ColumnEnd(ColumnEnd),
         Kind(Kind) {}
 
-  CounterMappingRegion(MCDCParameters MCDCParams, unsigned FileID,
+  CounterMappingRegion(mcdc::Parameters MCDCParams, unsigned FileID,
                        unsigned LineStart, unsigned ColumnStart,
                        unsigned LineEnd, unsigned ColumnEnd, RegionKind Kind)
       : MCDCParams(MCDCParams), FileID(FileID), LineStart(LineStart),
@@ -334,15 +322,16 @@ struct CounterMappingRegion {
   makeBranchRegion(Counter Count, Counter FalseCount, unsigned FileID,
                    unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
                    unsigned ColumnEnd) {
-    return CounterMappingRegion(Count, FalseCount, MCDCParameters(), FileID, 0,
-                                LineStart, ColumnStart, LineEnd, ColumnEnd,
+    return CounterMappingRegion(Count, FalseCount, mcdc::Parameters(), FileID,
+                                0, LineStart, ColumnStart, LineEnd, ColumnEnd,
                                 BranchRegion);
   }
 
   static CounterMappingRegion
-  makeBranchRegion(Counter Count, Counter FalseCount, MCDCParameters MCDCParams,
-                   unsigned FileID, unsigned LineStart, unsigned ColumnStart,
-                   unsigned LineEnd, unsigned ColumnEnd) {
+  makeBranchRegion(Counter Count, Counter FalseCount,
+                   mcdc::Parameters MCDCParams, unsigned FileID,
+                   unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
+                   unsigned ColumnEnd) {
     return CounterMappingRegion(Count, FalseCount, MCDCParams, FileID, 0,
                                 LineStart, ColumnStart, LineEnd, ColumnEnd,
                                 MCDCParams.ID == 0 ? BranchRegion
@@ -350,7 +339,7 @@ struct CounterMappingRegion {
   }
 
   static CounterMappingRegion
-  makeDecisionRegion(MCDCParameters MCDCParams, unsigned FileID,
+  makeDecisionRegion(mcdc::Parameters MCDCParams, unsigned FileID,
                      unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
                      unsigned ColumnEnd) {
     return CounterMappingRegion(MCDCParams, FileID, LineStart, ColumnStart,
diff --git a/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
new file mode 100644
index 00000000000000..0d3a79c4674eb2
--- /dev/null
+++ b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
@@ -0,0 +1,36 @@
+//===- CoverageMapping.h - Code coverage mapping support --------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Types related to MC/DC Coverage.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
+#define LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
+
+namespace llvm::coverage::mcdc {
+
+/// The ID for MCDCBranch.
+using ConditionID = unsigned int;
+
+/// MC/DC-specifig parameters
+struct Parameters {
+  /// Byte Index of Bitmap Coverage Object for a Decision Region.
+  unsigned BitmapIdx = 0;
+
+  /// Number of Conditions used for a Decision Region.
+  unsigned NumConditions = 0;
+
+  /// IDs used to represent a branch region and other branch regions
+  /// evaluated based on True and False branches.
+  ConditionID ID = 0, TrueID = 0, FalseID = 0;
+};
+
+} // namespace llvm::coverage::mcdc
+
+#endif // LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index eb0996e33b70dc..517a81d5740147 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -524,7 +524,7 @@ class MCDCDecisionRecorder {
 
     /// IDs that are stored in MCDCBranches
     /// Complete when all IDs (1 to NumConditions) are met.
-    DenseSet<CounterMappingRegion::MCDCConditionID> ConditionIDs;
+    DenseSet<mcdc::ConditionID> 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..fc6014c1f7be70 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -371,10 +371,9 @@ 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)},
+        mcdc::Parameters{static_cast<unsigned>(BIDX), static_cast<unsigned>(NC),
+                         static_cast<unsigned>(ID), static_cast<unsigned>(TID),
+                         static_cast<unsigned>(FID)},
         InferredFileID, ExpandedFileID, LineStart, ColumnStart,
         LineStart + NumLines, ColumnEnd, Kind);
     if (CMR.startLoc() > CMR.endLoc())
diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
index 2849781a9dc43b..d60312b68237bb 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -197,8 +197,7 @@ 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));
+        mcdc::Parameters{Mask, NC}, FileID, LS, CS, LE, CE));
   }
 
   void addMCDCBranchCMR(Counter C1, Counter C2, unsigned ID, unsigned TrueID,
@@ -207,8 +206,8 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
     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, mcdc::Parameters{0, 0, ID, TrueID, FalseID}, FileID, LS, CS, LE,
+        CE));
   }
 
   void addExpansionCMR(StringRef File, StringRef ExpandedFile, unsigned LS,

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 12, 2024

@llvm/pr-subscribers-clang-codegen

Author: NAKAMURA Takumi (chapuni)

Changes

They can be also used in clang.
Introduce the lightweight header instead of CoverageMapping.h.


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

6 Files Affected:

  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+26-27)
  • (modified) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+11-22)
  • (added) llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h (+36)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+1-1)
  • (modified) llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp (+3-4)
  • (modified) llvm/unittests/ProfileData/CoverageMappingTest.cpp (+3-4)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0c43317642bca4..888725dd2fc734 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -95,8 +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.
 class SourceMappingRegion {
@@ -107,7 +105,7 @@ class SourceMappingRegion {
   std::optional<Counter> FalseCount;
 
   /// Parameters used for Modified Condition/Decision Coverage
-  MCDCParameters MCDCParams;
+  mcdc::Parameters MCDCParams;
 
   /// The region's starting location.
   std::optional<SourceLocation> LocStart;
@@ -131,7 +129,7 @@ class SourceMappingRegion {
         SkippedRegion(false) {}
 
   SourceMappingRegion(Counter Count, std::optional<Counter> FalseCount,
-                      MCDCParameters MCDCParams,
+                      mcdc::Parameters MCDCParams,
                       std::optional<SourceLocation> LocStart,
                       std::optional<SourceLocation> LocEnd,
                       bool GapRegion = false)
@@ -139,7 +137,7 @@ class SourceMappingRegion {
         LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion),
         SkippedRegion(false) {}
 
-  SourceMappingRegion(MCDCParameters MCDCParams,
+  SourceMappingRegion(mcdc::Parameters MCDCParams,
                       std::optional<SourceLocation> LocStart,
                       std::optional<SourceLocation> LocEnd)
       : MCDCParams(MCDCParams), LocStart(LocStart), LocEnd(LocEnd),
@@ -187,7 +185,7 @@ class SourceMappingRegion {
 
   bool isMCDCDecision() const { return MCDCParams.NumConditions != 0; }
 
-  const MCDCParameters &getMCDCParams() const { return MCDCParams; }
+  const mcdc::Parameters &getMCDCParams() const { return MCDCParams; }
 };
 
 /// Spelling locations for the start and end of a source region.
@@ -587,8 +585,8 @@ struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder {
 struct MCDCCoverageBuilder {
 
   struct DecisionIDPair {
-    MCDCConditionID TrueID = 0;
-    MCDCConditionID FalseID = 0;
+    mcdc::ConditionID TrueID = 0;
+    mcdc::ConditionID FalseID = 0;
   };
 
   /// The AST walk recursively visits nested logical-AND or logical-OR binary
@@ -682,9 +680,9 @@ struct MCDCCoverageBuilder {
   CodeGenModule &CGM;
 
   llvm::SmallVector<DecisionIDPair> DecisionStack;
-  llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDs;
+  llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDs;
   llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap;
-  MCDCConditionID NextID = 1;
+  mcdc::ConditionID NextID = 1;
   bool NotMapped = false;
 
   /// Represent a sentinel value of [0,0] for the bottom of DecisionStack.
@@ -696,9 +694,10 @@ struct MCDCCoverageBuilder {
   }
 
 public:
-  MCDCCoverageBuilder(CodeGenModule &CGM,
-                      llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDMap,
-                      llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap)
+  MCDCCoverageBuilder(
+      CodeGenModule &CGM,
+      llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDMap,
+      llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap)
       : CGM(CGM), DecisionStack(1, DecisionStackSentinel), CondIDs(CondIDMap),
         MCDCBitmapMap(MCDCBitmapMap) {}
 
@@ -713,12 +712,12 @@ struct MCDCCoverageBuilder {
   bool isBuilding() const { return (NextID > 1); }
 
   /// Set the given condition's ID.
-  void setCondID(const Expr *Cond, MCDCConditionID ID) {
+  void setCondID(const Expr *Cond, mcdc::ConditionID ID) {
     CondIDs[CodeGenFunction::stripCond(Cond)] = ID;
   }
 
   /// Return the ID of a given condition.
-  MCDCConditionID getCondID(const Expr *Cond) const {
+  mcdc::ConditionID getCondID(const Expr *Cond) const {
     auto I = CondIDs.find(CodeGenFunction::stripCond(Cond));
     if (I == CondIDs.end())
       return 0;
@@ -755,7 +754,7 @@ struct MCDCCoverageBuilder {
       setCondID(E->getLHS(), NextID++);
 
     // Assign a ID+1 for the RHS.
-    MCDCConditionID RHSid = NextID++;
+    mcdc::ConditionID RHSid = NextID++;
     setCondID(E->getRHS(), RHSid);
 
     // Push the LHS decision IDs onto the DecisionStack.
@@ -865,8 +864,8 @@ 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) {
+                    mcdc::ConditionID ID = 0, mcdc::ConditionID TrueID = 0,
+                    mcdc::ConditionID FalseID = 0) {
 
     if (StartLoc && !FalseCount) {
       MostRecentLocation = *StartLoc;
@@ -886,7 +885,7 @@ struct CounterCoverageMappingBuilder
     if (EndLoc && EndLoc->isInvalid())
       EndLoc = std::nullopt;
     RegionStack.emplace_back(Count, FalseCount,
-                             MCDCParameters{0, 0, ID, TrueID, FalseID},
+                             mcdc::Parameters{0, 0, ID, TrueID, FalseID},
                              StartLoc, EndLoc);
 
     return RegionStack.size() - 1;
@@ -896,7 +895,7 @@ struct CounterCoverageMappingBuilder
                     std::optional<SourceLocation> StartLoc = std::nullopt,
                     std::optional<SourceLocation> EndLoc = std::nullopt) {
 
-    RegionStack.emplace_back(MCDCParameters{BitmapIdx, Conditions}, StartLoc,
+    RegionStack.emplace_back(mcdc::Parameters{BitmapIdx, Conditions}, StartLoc,
                              EndLoc);
 
     return RegionStack.size() - 1;
@@ -1042,9 +1041,9 @@ struct CounterCoverageMappingBuilder
     // function's SourceRegions) because it doesn't apply to any other source
     // code other than the Condition.
     if (CodeGenFunction::isInstrumentedCondition(C)) {
-      MCDCConditionID ID = MCDCBuilder.getCondID(C);
-      MCDCConditionID TrueID = IDPair.TrueID;
-      MCDCConditionID FalseID = IDPair.FalseID;
+      mcdc::ConditionID ID = MCDCBuilder.getCondID(C);
+      mcdc::ConditionID TrueID = IDPair.TrueID;
+      mcdc::ConditionID 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
@@ -1151,9 +1150,9 @@ struct CounterCoverageMappingBuilder
           if (I.isBranch())
             SourceRegions.emplace_back(
                 I.getCounter(), I.getFalseCounter(),
-                MCDCParameters{0, 0, I.getMCDCParams().ID,
-                               I.getMCDCParams().TrueID,
-                               I.getMCDCParams().FalseID},
+                mcdc::Parameters{0, 0, I.getMCDCParams().ID,
+                                 I.getMCDCParams().TrueID,
+                                 I.getMCDCParams().FalseID},
                 Loc, getEndOfFileOrMacro(Loc), I.isBranch());
           else
             SourceRegions.emplace_back(I.getCounter(), Loc,
@@ -1338,7 +1337,7 @@ struct CounterCoverageMappingBuilder
       CoverageMappingModuleGen &CVM,
       llvm::DenseMap<const Stmt *, unsigned> &CounterMap,
       llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap,
-      llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDMap,
+      llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDMap,
       SourceManager &SM, const LangOptions &LangOpts)
       : CoverageMappingBuilder(CVM, SM, LangOpts), CounterMap(CounterMap),
         MCDCBitmapMap(MCDCBitmapMap),
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 88ec60c7aa33c6..2869ae8fe861cc 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -23,6 +23,7 @@
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Object/BuildID.h"
+#include "llvm/ProfileData/Coverage/MCDCTypes.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Alignment.h"
 #include "llvm/Support/Compiler.h"
@@ -249,19 +250,6 @@ struct CounterMappingRegion {
     MCDCBranchRegion
   };
 
-  using MCDCConditionID = unsigned int;
-  struct MCDCParameters {
-    /// Byte Index of Bitmap Coverage Object for a Decision Region.
-    unsigned BitmapIdx = 0;
-
-    /// Number of Conditions used for a Decision Region.
-    unsigned NumConditions = 0;
-
-    /// 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;
-  };
-
   /// Primary Counter that is also used for Branch Regions (TrueCount).
   Counter Count;
 
@@ -269,7 +257,7 @@ struct CounterMappingRegion {
   Counter FalseCount;
 
   /// Parameters used for Modified Condition/Decision Coverage
-  MCDCParameters MCDCParams;
+  mcdc::Parameters MCDCParams;
 
   unsigned FileID = 0;
   unsigned ExpandedFileID = 0;
@@ -285,7 +273,7 @@ struct CounterMappingRegion {
         ColumnEnd(ColumnEnd), Kind(Kind) {}
 
   CounterMappingRegion(Counter Count, Counter FalseCount,
-                       MCDCParameters MCDCParams, unsigned FileID,
+                       mcdc::Parameters MCDCParams, unsigned FileID,
                        unsigned ExpandedFileID, unsigned LineStart,
                        unsigned ColumnStart, unsigned LineEnd,
                        unsigned ColumnEnd, RegionKind Kind)
@@ -294,7 +282,7 @@ struct CounterMappingRegion {
         ColumnStart(ColumnStart), LineEnd(LineEnd), ColumnEnd(ColumnEnd),
         Kind(Kind) {}
 
-  CounterMappingRegion(MCDCParameters MCDCParams, unsigned FileID,
+  CounterMappingRegion(mcdc::Parameters MCDCParams, unsigned FileID,
                        unsigned LineStart, unsigned ColumnStart,
                        unsigned LineEnd, unsigned ColumnEnd, RegionKind Kind)
       : MCDCParams(MCDCParams), FileID(FileID), LineStart(LineStart),
@@ -334,15 +322,16 @@ struct CounterMappingRegion {
   makeBranchRegion(Counter Count, Counter FalseCount, unsigned FileID,
                    unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
                    unsigned ColumnEnd) {
-    return CounterMappingRegion(Count, FalseCount, MCDCParameters(), FileID, 0,
-                                LineStart, ColumnStart, LineEnd, ColumnEnd,
+    return CounterMappingRegion(Count, FalseCount, mcdc::Parameters(), FileID,
+                                0, LineStart, ColumnStart, LineEnd, ColumnEnd,
                                 BranchRegion);
   }
 
   static CounterMappingRegion
-  makeBranchRegion(Counter Count, Counter FalseCount, MCDCParameters MCDCParams,
-                   unsigned FileID, unsigned LineStart, unsigned ColumnStart,
-                   unsigned LineEnd, unsigned ColumnEnd) {
+  makeBranchRegion(Counter Count, Counter FalseCount,
+                   mcdc::Parameters MCDCParams, unsigned FileID,
+                   unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
+                   unsigned ColumnEnd) {
     return CounterMappingRegion(Count, FalseCount, MCDCParams, FileID, 0,
                                 LineStart, ColumnStart, LineEnd, ColumnEnd,
                                 MCDCParams.ID == 0 ? BranchRegion
@@ -350,7 +339,7 @@ struct CounterMappingRegion {
   }
 
   static CounterMappingRegion
-  makeDecisionRegion(MCDCParameters MCDCParams, unsigned FileID,
+  makeDecisionRegion(mcdc::Parameters MCDCParams, unsigned FileID,
                      unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
                      unsigned ColumnEnd) {
     return CounterMappingRegion(MCDCParams, FileID, LineStart, ColumnStart,
diff --git a/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
new file mode 100644
index 00000000000000..0d3a79c4674eb2
--- /dev/null
+++ b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
@@ -0,0 +1,36 @@
+//===- CoverageMapping.h - Code coverage mapping support --------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Types related to MC/DC Coverage.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
+#define LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
+
+namespace llvm::coverage::mcdc {
+
+/// The ID for MCDCBranch.
+using ConditionID = unsigned int;
+
+/// MC/DC-specifig parameters
+struct Parameters {
+  /// Byte Index of Bitmap Coverage Object for a Decision Region.
+  unsigned BitmapIdx = 0;
+
+  /// Number of Conditions used for a Decision Region.
+  unsigned NumConditions = 0;
+
+  /// IDs used to represent a branch region and other branch regions
+  /// evaluated based on True and False branches.
+  ConditionID ID = 0, TrueID = 0, FalseID = 0;
+};
+
+} // namespace llvm::coverage::mcdc
+
+#endif // LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index eb0996e33b70dc..517a81d5740147 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -524,7 +524,7 @@ class MCDCDecisionRecorder {
 
     /// IDs that are stored in MCDCBranches
     /// Complete when all IDs (1 to NumConditions) are met.
-    DenseSet<CounterMappingRegion::MCDCConditionID> ConditionIDs;
+    DenseSet<mcdc::ConditionID> 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..fc6014c1f7be70 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -371,10 +371,9 @@ 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)},
+        mcdc::Parameters{static_cast<unsigned>(BIDX), static_cast<unsigned>(NC),
+                         static_cast<unsigned>(ID), static_cast<unsigned>(TID),
+                         static_cast<unsigned>(FID)},
         InferredFileID, ExpandedFileID, LineStart, ColumnStart,
         LineStart + NumLines, ColumnEnd, Kind);
     if (CMR.startLoc() > CMR.endLoc())
diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
index 2849781a9dc43b..d60312b68237bb 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -197,8 +197,7 @@ 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));
+        mcdc::Parameters{Mask, NC}, FileID, LS, CS, LE, CE));
   }
 
   void addMCDCBranchCMR(Counter C1, Counter C2, unsigned ID, unsigned TrueID,
@@ -207,8 +206,8 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
     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, mcdc::Parameters{0, 0, ID, TrueID, FalseID}, FileID, LS, CS, LE,
+        CE));
   }
 
   void addExpansionCMR(StringRef File, StringRef ExpandedFile, unsigned LS,

Copy link

@ornata ornata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chapuni chapuni merged commit f0db35b into llvm:main Feb 13, 2024
3 of 4 checks passed
@chapuni chapuni deleted the mcdc/refactor/ns branch February 13, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants