[clang] [llvm] [MC/DC] Refactor: Introduce `ConditionIDs` as `std::array<2>` (PR #81221)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 8 19:34:25 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: NAKAMURA Takumi (chapuni)
<details>
<summary>Changes</summary>
Its 0th element corresponds to `FalseID` and 1st to `TrueID`.
CoverageMappingGen.cpp: `DecisionIDPair` is replaced with `ConditionIDs`
---
Full diff: https://github.com/llvm/llvm-project/pull/81221.diff
6 Files Affected:
- (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+18-32)
- (modified) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+6-3)
- (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+14-17)
- (modified) llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp (+4-3)
- (modified) llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp (+2-2)
- (modified) llvm/unittests/ProfileData/CoverageMappingTest.cpp (+12-12)
``````````diff
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0c43317642bca4..54559af47e63dc 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -95,7 +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.
@@ -586,11 +585,6 @@ struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder {
/// creation.
struct MCDCCoverageBuilder {
- struct DecisionIDPair {
- MCDCConditionID TrueID = 0;
- MCDCConditionID FalseID = 0;
- };
-
/// The AST walk recursively visits nested logical-AND or logical-OR binary
/// operator nodes and then visits their LHS and RHS children nodes. As this
/// happens, the algorithm will assign IDs to each operator's LHS and RHS side
@@ -681,14 +675,14 @@ struct MCDCCoverageBuilder {
private:
CodeGenModule &CGM;
- llvm::SmallVector<DecisionIDPair> DecisionStack;
+ llvm::SmallVector<MCDCConditionIDs> DecisionStack;
llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDs;
llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap;
MCDCConditionID NextID = 1;
bool NotMapped = false;
/// Represent a sentinel value of [0,0] for the bottom of DecisionStack.
- static constexpr DecisionIDPair DecisionStackSentinel{0, 0};
+ static constexpr MCDCConditionIDs DecisionStackSentinel{0, 0};
/// Is this a logical-AND operation?
bool isLAnd(const BinaryOperator *E) const {
@@ -727,7 +721,7 @@ struct MCDCCoverageBuilder {
}
/// Return the LHS Decision ([0,0] if not set).
- const DecisionIDPair &back() const { return DecisionStack.back(); }
+ const MCDCConditionIDs &back() const { return DecisionStack.back(); }
/// Push the binary operator statement to track the nest level and assign IDs
/// to the operator's LHS and RHS. The RHS may be a larger subtree that is
@@ -744,7 +738,7 @@ struct MCDCCoverageBuilder {
if (NotMapped)
return;
- const DecisionIDPair &ParentDecision = DecisionStack.back();
+ const MCDCConditionIDs &ParentDecision = DecisionStack.back();
// If the operator itself has an assigned ID, this means it represents a
// larger subtree. In this case, assign that ID to its LHS node. Its RHS
@@ -760,18 +754,18 @@ struct MCDCCoverageBuilder {
// Push the LHS decision IDs onto the DecisionStack.
if (isLAnd(E))
- DecisionStack.push_back({RHSid, ParentDecision.FalseID});
+ DecisionStack.push_back({ParentDecision[0], RHSid});
else
- DecisionStack.push_back({ParentDecision.TrueID, RHSid});
+ DecisionStack.push_back({RHSid, ParentDecision[1]});
}
/// Pop and return the LHS Decision ([0,0] if not set).
- DecisionIDPair pop() {
+ MCDCConditionIDs pop() {
if (!CGM.getCodeGenOpts().MCDCCoverage || NotMapped)
return DecisionStack.front();
assert(DecisionStack.size() > 1);
- DecisionIDPair D = DecisionStack.back();
+ MCDCConditionIDs D = DecisionStack.back();
DecisionStack.pop_back();
return D;
}
@@ -865,8 +859,7 @@ 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 = 0, MCDCConditionIDs Conds = {}) {
if (StartLoc && !FalseCount) {
MostRecentLocation = *StartLoc;
@@ -885,8 +878,7 @@ struct CounterCoverageMappingBuilder
StartLoc = std::nullopt;
if (EndLoc && EndLoc->isInvalid())
EndLoc = std::nullopt;
- RegionStack.emplace_back(Count, FalseCount,
- MCDCParameters{0, 0, ID, TrueID, FalseID},
+ RegionStack.emplace_back(Count, FalseCount, MCDCParameters{0, 0, ID, Conds},
StartLoc, EndLoc);
return RegionStack.size() - 1;
@@ -1024,15 +1016,12 @@ struct CounterCoverageMappingBuilder
return (Cond->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext()));
}
- using MCDCDecisionIDPair = MCDCCoverageBuilder::DecisionIDPair;
-
/// Create a Branch Region around an instrumentable condition for coverage
/// and add it to the function's SourceRegions. A branch region tracks a
/// "True" counter and a "False" counter for boolean expressions that
/// result in the generation of a branch.
- void
- createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt,
- const MCDCDecisionIDPair &IDPair = MCDCDecisionIDPair()) {
+ void createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt,
+ const MCDCConditionIDs &Conds = {}) {
// Check for NULL conditions.
if (!C)
return;
@@ -1043,8 +1032,6 @@ struct CounterCoverageMappingBuilder
// code other than the Condition.
if (CodeGenFunction::isInstrumentedCondition(C)) {
MCDCConditionID ID = MCDCBuilder.getCondID(C);
- MCDCConditionID TrueID = IDPair.TrueID;
- MCDCConditionID 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
@@ -1054,11 +1041,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(), ID, Conds));
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, ID, Conds));
}
}
@@ -1152,8 +1139,7 @@ struct CounterCoverageMappingBuilder
SourceRegions.emplace_back(
I.getCounter(), I.getFalseCounter(),
MCDCParameters{0, 0, I.getMCDCParams().ID,
- I.getMCDCParams().TrueID,
- I.getMCDCParams().FalseID},
+ I.getMCDCParams().Conds},
Loc, getEndOfFileOrMacro(Loc), I.isBranch());
else
SourceRegions.emplace_back(I.getCounter(), Loc,
@@ -2134,8 +2120,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 << "," << R.MCDCParams.Conds[1];
+ OS << "," << R.MCDCParams.Conds[0] << "] ";
}
if (R.Kind == CounterMappingRegion::ExpansionRegion)
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 88ec60c7aa33c6..d07baa8e71a29f 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -30,6 +30,7 @@
#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/raw_ostream.h"
+#include <array>
#include <cassert>
#include <cstdint>
#include <iterator>
@@ -37,7 +38,6 @@
#include <sstream>
#include <string>
#include <system_error>
-#include <tuple>
#include <utility>
#include <vector>
@@ -217,6 +217,9 @@ class CounterExpressionBuilder {
using LineColPair = std::pair<unsigned, unsigned>;
+using MCDCConditionID = unsigned int;
+using MCDCConditionIDs = std::array<MCDCConditionID, 2>;
+
/// A Counter mapping region associates a source range with a specific counter.
struct CounterMappingRegion {
enum RegionKind {
@@ -249,7 +252,6 @@ struct CounterMappingRegion {
MCDCBranchRegion
};
- using MCDCConditionID = unsigned int;
struct MCDCParameters {
/// Byte Index of Bitmap Coverage Object for a Decision Region.
unsigned BitmapIdx = 0;
@@ -259,7 +261,8 @@ struct CounterMappingRegion {
/// 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 = 0;
+ MCDCConditionIDs Conds;
};
/// Primary Counter that is also used for Branch Regions (TrueCount).
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index eb0996e33b70dc..66152685af831b 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -245,7 +245,7 @@ class MCDCRecordProcessor {
unsigned BitmapIdx;
/// Mapping of a condition ID to its corresponding branch region.
- llvm::DenseMap<unsigned, const CounterMappingRegion *> Map;
+ llvm::DenseMap<unsigned, MCDCConditionIDs> CondsMap;
/// Vector used to track whether a condition is constant folded.
MCDCRecord::BoolVector Folded;
@@ -285,20 +285,17 @@ class MCDCRecordProcessor {
// the truth table.
void buildTestVector(MCDCRecord::TestVector &TV, unsigned ID,
unsigned Index) {
- const CounterMappingRegion *Branch = Map[ID];
-
- TV[ID - 1] = 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)
- buildTestVector(TV, Branch->MCDCParams.TrueID, Index);
- else
- recordTestVector(TV, Index, MCDCRecord::MCDC_True);
+ const auto &Conds = CondsMap[ID];
+
+ for (unsigned I = 0; I < 2; ++I) {
+ auto MCDCCond = (I ? MCDCRecord::MCDC_True : MCDCRecord::MCDC_False);
+ Index |= I << (ID - 1);
+ TV[ID - 1] = MCDCCond;
+ if (Conds[I] > 0)
+ buildTestVector(TV, Conds[I], Index);
+ else
+ recordTestVector(TV, Index, MCDCCond);
+ }
// Reset back to DontCare.
TV[ID - 1] = MCDCRecord::MCDC_DontCare;
@@ -371,7 +368,7 @@ class MCDCRecordProcessor {
// - Record whether the condition is constant folded so that we exclude it
// from being measured.
for (const auto *B : Branches) {
- Map[B->MCDCParams.ID] = B;
+ CondsMap[B->MCDCParams.ID] = B->MCDCParams.Conds;
PosToID[I] = B->MCDCParams.ID - 1;
CondLoc[I] = B->startLoc();
Folded[I++] = (B->Count.isZero() && B->FalseCount.isZero());
@@ -524,7 +521,7 @@ class MCDCDecisionRecorder {
/// IDs that are stored in MCDCBranches
/// Complete when all IDs (1 to NumConditions) are met.
- DenseSet<CounterMappingRegion::MCDCConditionID> ConditionIDs;
+ DenseSet<MCDCConditionID> 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..351c16397da8f0 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -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<unsigned>(NC),
+ static_cast<unsigned>(ID),
+ {static_cast<unsigned>(FID), static_cast<unsigned>(TID)}},
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..5a5d546de8bbb7 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
@@ -252,8 +252,8 @@ void CoverageMappingWriter::write(raw_ostream &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.Conds[1]), OS);
+ encodeULEB128(unsigned(I->MCDCParams.Conds[0]), 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..c26c096fdf9f36 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -197,18 +197,18 @@ 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));
+ CounterMappingRegion::MCDCParameters{Mask, NC, 0, {}}, FileID, LS, CS,
+ LE, CE));
}
- void addMCDCBranchCMR(Counter C1, Counter C2, unsigned ID, unsigned TrueID,
- unsigned FalseID, StringRef File, unsigned LS,
+ void addMCDCBranchCMR(Counter C1, Counter C2, unsigned ID,
+ MCDCConditionIDs Conds, StringRef File, unsigned LS,
unsigned CS, unsigned LE, unsigned CE) {
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, CounterMappingRegion::MCDCParameters{0, 0, ID, Conds}, FileID,
+ LS, CS, LE, CE));
}
void addExpansionCMR(StringRef File, StringRef ExpandedFile, unsigned LS,
@@ -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), 1, {0, 2},
"file", 7, 2, 7, 3);
- addMCDCBranchCMR(Counter::getCounter(2), Counter::getCounter(3), 2, 0, 0,
+ addMCDCBranchCMR(Counter::getCounter(2), Counter::getCounter(3), 2, {0, 0},
"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), 1, {0, 2},
+ "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), 2, {0, 0},
+ "B", 1, 14, 1, 17);
// InputFunctionCoverageData::Regions is rewritten after the write.
auto InputRegions = InputFunctions.back().Regions;
``````````
</details>
https://github.com/llvm/llvm-project/pull/81221
More information about the llvm-commits
mailing list