Skip to content

Commit a17a3e9

Browse files
chapunichapuni
authored
[MC/DC] Refactor: Make MCDCParams as std::variant (#81227)
Introduce `mcdc::DecisionParameters` and `mcdc::BranchParameters` and make sure them not initialized as zero. FIXME: Could we make `CoverageMappingRegion` as a smart tagged union?
1 parent a70077e commit a17a3e9

File tree

7 files changed

+153
-100
lines changed

7 files changed

+153
-100
lines changed

‎clang/lib/CodeGen/CoverageMappingGen.cpp

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,17 @@ class SourceMappingRegion {
182182

183183
bool isBranch() const { return FalseCount.has_value(); }
184184

185-
bool isMCDCDecision() const { return MCDCParams.NumConditions != 0; }
185+
bool isMCDCDecision() const {
186+
const auto *DecisionParams =
187+
std::get_if<mcdc::DecisionParameters>(&MCDCParams);
188+
assert(!DecisionParams || DecisionParams->NumConditions > 0);
189+
return DecisionParams;
190+
}
191+
192+
const auto &getMCDCDecisionParams() const {
193+
return CounterMappingRegion::getParams<const mcdc::DecisionParameters>(
194+
MCDCParams);
195+
}
186196

187197
const mcdc::Parameters &getMCDCParams() const { return MCDCParams; }
188198
};
@@ -480,13 +490,13 @@ class CoverageMappingBuilder {
480490
SR.ColumnEnd));
481491
} else if (Region.isBranch()) {
482492
MappingRegions.push_back(CounterMappingRegion::makeBranchRegion(
483-
Region.getCounter(), Region.getFalseCounter(),
484-
Region.getMCDCParams(), *CovFileID, SR.LineStart, SR.ColumnStart,
485-
SR.LineEnd, SR.ColumnEnd));
493+
Region.getCounter(), Region.getFalseCounter(), *CovFileID,
494+
SR.LineStart, SR.ColumnStart, SR.LineEnd, SR.ColumnEnd,
495+
Region.getMCDCParams()));
486496
} else if (Region.isMCDCDecision()) {
487497
MappingRegions.push_back(CounterMappingRegion::makeDecisionRegion(
488-
Region.getMCDCParams(), *CovFileID, SR.LineStart, SR.ColumnStart,
489-
SR.LineEnd, SR.ColumnEnd));
498+
Region.getMCDCDecisionParams(), *CovFileID, SR.LineStart,
499+
SR.ColumnStart, SR.LineEnd, SR.ColumnEnd));
490500
} else {
491501
MappingRegions.push_back(CounterMappingRegion::makeRegion(
492502
Region.getCounter(), *CovFileID, SR.LineStart, SR.ColumnStart,
@@ -863,8 +873,7 @@ struct CounterCoverageMappingBuilder
863873
std::optional<SourceLocation> StartLoc = std::nullopt,
864874
std::optional<SourceLocation> EndLoc = std::nullopt,
865875
std::optional<Counter> FalseCount = std::nullopt,
866-
mcdc::ConditionID ID = 0, mcdc::ConditionID TrueID = 0,
867-
mcdc::ConditionID FalseID = 0) {
876+
const mcdc::Parameters &BranchParams = std::monostate()) {
868877

869878
if (StartLoc && !FalseCount) {
870879
MostRecentLocation = *StartLoc;
@@ -883,9 +892,7 @@ struct CounterCoverageMappingBuilder
883892
StartLoc = std::nullopt;
884893
if (EndLoc && EndLoc->isInvalid())
885894
EndLoc = std::nullopt;
886-
RegionStack.emplace_back(Count, FalseCount,
887-
mcdc::Parameters{0, 0, ID, TrueID, FalseID},
888-
StartLoc, EndLoc);
895+
RegionStack.emplace_back(Count, FalseCount, BranchParams, StartLoc, EndLoc);
889896

890897
return RegionStack.size() - 1;
891898
}
@@ -894,8 +901,8 @@ struct CounterCoverageMappingBuilder
894901
std::optional<SourceLocation> StartLoc = std::nullopt,
895902
std::optional<SourceLocation> EndLoc = std::nullopt) {
896903

897-
RegionStack.emplace_back(mcdc::Parameters{BitmapIdx, Conditions}, StartLoc,
898-
EndLoc);
904+
RegionStack.emplace_back(mcdc::DecisionParameters{BitmapIdx, Conditions},
905+
StartLoc, EndLoc);
899906

900907
return RegionStack.size() - 1;
901908
}
@@ -1040,9 +1047,11 @@ struct CounterCoverageMappingBuilder
10401047
// function's SourceRegions) because it doesn't apply to any other source
10411048
// code other than the Condition.
10421049
if (CodeGenFunction::isInstrumentedCondition(C)) {
1050+
mcdc::Parameters BranchParams;
10431051
mcdc::ConditionID ID = MCDCBuilder.getCondID(C);
1044-
mcdc::ConditionID TrueID = IDPair.TrueID;
1045-
mcdc::ConditionID FalseID = IDPair.FalseID;
1052+
if (ID > 0)
1053+
BranchParams =
1054+
mcdc::BranchParameters{ID, IDPair.TrueID, IDPair.FalseID};
10461055

10471056
// If a condition can fold to true or false, the corresponding branch
10481057
// will be removed. Create a region with both counters hard-coded to
@@ -1052,11 +1061,11 @@ struct CounterCoverageMappingBuilder
10521061
// CodeGenFunction.c always returns false, but that is very heavy-handed.
10531062
if (ConditionFoldsToBool(C))
10541063
popRegions(pushRegion(Counter::getZero(), getStart(C), getEnd(C),
1055-
Counter::getZero(), ID, TrueID, FalseID));
1064+
Counter::getZero(), BranchParams));
10561065
else
10571066
// Otherwise, create a region with the True counter and False counter.
1058-
popRegions(pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt, ID,
1059-
TrueID, FalseID));
1067+
popRegions(pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt,
1068+
BranchParams));
10601069
}
10611070
}
10621071

