[clang] 1a1fcac - [MC/DC] Refactor: Introduce `ConditionIDs` as `std::array<2>` (#81221)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 14 06:17:04 PST 2024
Author: NAKAMURA Takumi
Date: 2024-02-14T23:17:00+09:00
New Revision: 1a1fcacbce805e3c409d9d41de61413e3fd8aa36
URL: https://github.com/llvm/llvm-project/commit/1a1fcacbce805e3c409d9d41de61413e3fd8aa36
DIFF: https://github.com/llvm/llvm-project/commit/1a1fcacbce805e3c409d9d41de61413e3fd8aa36.diff
LOG: [MC/DC] Refactor: Introduce `ConditionIDs` as `std::array<2>` (#81221)
Its 0th element corresponds to `FalseID` and 1st to `TrueID`.
CoverageMappingGen.cpp: `DecisionIDPair` is replaced with `ConditionIDs`
Added:
Modified:
clang/lib/CodeGen/CoverageMappingGen.cpp
llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
llvm/unittests/ProfileData/CoverageMappingTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 3b711c05e92754..93c3c31e71fa83 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -593,11 +593,6 @@ struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder {
/// creation.
struct MCDCCoverageBuilder {
- struct DecisionIDPair {
- mcdc::ConditionID TrueID = 0;
- mcdc::ConditionID 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
@@ -688,14 +683,14 @@ struct MCDCCoverageBuilder {
private:
CodeGenModule &CGM;
- llvm::SmallVector<DecisionIDPair> DecisionStack;
+ llvm::SmallVector<mcdc::ConditionIDs> DecisionStack;
MCDC::State &MCDCState;
llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDs;
mcdc::ConditionID 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 mcdc::ConditionIDs DecisionStackSentinel{0, 0};
/// Is this a logical-AND operation?
bool isLAnd(const BinaryOperator *E) const {
@@ -732,7 +727,7 @@ struct MCDCCoverageBuilder {
}
/// Return the LHS Decision ([0,0] if not set).
- const DecisionIDPair &back() const { return DecisionStack.back(); }
+ const mcdc::ConditionIDs &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
@@ -750,7 +745,7 @@ struct MCDCCoverageBuilder {
if (NotMapped)
return;
- const DecisionIDPair &ParentDecision = DecisionStack.back();
+ const mcdc::ConditionIDs &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
@@ -766,18 +761,18 @@ struct MCDCCoverageBuilder {
// Push the LHS decision IDs onto the DecisionStack.
if (isLAnd(E))
- DecisionStack.push_back({RHSid, ParentDecision.FalseID});
+ DecisionStack.push_back({ParentDecision[false], RHSid});
else
- DecisionStack.push_back({ParentDecision.TrueID, RHSid});
+ DecisionStack.push_back({RHSid, ParentDecision[true]});
}
/// Pop and return the LHS Decision ([0,0] if not set).
- DecisionIDPair pop() {
+ mcdc::ConditionIDs pop() {
if (!CGM.getCodeGenOpts().MCDCCoverage || NotMapped)
return DecisionStack.front();
assert(DecisionStack.size() > 1);
- DecisionIDPair D = DecisionStack.back();
+ mcdc::ConditionIDs D = DecisionStack.back();
DecisionStack.pop_back();
return D;
}
@@ -1026,15 +1021,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 mcdc::ConditionIDs &Conds = {}) {
// Check for NULL conditions.
if (!C)
return;
@@ -1047,8 +1039,7 @@ struct CounterCoverageMappingBuilder
mcdc::Parameters BranchParams;
mcdc::ConditionID ID = MCDCBuilder.getCondID(C);
if (ID > 0)
- BranchParams =
- mcdc::BranchParameters{ID, IDPair.TrueID, IDPair.FalseID};
+ BranchParams = mcdc::BranchParameters{ID, Conds};
// If a condition can fold to true or false, the corresponding branch
// will be removed. Create a region with both counters hard-coded to
@@ -2134,8 +2125,8 @@ static void dump(llvm::raw_ostream &OS, StringRef FunctionName,
if (const auto *BranchParams =
std::get_if<mcdc::BranchParameters>(&R.MCDCParams)) {
- OS << " [" << BranchParams->ID << "," << BranchParams->TrueID;
- OS << "," << BranchParams->FalseID << "] ";
+ OS << " [" << BranchParams->ID << "," << BranchParams->Conds[true];
+ OS << "," << BranchParams->Conds[false] << "] ";
}
if (R.Kind == CounterMappingRegion::ExpansionRegion)
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index c6fbdb512b807e..e3b394287f3352 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -38,7 +38,6 @@
#include <sstream>
#include <string>
#include <system_error>
-#include <tuple>
#include <utility>
#include <vector>
diff --git a/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
index d7520fa0824243..61272174fef827 100644
--- a/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
+++ b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
@@ -13,12 +13,14 @@
#ifndef LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
#define LLVM_PROFILEDATA_COVERAGE_MCDCTYPES_H
+#include <array>
#include <variant>
namespace llvm::coverage::mcdc {
/// The ID for MCDCBranch.
using ConditionID = unsigned int;
+using ConditionIDs = std::array<ConditionID, 2>;
struct DecisionParameters {
/// Byte Index of Bitmap Coverage Object for a Decision Region.
@@ -35,11 +37,12 @@ struct DecisionParameters {
struct BranchParameters {
/// IDs used to represent a branch region and other branch regions
/// evaluated based on True and False branches.
- ConditionID ID, TrueID, FalseID;
+ ConditionID ID;
+ ConditionIDs Conds;
BranchParameters() = delete;
- BranchParameters(ConditionID ID, ConditionID TrueID, ConditionID FalseID)
- : ID(ID), TrueID(TrueID), FalseID(FalseID) {}
+ BranchParameters(ConditionID ID, const ConditionIDs &Conds)
+ : ID(ID), Conds(Conds) {}
};
/// The type of MC/DC-specific parameters.
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 80b80f7a26f454..9adeceb1faee2b 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -246,7 +246,7 @@ class MCDCRecordProcessor {
unsigned BitmapIdx;
/// Mapping of a condition ID to its corresponding branch params.
- llvm::DenseMap<unsigned, const mcdc::BranchParameters *> BranchParamsMap;
+ llvm::DenseMap<unsigned, mcdc::ConditionIDs> CondsMap;
/// Vector used to track whether a condition is constant folded.
MCDCRecord::BoolVector Folded;
@@ -269,38 +269,34 @@ class MCDCRecordProcessor {
Folded(NumConditions, false), IndependencePairs(NumConditions) {}
private:
- void recordTestVector(MCDCRecord::TestVector &TV, unsigned Index,
- MCDCRecord::CondState Result) {
- if (!Bitmap[BitmapIdx + Index])
- return;
-
- // Copy the completed test vector to the vector of testvectors.
- ExecVectors.push_back(TV);
-
- // The final value (T,F) is equal to the last non-dontcare state on the
- // path (in a short-circuiting system).
- ExecVectors.back().push_back(Result);
- }
-
// 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,
unsigned Index) {
- auto [UnusedID, TrueID, FalseID] = *BranchParamsMap[ID];
+ assert((Index & (1 << (ID - 1))) == 0);
+
+ for (auto MCDCCond : {MCDCRecord::MCDC_False, MCDCRecord::MCDC_True}) {
+ static_assert(MCDCRecord::MCDC_False == 0);
+ static_assert(MCDCRecord::MCDC_True == 1);
+ Index |= MCDCCond << (ID - 1);
+ TV[ID - 1] = MCDCCond;
+ auto NextID = CondsMap[ID][MCDCCond];
+ if (NextID > 0) {
+ buildTestVector(TV, NextID, Index);
+ continue;
+ }
- TV[ID - 1] = MCDCRecord::MCDC_False;
- if (FalseID > 0)
- buildTestVector(TV, FalseID, Index);
- else
- recordTestVector(TV, Index, MCDCRecord::MCDC_False);
+ if (!Bitmap[BitmapIdx + Index])
+ continue;
- Index |= 1 << (ID - 1);
- TV[ID - 1] = MCDCRecord::MCDC_True;
- if (TrueID > 0)
- buildTestVector(TV, TrueID, Index);
- else
- recordTestVector(TV, Index, MCDCRecord::MCDC_True);
+ // Copy the completed test vector to the vector of testvectors.
+ ExecVectors.push_back(TV);
+
+ // The final value (T,F) is equal to the last non-dontcare state on the
+ // path (in a short-circuiting system).
+ ExecVectors.back().push_back(MCDCCond);
+ }
// Reset back to DontCare.
TV[ID - 1] = MCDCRecord::MCDC_DontCare;
@@ -374,7 +370,7 @@ class MCDCRecordProcessor {
// from being measured.
for (const auto *B : Branches) {
const auto &BranchParams = B->getBranchParams();
- BranchParamsMap[BranchParams.ID] = &BranchParams;
+ CondsMap[BranchParams.ID] = BranchParams.Conds;
PosToID[I] = BranchParams.ID - 1;
CondLoc[I] = B->startLoc();
Folded[I++] = (B->Count.isZero() && B->FalseCount.isZero());
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
index d528d9aa95648f..de7be523ef33ca 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -313,9 +313,9 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray(
return make_error<CoverageMapError>(
coveragemap_error::malformed,
"MCDCConditionID shouldn't be zero");
- Params = mcdc::BranchParameters{static_cast<unsigned>(ID),
- static_cast<unsigned>(TID),
- static_cast<unsigned>(FID)};
+ Params = mcdc::BranchParameters{
+ static_cast<unsigned>(ID),
+ {static_cast<unsigned>(FID), static_cast<unsigned>(TID)}};
break;
case CounterMappingRegion::MCDCDecisionRegion:
Kind = CounterMappingRegion::MCDCDecisionRegion;
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
index 3267afdbe15c28..6125cce0fa4cd9 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
@@ -257,8 +257,8 @@ void CoverageMappingWriter::write(raw_ostream &OS) {
ParamsShouldBeNull = false;
assert(BranchParams.ID > 0);
encodeULEB128(static_cast<unsigned>(BranchParams.ID), OS);
- encodeULEB128(static_cast<unsigned>(BranchParams.TrueID), OS);
- encodeULEB128(static_cast<unsigned>(BranchParams.FalseID), OS);
+ encodeULEB128(static_cast<unsigned>(BranchParams.Conds[true]), OS);
+ encodeULEB128(static_cast<unsigned>(BranchParams.Conds[false]), OS);
}
break;
case CounterMappingRegion::MCDCDecisionRegion:
diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
index 6f6718fbd94591..db6689bc58839c 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -200,14 +200,13 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
mcdc::DecisionParameters{Mask, NC}, 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, mcdc::ConditionID ID,
+ mcdc::ConditionIDs 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, FileID, LS, CS, LE, CE,
- mcdc::BranchParameters{ID, TrueID, FalseID}));
+ C1, C2, FileID, LS, CS, LE, CE, mcdc::BranchParameters{ID, Conds}));
}
void addExpansionCMR(StringRef File, StringRef ExpandedFile, unsigned LS,
@@ -873,9 +872,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());
@@ -901,11 +900,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;
More information about the cfe-commits
mailing list