[clang] [llvm] [MC/DC] Refactor: Let MCDCConditionID int16_t with zero-origin (PR #81257)
NAKAMURA Takumi via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 13 16:28:54 PST 2024
https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/81257
>From b2e8b3eaa067844a5fa5643aca17dbb0f237182e Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sat, 10 Feb 2024 00:08:36 +0900
Subject: [PATCH 1/2] [MC/DC] Refactor: Let MCDCConditionID int16_t with
zero-origin
Also, Let NumConditions uint16_t
---
clang/lib/CodeGen/CodeGenPGO.cpp | 8 ++---
clang/lib/CodeGen/CodeGenPGO.h | 2 +-
clang/lib/CodeGen/CoverageMappingGen.cpp | 30 +++++++++----------
clang/lib/CodeGen/CoverageMappingGen.h | 4 +--
.../ProfileData/Coverage/CoverageMapping.h | 10 +++----
.../ProfileData/Coverage/CoverageMapping.cpp | 25 ++++++++--------
.../Coverage/CoverageMappingReader.cpp | 17 ++++++-----
.../Coverage/CoverageMappingWriter.cpp | 6 ++--
.../ProfileData/CoverageMappingTest.cpp | 18 +++++------
9 files changed, 61 insertions(+), 59 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 5d7c3847745762..9025889f443b88 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -1033,7 +1033,7 @@ void CodeGenPGO::emitCounterRegionMapping(const Decl *D) {
std::string CoverageMapping;
llvm::raw_string_ostream OS(CoverageMapping);
- RegionCondIDMap.reset(new llvm::DenseMap<const Stmt *, unsigned>);
+ RegionCondIDMap.reset(new llvm::DenseMap<const Stmt *, int16_t>);
CoverageMappingGen MappingGen(
*CGM.getCoverageMapping(), CGM.getContext().getSourceManager(),
CGM.getLangOpts(), RegionCounterMap.get(), RegionMCDCBitmapMap.get(),
@@ -1198,8 +1198,8 @@ void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy &Builder, const Expr *S,
return;
// Extract the ID of the condition we are setting in the bitmap.
- unsigned CondID = ExprMCDCConditionIDMapIterator->second;
- assert(CondID > 0 && "Condition has no ID!");
+ auto CondID = ExprMCDCConditionIDMapIterator->second;
+ assert(CondID >= 0 && "Condition has no ID!");
auto *I8PtrTy = llvm::PointerType::getUnqual(CGM.getLLVMContext());
@@ -1208,7 +1208,7 @@ void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy &Builder, const Expr *S,
// the resulting value is used to update the boolean expression's bitmap.
llvm::Value *Args[5] = {llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy),
Builder.getInt64(FunctionHash),
- Builder.getInt32(CondID - 1),
+ Builder.getInt32(CondID),
MCDCCondBitmapAddr.getPointer(), Val};
Builder.CreateCall(
CGM.getIntrinsic(llvm::Intrinsic::instrprof_mcdc_condbitmap_update),
diff --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h
index 6596b6c3527764..5f2941cfb2e95e 100644
--- a/clang/lib/CodeGen/CodeGenPGO.h
+++ b/clang/lib/CodeGen/CodeGenPGO.h
@@ -37,7 +37,7 @@ class CodeGenPGO {
uint64_t FunctionHash;
std::unique_ptr<llvm::DenseMap<const Stmt *, unsigned>> RegionCounterMap;
std::unique_ptr<llvm::DenseMap<const Stmt *, unsigned>> RegionMCDCBitmapMap;
- std::unique_ptr<llvm::DenseMap<const Stmt *, unsigned>> RegionCondIDMap;
+ std::unique_ptr<llvm::DenseMap<const Stmt *, int16_t>> RegionCondIDMap;
std::unique_ptr<llvm::DenseMap<const Stmt *, uint64_t>> StmtCountMap;
std::unique_ptr<llvm::InstrProfRecord> ProfRecord;
std::vector<uint64_t> RegionCounts;
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0c43317642bca4..d918acd951dd8c 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -587,8 +587,8 @@ struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder {
struct MCDCCoverageBuilder {
struct DecisionIDPair {
- MCDCConditionID TrueID = 0;
- MCDCConditionID FalseID = 0;
+ MCDCConditionID TrueID = -1;
+ MCDCConditionID FalseID = -1;
};
/// The AST walk recursively visits nested logical-AND or logical-OR binary
@@ -684,11 +684,11 @@ struct MCDCCoverageBuilder {
llvm::SmallVector<DecisionIDPair> DecisionStack;
llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDs;
llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap;
- MCDCConditionID NextID = 1;
+ MCDCConditionID NextID = 0;
bool NotMapped = false;
/// Represent a sentinel value of [0,0] for the bottom of DecisionStack.
- static constexpr DecisionIDPair DecisionStackSentinel{0, 0};
+ static constexpr DecisionIDPair DecisionStackSentinel{-1, -1};
/// Is this a logical-AND operation?
bool isLAnd(const BinaryOperator *E) const {
@@ -705,12 +705,12 @@ struct MCDCCoverageBuilder {
/// Return whether the build of the control flow map is at the top-level
/// (root) of a logical operator nest in a boolean expression prior to the
/// assignment of condition IDs.
- bool isIdle() const { return (NextID == 1 && !NotMapped); }
+ bool isIdle() const { return (NextID == 0 && !NotMapped); }
/// Return whether any IDs have been assigned in the build of the control
/// flow map, indicating that the map is being generated for this boolean
/// expression.
- bool isBuilding() const { return (NextID > 1); }
+ bool isBuilding() const { return (NextID > 0); }
/// Set the given condition's ID.
void setCondID(const Expr *Cond, MCDCConditionID ID) {
@@ -721,7 +721,7 @@ struct MCDCCoverageBuilder {
MCDCConditionID getCondID(const Expr *Cond) const {
auto I = CondIDs.find(CodeGenFunction::stripCond(Cond));
if (I == CondIDs.end())
- return 0;
+ return -1;
else
return I->second;
}
@@ -788,15 +788,15 @@ struct MCDCCoverageBuilder {
// Reset state if not doing mapping.
if (NotMapped) {
NotMapped = false;
- assert(NextID == 1);
+ assert(NextID == 0);
return 0;
}
// Set number of conditions and reset.
- unsigned TotalConds = NextID - 1;
+ unsigned TotalConds = NextID;
// Reset ID back to beginning.
- NextID = 1;
+ NextID = 0;
return TotalConds;
}
@@ -865,8 +865,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) {
+ MCDCConditionID ID = -1, MCDCConditionID TrueID = -1,
+ MCDCConditionID FalseID = -1) {
if (StartLoc && !FalseCount) {
MostRecentLocation = *StartLoc;
@@ -892,7 +892,7 @@ struct CounterCoverageMappingBuilder
return RegionStack.size() - 1;
}
- size_t pushRegion(unsigned BitmapIdx, unsigned Conditions,
+ size_t pushRegion(unsigned BitmapIdx, uint16_t Conditions,
std::optional<SourceLocation> StartLoc = std::nullopt,
std::optional<SourceLocation> EndLoc = std::nullopt) {
@@ -2134,8 +2134,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 + 1 << "," << R.MCDCParams.TrueID + 1;
+ OS << "," << R.MCDCParams.FalseID + 1 << "] ";
}
if (R.Kind == CounterMappingRegion::ExpansionRegion)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.h b/clang/lib/CodeGen/CoverageMappingGen.h
index 62cea173c9fc93..915099f8331923 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.h
+++ b/clang/lib/CodeGen/CoverageMappingGen.h
@@ -151,7 +151,7 @@ class CoverageMappingGen {
const LangOptions &LangOpts;
llvm::DenseMap<const Stmt *, unsigned> *CounterMap;
llvm::DenseMap<const Stmt *, unsigned> *MCDCBitmapMap;
- llvm::DenseMap<const Stmt *, unsigned> *CondIDMap;
+ llvm::DenseMap<const Stmt *, int16_t> *CondIDMap;
public:
CoverageMappingGen(CoverageMappingModuleGen &CVM, SourceManager &SM,
@@ -163,7 +163,7 @@ class CoverageMappingGen {
const LangOptions &LangOpts,
llvm::DenseMap<const Stmt *, unsigned> *CounterMap,
llvm::DenseMap<const Stmt *, unsigned> *MCDCBitmapMap,
- llvm::DenseMap<const Stmt *, unsigned> *CondIDMap)
+ llvm::DenseMap<const Stmt *, int16_t> *CondIDMap)
: CVM(CVM), SM(SM), LangOpts(LangOpts), CounterMap(CounterMap),
MCDCBitmapMap(MCDCBitmapMap), CondIDMap(CondIDMap) {}
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 88ec60c7aa33c6..959edea1bbaedd 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -249,17 +249,17 @@ struct CounterMappingRegion {
MCDCBranchRegion
};
- using MCDCConditionID = unsigned int;
+ using MCDCConditionID = int16_t;
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;
+ uint16_t 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;
+ MCDCConditionID ID = -1, TrueID = -1, FalseID = -1;
};
/// Primary Counter that is also used for Branch Regions (TrueCount).
@@ -345,8 +345,8 @@ struct CounterMappingRegion {
unsigned LineEnd, unsigned ColumnEnd) {
return CounterMappingRegion(Count, FalseCount, MCDCParams, FileID, 0,
LineStart, ColumnStart, LineEnd, ColumnEnd,
- MCDCParams.ID == 0 ? BranchRegion
- : MCDCBranchRegion);
+ MCDCParams.ID < 0 ? BranchRegion
+ : MCDCBranchRegion);
}
static CounterMappingRegion
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index eb0996e33b70dc..5b631bd7c27c2a 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -283,25 +283,26 @@ class MCDCRecordProcessor {
// 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.
- void buildTestVector(MCDCRecord::TestVector &TV, unsigned ID,
+ void buildTestVector(MCDCRecord::TestVector &TV,
+ CounterMappingRegion::MCDCConditionID ID,
unsigned Index) {
const CounterMappingRegion *Branch = Map[ID];
- TV[ID - 1] = MCDCRecord::MCDC_False;
- if (Branch->MCDCParams.FalseID > 0)
+ TV[ID] = 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)
+ Index |= 1 << ID;
+ TV[ID] = MCDCRecord::MCDC_True;
+ if (Branch->MCDCParams.TrueID >= 0)
buildTestVector(TV, Branch->MCDCParams.TrueID, Index);
else
recordTestVector(TV, Index, MCDCRecord::MCDC_True);
// Reset back to DontCare.
- TV[ID - 1] = MCDCRecord::MCDC_DontCare;
+ TV[ID] = MCDCRecord::MCDC_DontCare;
}
/// Walk the bits in the bitmap. A bit set to '1' indicates that the test
@@ -311,7 +312,7 @@ class MCDCRecordProcessor {
// We start at the root node (ID == 1) with all values being DontCare.
// `Index` encodes the bitmask of true values and is initially 0.
MCDCRecord::TestVector TV(NumConditions, MCDCRecord::MCDC_DontCare);
- buildTestVector(TV, 1, 0);
+ buildTestVector(TV, 0, 0);
}
// Find an independence pair for each condition:
@@ -372,7 +373,7 @@ class MCDCRecordProcessor {
// from being measured.
for (const auto *B : Branches) {
Map[B->MCDCParams.ID] = B;
- PosToID[I] = B->MCDCParams.ID - 1;
+ PosToID[I] = B->MCDCParams.ID;
CondLoc[I] = B->startLoc();
Folded[I++] = (B->Count.isZero() && B->FalseCount.isZero());
}
@@ -562,10 +563,10 @@ class MCDCDecisionRecorder {
assert(Branch.Kind == CounterMappingRegion::MCDCBranchRegion);
auto ConditionID = Branch.MCDCParams.ID;
- assert(ConditionID > 0 && "ConditionID should begin with 1");
+ assert(ConditionID >= 0 && "ConditionID should be positive");
if (ConditionIDs.contains(ConditionID) ||
- ConditionID > DecisionRegion->MCDCParams.NumConditions)
+ ConditionID >= DecisionRegion->MCDCParams.NumConditions)
return NotProcessed;
if (!this->dominates(Branch))
@@ -575,7 +576,7 @@ class MCDCDecisionRecorder {
// Put `ID=1` in front of `MCDCBranches` for convenience
// even if `MCDCBranches` is not topological.
- if (ConditionID == 1)
+ if (ConditionID == 0)
MCDCBranches.insert(MCDCBranches.begin(), &Branch);
else
MCDCBranches.push_back(&Branch);
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
index ac8e6b56379f21..df12d2fec3050a 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -244,7 +244,7 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray(
unsigned LineStart = 0;
for (size_t I = 0; I < NumRegions; ++I) {
Counter C, C2;
- uint64_t BIDX = 0, NC = 0, ID = 0, TID = 0, FID = 0;
+ uint64_t BIDX = 0, NC = 0, ID1 = 0, TID1 = 0, FID1 = 0;
CounterMappingRegion::RegionKind Kind = CounterMappingRegion::CodeRegion;
// Read the combined counter + region kind.
@@ -302,18 +302,18 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray(
return Err;
if (auto Err = readCounter(C2))
return Err;
- if (auto Err = readIntMax(ID, std::numeric_limits<unsigned>::max()))
+ if (auto Err = readIntMax(ID1, std::numeric_limits<int16_t>::max()))
return Err;
- if (auto Err = readIntMax(TID, std::numeric_limits<unsigned>::max()))
+ if (auto Err = readIntMax(TID1, std::numeric_limits<int16_t>::max()))
return Err;
- if (auto Err = readIntMax(FID, std::numeric_limits<unsigned>::max()))
+ if (auto Err = readIntMax(FID1, std::numeric_limits<int16_t>::max()))
return Err;
break;
case CounterMappingRegion::MCDCDecisionRegion:
Kind = CounterMappingRegion::MCDCDecisionRegion;
if (auto Err = readIntMax(BIDX, std::numeric_limits<unsigned>::max()))
return Err;
- if (auto Err = readIntMax(NC, std::numeric_limits<unsigned>::max()))
+ if (auto Err = readIntMax(NC, std::numeric_limits<int16_t>::max()))
return Err;
break;
default:
@@ -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<uint16_t>(NC),
+ static_cast<int16_t>(int(ID1) - 1),
+ static_cast<int16_t>(int(TID1) - 1),
+ static_cast<int16_t>(int(FID1) - 1)},
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..b9257a1c95e7d8 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
@@ -251,9 +251,9 @@ void CoverageMappingWriter::write(raw_ostream &OS) {
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.ID + 1), OS);
+ encodeULEB128(unsigned(I->MCDCParams.TrueID + 1), OS);
+ encodeULEB128(unsigned(I->MCDCParams.FalseID + 1), 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..cce271d523960e 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -192,7 +192,7 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
addCMR(Counter::getZero(), File, LS, CS, LE, CE, true);
}
- void addMCDCDecisionCMR(unsigned Mask, unsigned NC, StringRef File,
+ void addMCDCDecisionCMR(unsigned Mask, uint16_t NC, StringRef File,
unsigned LS, unsigned CS, unsigned LE, unsigned CE) {
auto &Regions = InputFunctions.back().Regions;
unsigned FileID = getFileIndexForFunction(File);
@@ -201,8 +201,8 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
CE));
}
- void addMCDCBranchCMR(Counter C1, Counter C2, unsigned ID, unsigned TrueID,
- unsigned FalseID, StringRef File, unsigned LS,
+ void addMCDCBranchCMR(Counter C1, Counter C2, int16_t ID, int16_t TrueID,
+ int16_t FalseID, StringRef File, unsigned LS,
unsigned CS, unsigned LE, unsigned CE) {
auto &Regions = InputFunctions.back().Regions;
unsigned FileID = getFileIndexForFunction(File);
@@ -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), 0, 1, -1,
"file", 7, 2, 7, 3);
- addMCDCBranchCMR(Counter::getCounter(2), Counter::getCounter(3), 2, 0, 0,
+ addMCDCBranchCMR(Counter::getCounter(2), Counter::getCounter(3), 1, -1, -1,
"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), 0, 1, -1,
+ "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), 1, -1, -1,
+ "B", 1, 14, 1, 17);
// InputFunctionCoverageData::Regions is rewritten after the write.
auto InputRegions = InputFunctions.back().Regions;
>From 7c8b750820e604b419dba763062a4ebe77c242bd Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sat, 10 Feb 2024 22:51:31 +0900
Subject: [PATCH 2/2] Update
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 3 ++-
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp | 4 ++--
.../ProfileData/Coverage/CoverageMappingReader.cpp | 4 +++-
.../ProfileData/Coverage/CoverageMappingWriter.cpp | 12 +++++++++---
llvm/unittests/ProfileData/CoverageMappingTest.cpp | 9 ++++++---
5 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index d918acd951dd8c..bd6a1d4b3b3e3a 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -687,7 +687,8 @@ struct MCDCCoverageBuilder {
MCDCConditionID NextID = 0;
bool NotMapped = false;
- /// Represent a sentinel value of [0,0] for the bottom of DecisionStack.
+ /// Represent a sentinel value as a pair of final decisions for the bottom
+ // of DecisionStack.
static constexpr DecisionIDPair DecisionStackSentinel{-1, -1};
/// Is this a logical-AND operation?
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 5b631bd7c27c2a..baf7ab39ee737d 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -309,7 +309,7 @@ class MCDCRecordProcessor {
/// vector at the corresponding index was executed during a test run.
void findExecutedTestVectors() {
// Walk the binary decision diagram to enumerate all possible test vectors.
- // We start at the root node (ID == 1) with all values being DontCare.
+ // We start at the root node (ID == 0) with all values being DontCare.
// `Index` encodes the bitmask of true values and is initially 0.
MCDCRecord::TestVector TV(NumConditions, MCDCRecord::MCDC_DontCare);
buildTestVector(TV, 0, 0);
@@ -574,7 +574,7 @@ class MCDCDecisionRecorder {
assert(MCDCBranches.size() < DecisionRegion->MCDCParams.NumConditions);
- // Put `ID=1` in front of `MCDCBranches` for convenience
+ // Put `ID=0` in front of `MCDCBranches` for convenience
// even if `MCDCBranches` is not topological.
if (ConditionID == 0)
MCDCBranches.insert(MCDCBranches.begin(), &Branch);
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
index df12d2fec3050a..1dfd7d9d27a5a1 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -244,7 +244,9 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray(
unsigned LineStart = 0;
for (size_t I = 0; I < NumRegions; ++I) {
Counter C, C2;
- uint64_t BIDX = 0, NC = 0, ID1 = 0, TID1 = 0, FID1 = 0;
+ uint64_t BIDX = 0, NC = 0;
+ // Tye are stored as internal values plus 1 (min is -1)
+ uint64_t ID1 = 0, TID1 = 0, FID1 = 0;
CounterMappingRegion::RegionKind Kind = CounterMappingRegion::CodeRegion;
// Read the combined counter + region kind.
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
index b9257a1c95e7d8..de3585d4fb0ca2 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
@@ -251,9 +251,15 @@ void CoverageMappingWriter::write(raw_ostream &OS) {
OS);
writeCounter(MinExpressions, Count, OS);
writeCounter(MinExpressions, FalseCount, OS);
- encodeULEB128(unsigned(I->MCDCParams.ID + 1), OS);
- encodeULEB128(unsigned(I->MCDCParams.TrueID + 1), OS);
- encodeULEB128(unsigned(I->MCDCParams.FalseID + 1), OS);
+ {
+ // They are written as internal values plus 1.
+ unsigned ID1 = I->MCDCParams.ID + 1;
+ unsigned TID1 = I->MCDCParams.TrueID + 1;
+ unsigned FID1 = I->MCDCParams.FalseID + 1;
+ encodeULEB128(ID1, OS);
+ encodeULEB128(TID1, OS);
+ encodeULEB128(FID1, OS);
+ }
break;
case CounterMappingRegion::MCDCDecisionRegion:
encodeULEB128(unsigned(I->Kind)
diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
index cce271d523960e..aac2aae4641647 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -22,6 +22,8 @@
using namespace llvm;
using namespace coverage;
+using MCDCConditionID = CounterMappingRegion::MCDCConditionID;
+
[[nodiscard]] static ::testing::AssertionResult
ErrorEquals(Error E, coveragemap_error Expected_Err,
const std::string &Expected_Msg = std::string()) {
@@ -201,9 +203,10 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
CE));
}
- void addMCDCBranchCMR(Counter C1, Counter C2, int16_t ID, int16_t TrueID,
- int16_t FalseID, StringRef File, unsigned LS,
- unsigned CS, unsigned LE, unsigned CE) {
+ void addMCDCBranchCMR(Counter C1, Counter C2, MCDCConditionID ID,
+ MCDCConditionID TrueID, MCDCConditionID FalseID,
+ StringRef File, unsigned LS, unsigned CS, unsigned LE,
+ unsigned CE) {
auto &Regions = InputFunctions.back().Regions;
unsigned FileID = getFileIndexForFunction(File);
Regions.push_back(CounterMappingRegion::makeBranchRegion(
More information about the cfe-commits
mailing list