Skip to content

Commit

Permalink
[MC/DC] Refactor: Make MCDCParams as std::variant (#81227)
Browse files Browse the repository at this point in the history 
Introduce `mcdc::DecisionParameters` and `mcdc::BranchParameters` and make
sure them not initialized as zero.

FIXME: Could we make `CoverageMappingRegion` as a smart tagged union?
  • Loading branch information
@chapuni
chapuni committed Feb 13, 2024
1 parent a70077e commit a17a3e9
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 100 deletions.
68 changes: 38 additions & 30 deletions clang/lib/CodeGen/CoverageMappingGen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,17 @@ class SourceMappingRegion {

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

bool isMCDCDecision() const { return MCDCParams.NumConditions != 0; }
bool isMCDCDecision() const {
const auto *DecisionParams =
std::get_if<mcdc::DecisionParameters>(&MCDCParams);
assert(!DecisionParams || DecisionParams->NumConditions > 0);
return DecisionParams;
}

const auto &getMCDCDecisionParams() const {
return CounterMappingRegion::getParams<const mcdc::DecisionParameters>(
MCDCParams);
}

const mcdc::Parameters &getMCDCParams() const { return MCDCParams; }
};
Expand Down Expand Up @@ -480,13 +490,13 @@ class CoverageMappingBuilder {
SR.ColumnEnd));
} else if (Region.isBranch()) {
MappingRegions.push_back(CounterMappingRegion::makeBranchRegion(
Region.getCounter(), Region.getFalseCounter(),
Region.getMCDCParams(), *CovFileID, SR.LineStart, SR.ColumnStart,
SR.LineEnd, SR.ColumnEnd));
Region.getCounter(), Region.getFalseCounter(), *CovFileID,
SR.LineStart, SR.ColumnStart, SR.LineEnd, SR.ColumnEnd,
Region.getMCDCParams()));
} else if (Region.isMCDCDecision()) {
MappingRegions.push_back(CounterMappingRegion::makeDecisionRegion(
Region.getMCDCParams(), *CovFileID, SR.LineStart, SR.ColumnStart,
SR.LineEnd, SR.ColumnEnd));
Region.getMCDCDecisionParams(), *CovFileID, SR.LineStart,
SR.ColumnStart, SR.LineEnd, SR.ColumnEnd));
} else {
MappingRegions.push_back(CounterMappingRegion::makeRegion(
Region.getCounter(), *CovFileID, SR.LineStart, SR.ColumnStart,
Expand Down Expand Up @@ -863,8 +873,7 @@ struct CounterCoverageMappingBuilder
std::optional<SourceLocation> StartLoc = std::nullopt,
std::optional<SourceLocation> EndLoc = std::nullopt,
std::optional<Counter> FalseCount = std::nullopt,
mcdc::ConditionID ID = 0, mcdc::ConditionID TrueID = 0,
mcdc::ConditionID FalseID = 0) {
const mcdc::Parameters &BranchParams = std::monostate()) {

if (StartLoc && !FalseCount) {
MostRecentLocation = *StartLoc;
Expand All @@ -883,9 +892,7 @@ struct CounterCoverageMappingBuilder
StartLoc = std::nullopt;
if (EndLoc && EndLoc->isInvalid())
EndLoc = std::nullopt;
RegionStack.emplace_back(Count, FalseCount,
mcdc::Parameters{0, 0, ID, TrueID, FalseID},
StartLoc, EndLoc);
RegionStack.emplace_back(Count, FalseCount, BranchParams, StartLoc, EndLoc);

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

RegionStack.emplace_back(mcdc::Parameters{BitmapIdx, Conditions}, StartLoc,
EndLoc);
RegionStack.emplace_back(mcdc::DecisionParameters{BitmapIdx, Conditions},
StartLoc, EndLoc);

return RegionStack.size() - 1;
}
Expand Down Expand Up @@ -1040,9 +1047,11 @@ struct CounterCoverageMappingBuilder
// function's SourceRegions) because it doesn't apply to any other source
// code other than the Condition.
if (CodeGenFunction::isInstrumentedCondition(C)) {
mcdc::Parameters BranchParams;
mcdc::ConditionID ID = MCDCBuilder.getCondID(C);
mcdc::ConditionID TrueID = IDPair.TrueID;
mcdc::ConditionID FalseID = IDPair.FalseID;
if (ID > 0)
BranchParams =
mcdc::BranchParameters{ID, IDPair.TrueID, 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
Expand All @@ -1052,11 +1061,11 @@ struct CounterCoverageMappingBuilder
// CodeGenFunction.c always returns false, but that is very heavy-handed.
if (ConditionFoldsToBool(C))
popRegions(pushRegion(Counter::getZero(), getStart(C), getEnd(C),
Counter::getZero(), ID, TrueID, FalseID));
Counter::getZero(), BranchParams));
else
// Otherwise, create a region with the True counter and False counter.
popRegions(pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt, ID,
TrueID, FalseID));
popRegions(pushRegion(TrueCnt, getStart(C), getEnd(C), FalseCnt,
BranchParams));
}
}