@@ -1147,12 +1156,9 @@ struct CounterCoverageMappingBuilder
11471156
// we've seen this region.
11481157
if (StartLocs.insert(Loc).second) {
11491158
if (I.isBranch())
1150-
SourceRegions.emplace_back(
1151-
I.getCounter(), I.getFalseCounter(),
1152-
mcdc::Parameters{0, 0, I.getMCDCParams().ID,
1153-
I.getMCDCParams().TrueID,
1154-
I.getMCDCParams().FalseID},
1155-
Loc, getEndOfFileOrMacro(Loc), I.isBranch());
1159+
SourceRegions.emplace_back(I.getCounter(), I.getFalseCounter(),
1160+
I.getMCDCParams(), Loc,
1161+
getEndOfFileOrMacro(Loc), I.isBranch());
11561162
else
11571163
SourceRegions.emplace_back(I.getCounter(), Loc,
11581164
getEndOfFileOrMacro(Loc));
@@ -2118,9 +2124,10 @@ static void dump(llvm::raw_ostream &OS, StringRef FunctionName,
21182124
OS << "File " << R.FileID << ", " << R.LineStart << ":" << R.ColumnStart
21192125
<< " -> " << R.LineEnd << ":" << R.ColumnEnd << " = ";
21202126

2121-
if (R.Kind == CounterMappingRegion::MCDCDecisionRegion) {
2122-
OS << "M:" << R.MCDCParams.BitmapIdx;
2123-
OS << ", C:" << R.MCDCParams.NumConditions;
2127+
if (const auto *DecisionParams =
2128+
std::get_if<mcdc::DecisionParameters>(&R.MCDCParams)) {
2129+
OS << "M:" << DecisionParams->BitmapIdx;
2130+
OS << ", C:" << DecisionParams->NumConditions;
21242131
} else {
21252132
Ctx.dump(R.Count, OS);
21262133

@@ -2131,9 +2138,10 @@ static void dump(llvm::raw_ostream &OS, StringRef FunctionName,
21312138
}
21322139
}
21332140

2134-
if (R.Kind == CounterMappingRegion::MCDCBranchRegion) {
2135-
OS << " [" << R.MCDCParams.ID << "," << R.MCDCParams.TrueID;
2136-
OS << "," << R.MCDCParams.FalseID << "] ";
2141+
if (const auto *BranchParams =
2142+
std::get_if<mcdc::BranchParameters>(&R.MCDCParams)) {
2143+
OS << " [" << BranchParams->ID << "," << BranchParams->TrueID;
2144+
OS << "," << BranchParams->FalseID << "] ";
21372145
}
21382146

21392147
if (R.Kind == CounterMappingRegion::ExpansionRegion)

‎llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,24 @@ struct CounterMappingRegion {
259259
/// Parameters used for Modified Condition/Decision Coverage
260260
mcdc::Parameters MCDCParams;
261261

262+
template <class MaybeConstInnerParameters, class MaybeConstMCDCParameters>
263+
static auto &getParams(MaybeConstMCDCParameters &MCDCParams) {
264+
using InnerParameters =
265+
typename std::remove_const<MaybeConstInnerParameters>::type;
266+
MaybeConstInnerParameters *Params =
267+
std::get_if<InnerParameters>(&MCDCParams);
268+
assert(Params && "InnerParameters unavailable");
269+
return *Params;
270+
}
271+
272+
const auto &getDecisionParams() const {
273+
return getParams<const mcdc::DecisionParameters>(MCDCParams);
274+
}
275+
276+
const auto &getBranchParams() const {
277+
return getParams<const mcdc::BranchParameters>(MCDCParams);
278+
}
279+
262280
unsigned FileID = 0;
263281
unsigned ExpandedFileID = 0;
264282
unsigned LineStart, ColumnStart, LineEnd, ColumnEnd;
@@ -272,19 +290,20 @@ struct CounterMappingRegion {
272290
LineStart(LineStart), ColumnStart(ColumnStart), LineEnd(LineEnd),
273291
ColumnEnd(ColumnEnd), Kind(Kind) {}
274292

275-
CounterMappingRegion(Counter Count, Counter FalseCount,
276-
mcdc::Parameters MCDCParams, unsigned FileID,
293+
CounterMappingRegion(Counter Count, Counter FalseCount, unsigned FileID,
277294
unsigned ExpandedFileID, unsigned LineStart,
278295
unsigned ColumnStart, unsigned LineEnd,
279-
unsigned ColumnEnd, RegionKind Kind)
296+
unsigned ColumnEnd, RegionKind Kind,
297+
const mcdc::Parameters &MCDCParams = std::monostate())
280298
: Count(Count), FalseCount(FalseCount), MCDCParams(MCDCParams),
281299
FileID(FileID), ExpandedFileID(ExpandedFileID), LineStart(LineStart),
282300
ColumnStart(ColumnStart), LineEnd(LineEnd), ColumnEnd(ColumnEnd),
283301
Kind(Kind) {}
284302

285-
CounterMappingRegion(mcdc::Parameters MCDCParams, unsigned FileID,
286-
unsigned LineStart, unsigned ColumnStart,
287-
unsigned LineEnd, unsigned ColumnEnd, RegionKind Kind)
303+
CounterMappingRegion(const mcdc::DecisionParameters &MCDCParams,
304+
unsigned FileID, unsigned LineStart,
305+
unsigned ColumnStart, unsigned LineEnd,
306+
unsigned ColumnEnd, RegionKind Kind)
288307
: MCDCParams(MCDCParams), FileID(FileID), LineStart(LineStart),
289308
ColumnStart(ColumnStart), LineEnd(LineEnd), ColumnEnd(ColumnEnd),
290309
Kind(Kind) {}
@@ -321,27 +340,20 @@ struct CounterMappingRegion {
321340
static CounterMappingRegion
322341
makeBranchRegion(Counter Count, Counter FalseCount, unsigned FileID,
323342
unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
324-
unsigned ColumnEnd) {
325-
return CounterMappingRegion(Count, FalseCount, mcdc::Parameters(), FileID,
326-
0, LineStart, ColumnStart, LineEnd, ColumnEnd,
327-
BranchRegion);
328-
}
329-
330-
static CounterMappingRegion
331-
makeBranchRegion(Counter Count, Counter FalseCount,
332-
mcdc::Parameters MCDCParams, unsigned FileID,
333-
unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
334-
unsigned ColumnEnd) {
335-
return CounterMappingRegion(Count, FalseCount, MCDCParams, FileID, 0,
336-
LineStart, ColumnStart, LineEnd, ColumnEnd,
337-
MCDCParams.ID == 0 ? BranchRegion
338-
: MCDCBranchRegion);
343+
unsigned ColumnEnd,
344+
const mcdc::Parameters &MCDCParams = std::monostate()) {
345+
return CounterMappingRegion(
346+
Count, FalseCount, FileID, 0, LineStart, ColumnStart, LineEnd,
347+
ColumnEnd,
348+
(std::get_if<mcdc::BranchParameters>(&MCDCParams) ? MCDCBranchRegion
349+
: BranchRegion),
350+
MCDCParams);
339351
}
340352

341353
static CounterMappingRegion
342-
makeDecisionRegion(mcdc::Parameters MCDCParams, unsigned FileID,
343-
unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
344-
unsigned ColumnEnd) {
354+
makeDecisionRegion(const mcdc::DecisionParameters &MCDCParams,
355+
unsigned FileID, unsigned LineStart, unsigned ColumnStart,
356+
unsigned LineEnd, unsigned ColumnEnd) {
345357
return CounterMappingRegion(MCDCParams, FileID, LineStart, ColumnStart,
346358
LineEnd, ColumnEnd, MCDCDecisionRegion);
347359
}
@@ -406,9 +418,10 @@ struct MCDCRecord {
406418

407419
CounterMappingRegion getDecisionRegion() const { return Region; }
408420
unsigned getNumConditions() const {
409-
assert(Region.MCDCParams.NumConditions != 0 &&
421+
unsigned NumConditions = Region.getDecisionParams().NumConditions;
422+
assert(NumConditions != 0 &&
410423
"In MC/DC, NumConditions should never be zero!");
411-
return Region.MCDCParams.NumConditions;
424+
return NumConditions;
412425
}
413426
unsigned getNumTestVectors() const { return TV.size(); }
414427
bool isCondFolded(unsigned Condition) const { return Folded[Condition]; }

‎llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,24 +13,35 @@
1313
#ifndef LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
1414
#define LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
1515

16+
#include <variant>
17+
1618
namespace llvm::coverage::mcdc {
1719

1820
/// The ID for MCDCBranch.
1921
using ConditionID = unsigned int;
2022

21-
/// MC/DC-specifig parameters
22-
struct Parameters {
23+
struct DecisionParameters {
2324
/// Byte Index of Bitmap Coverage Object for a Decision Region.
24-
unsigned BitmapIdx = 0;
25+
unsigned BitmapIdx;
2526

2627
/// Number of Conditions used for a Decision Region.
27-
unsigned NumConditions = 0;
28+
unsigned NumConditions;
29+
30+
DecisionParameters() = delete;
31+
};
2832

33+
struct BranchParameters {
2934
/// IDs used to represent a branch region and other branch regions
3035
/// evaluated based on True and False branches.
31-
ConditionID ID = 0, TrueID = 0, FalseID = 0;
36+
ConditionID ID, TrueID, FalseID;
37+
38+
BranchParameters() = delete;
3239
};
3340

41+
/// The type of MC/DC-specific parameters.
42+
using Parameters =
43+
std::variant<std::monostate, DecisionParameters, BranchParameters>;
44+
3445
} // namespace llvm::coverage::mcdc
3546

3647
#endif // LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H

0 commit comments

Comments
 (0)