[clang] [llvm] [MC/DC] Refactor: Introduce `MCDCTypes.h` for `coverage::mcdc` (PR #81459)
NAKAMURA Takumi via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 12 02:13:56 PST 2024
https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/81459
>From 03bab9acf5aa3bf13c2223e92dbad040202d34bd Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Mon, 12 Feb 2024 18:43:56 +0900
Subject: [PATCH 1/2] [MC/DC] Refactor: Introduce `MCDCTypes.h` for the
namespace `coverage::mcdc`
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 53 +++++++++----------
.../ProfileData/Coverage/CoverageMapping.h | 33 ++++--------
.../llvm/ProfileData/Coverage/MCDCTypes.h | 36 +++++++++++++
.../ProfileData/Coverage/CoverageMapping.cpp | 2 +-
.../Coverage/CoverageMappingReader.cpp | 7 ++-
.../ProfileData/CoverageMappingTest.cpp | 7 ++-
6 files changed, 80 insertions(+), 58 deletions(-)
create mode 100644 llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
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,
>From f26534bfeb9dc064ac90c00c5b9d4be6ef4efe7b Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Mon, 12 Feb 2024 19:12:24 +0900
Subject: [PATCH 2/2] Fix the header
---
llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
index 0d3a79c4674eb2..365c899b6eb520 100644
--- a/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
+++ b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
@@ -1,4 +1,4 @@
-//===- CoverageMapping.h - Code coverage mapping support --------*- C++ -*-===//
+//===- MCDCTypes.h - Types related to MC/DC Coverage ------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
More information about the cfe-commits
mailing list