[MC/DC] Refactor: Let MCDCConditionID int16_t with zero-origin #81257
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also, Let NumConditions uint16_t
llvmbot
added
clang
|
@llvm/pr-subscribers-pgo @llvm/pr-subscribers-clang-codegen Author: NAKAMURA Takumi (chapuni) ChangesAlso, Let NumConditions uint16_t Full diff: https://github.com/llvm/llvm-project/pull/81257.diff 9 Files Affected:
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;
|
|
@llvm/pr-subscribers-clang Author: NAKAMURA Takumi (chapuni) ChangesAlso, Let NumConditions uint16_t Full diff: https://github.com/llvm/llvm-project/pull/81257.diff 9 Files Affected:
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;
|
This was referenced Feb 10, 2024
|
Yes, zero-based numbering is more conventional in LLVM. LGTM |
ornata
approved these changes
Feb 15, 2024
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also, Let
NumConditionsuint16_t.It is smarter to handle the ID as signed.
Narrowing to
int16_twill reduce costs of handling byvalue. (See also #81221 and #81227)External behavior doesn't change. They below handle values as internal values plus 1.
-dump-coverage-mappingCoverageMappingReader.cppCoverageMappingWriter.cpp