Expand Down Expand Up @@ -1147,12 +1156,9 @@ struct CounterCoverageMappingBuilder
// we've seen this region.
if (StartLocs.insert(Loc).second) {
if (I.isBranch())
SourceRegions.emplace_back(
I.getCounter(), I.getFalseCounter(),
mcdc::Parameters{0, 0, I.getMCDCParams().ID,
I.getMCDCParams().TrueID,
I.getMCDCParams().FalseID},
Loc, getEndOfFileOrMacro(Loc), I.isBranch());
SourceRegions.emplace_back(I.getCounter(), I.getFalseCounter(),
I.getMCDCParams(), Loc,
getEndOfFileOrMacro(Loc), I.isBranch());
else
SourceRegions.emplace_back(I.getCounter(), Loc,
getEndOfFileOrMacro(Loc));
Expand Down Expand Up @@ -2118,9 +2124,10 @@ static void dump(llvm::raw_ostream &OS, StringRef FunctionName,
OS << "File " << R.FileID << ", " << R.LineStart << ":" << R.ColumnStart
<< " -> " << R.LineEnd << ":" << R.ColumnEnd << " = ";

if (R.Kind == CounterMappingRegion::MCDCDecisionRegion) {
OS << "M:" << R.MCDCParams.BitmapIdx;
OS << ", C:" << R.MCDCParams.NumConditions;
if (const auto *DecisionParams =
std::get_if<mcdc::DecisionParameters>(&R.MCDCParams)) {
OS << "M:" << DecisionParams->BitmapIdx;
OS << ", C:" << DecisionParams->NumConditions;
} else {
Ctx.dump(R.Count, OS);

Expand All @@ -2131,9 +2138,10 @@ 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 << "] ";
if (const auto *BranchParams =
std::get_if<mcdc::BranchParameters>(&R.MCDCParams)) {
OS << " [" << BranchParams->ID << "," << BranchParams->TrueID;
OS << "," << BranchParams->FalseID << "] ";
}

if (R.Kind == CounterMappingRegion::ExpansionRegion)
Expand Down
65 changes: 39 additions & 26 deletions llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,24 @@ struct CounterMappingRegion {
/// Parameters used for Modified Condition/Decision Coverage
mcdc::Parameters MCDCParams;

template <class MaybeConstInnerParameters, class MaybeConstMCDCParameters>
static auto &getParams(MaybeConstMCDCParameters &MCDCParams) {
using InnerParameters =
typename std::remove_const<MaybeConstInnerParameters>::type;
MaybeConstInnerParameters *Params =
std::get_if<InnerParameters>(&MCDCParams);
assert(Params && "InnerParameters unavailable");
return *Params;
}

const auto &getDecisionParams() const {
return getParams<const mcdc::DecisionParameters>(MCDCParams);
}

const auto &getBranchParams() const {
return getParams<const mcdc::BranchParameters>(MCDCParams);
}

unsigned FileID = 0;
unsigned ExpandedFileID = 0;
unsigned LineStart, ColumnStart, LineEnd, ColumnEnd;
Expand All @@ -272,19 +290,20 @@ struct CounterMappingRegion {
LineStart(LineStart), ColumnStart(ColumnStart), LineEnd(LineEnd),
ColumnEnd(ColumnEnd), Kind(Kind) {}

CounterMappingRegion(Counter Count, Counter FalseCount,
mcdc::Parameters MCDCParams, unsigned FileID,
CounterMappingRegion(Counter Count, Counter FalseCount, unsigned FileID,
unsigned ExpandedFileID, unsigned LineStart,
unsigned ColumnStart, unsigned LineEnd,
unsigned ColumnEnd, RegionKind Kind)
unsigned ColumnEnd, RegionKind Kind,
const mcdc::Parameters &MCDCParams = std::monostate())
: Count(Count), FalseCount(FalseCount), MCDCParams(MCDCParams),
FileID(FileID), ExpandedFileID(ExpandedFileID), LineStart(LineStart),
ColumnStart(ColumnStart), LineEnd(LineEnd), ColumnEnd(ColumnEnd),
Kind(Kind) {}

CounterMappingRegion(mcdc::Parameters MCDCParams, unsigned FileID,
unsigned LineStart, unsigned ColumnStart,
unsigned LineEnd, unsigned ColumnEnd, RegionKind Kind)
CounterMappingRegion(const mcdc::DecisionParameters &MCDCParams,
unsigned FileID, unsigned LineStart,
unsigned ColumnStart, unsigned LineEnd,
unsigned ColumnEnd, RegionKind Kind)
: MCDCParams(MCDCParams), FileID(FileID), LineStart(LineStart),
ColumnStart(ColumnStart), LineEnd(LineEnd), ColumnEnd(ColumnEnd),
Kind(Kind) {}
Expand Down Expand Up @@ -321,27 +340,20 @@ struct CounterMappingRegion {
static CounterMappingRegion
makeBranchRegion(Counter Count, Counter FalseCount, unsigned FileID,
unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
unsigned ColumnEnd) {
return CounterMappingRegion(Count, FalseCount, mcdc::Parameters(), FileID,
0, LineStart, ColumnStart, LineEnd, ColumnEnd,
BranchRegion);
}

static CounterMappingRegion
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
: MCDCBranchRegion);
unsigned ColumnEnd,
const mcdc::Parameters &MCDCParams = std::monostate()) {
return CounterMappingRegion(
Count, FalseCount, FileID, 0, LineStart, ColumnStart, LineEnd,
ColumnEnd,
(std::get_if<mcdc::BranchParameters>(&MCDCParams) ? MCDCBranchRegion
: BranchRegion),
MCDCParams);
}

static CounterMappingRegion
makeDecisionRegion(mcdc::Parameters MCDCParams, unsigned FileID,
unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
unsigned ColumnEnd) {
makeDecisionRegion(const mcdc::DecisionParameters &MCDCParams,
unsigned FileID, unsigned LineStart, unsigned ColumnStart,
unsigned LineEnd, unsigned ColumnEnd) {
return CounterMappingRegion(MCDCParams, FileID, LineStart, ColumnStart,
LineEnd, ColumnEnd, MCDCDecisionRegion);
}
Expand Down Expand Up @@ -406,9 +418,10 @@ struct MCDCRecord {

CounterMappingRegion getDecisionRegion() const { return Region; }
unsigned getNumConditions() const {
assert(Region.MCDCParams.NumConditions != 0 &&
unsigned NumConditions = Region.getDecisionParams().NumConditions;
assert(NumConditions != 0 &&
"In MC/DC, NumConditions should never be zero!");
return Region.MCDCParams.NumConditions;
return NumConditions;
}
unsigned getNumTestVectors() const { return TV.size(); }
bool isCondFolded(unsigned Condition) const { return Folded[Condition]; }
Expand Down
21 changes: 16 additions & 5 deletions llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,35 @@
#ifndef LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
#define LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H

#include <variant>

namespace llvm::coverage::mcdc {

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

/// MC/DC-specifig parameters
struct Parameters {
struct DecisionParameters {
/// Byte Index of Bitmap Coverage Object for a Decision Region.
unsigned BitmapIdx = 0;
unsigned BitmapIdx;

/// Number of Conditions used for a Decision Region.
unsigned NumConditions = 0;
unsigned NumConditions;

DecisionParameters() = delete;
};

struct BranchParameters {
/// 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;
ConditionID ID, TrueID, FalseID;

BranchParameters() = delete;
};

/// The type of MC/DC-specific parameters.
using Parameters =
std::variant<std::monostate, DecisionParameters, BranchParameters>;

} // namespace llvm::coverage::mcdc

#endif // LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H

0 comments on commit a17a3e9

Please sign in to comment.