[llvm] [Coverage] Rework Decision/Expansion/Branch (PR #78969)
NAKAMURA Takumi via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 25 03:20:19 PST 2024
https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/78969
>From 5d9de08db9025b67aecfcce65cee0d866759ce06 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sun, 21 Jan 2024 20:02:13 +0900
Subject: [PATCH 1/5] [Coverage] Rework Decision/Expansion/Branch (#77871)
---
.../ProfileData/Coverage/CoverageMapping.cpp | 112 +++++++++++++++---
1 file changed, 97 insertions(+), 15 deletions(-)
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 9c95cd5d76d215..d8e1af84e898aa 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -14,6 +14,7 @@
#include "llvm/ProfileData/Coverage/CoverageMapping.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallBitVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
@@ -582,6 +583,72 @@ static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
return MaxBitmapID + (SizeInBits / CHAR_BIT);
}
+struct DecisionRow {
+ const CounterMappingRegion *DecisionRegion;
+ LineColPair DecisionStartLoc;
+ LineColPair DecisionEndLoc;
+
+ SmallVector<const CounterMappingRegion *, 6> Branches;
+ DenseSet<CounterMappingRegion::MCDCConditionID> IDs;
+ SmallVector<const CounterMappingRegion *> Expansions;
+
+ DecisionRow(const CounterMappingRegion &Decision)
+ : DecisionRegion(&Decision), DecisionStartLoc(Decision.startLoc()),
+ DecisionEndLoc(Decision.endLoc()) {}
+
+ bool insert(const CounterMappingRegion &Branch) {
+ auto ID = Branch.MCDCParams.ID;
+ if (ID == 1)
+ Branches.insert(Branches.begin(), &Branch);
+ else
+ Branches.push_back(&Branch);
+ IDs.insert(ID);
+ return (Branches.size() == DecisionRegion->MCDCParams.NumConditions);
+ }
+
+ enum class UpdateResult {
+ NotFound = 0,
+ Updated,
+ Committed,
+ };
+
+ UpdateResult updateBranch(const CounterMappingRegion &Branch) {
+ if (IDs.contains(Branch.MCDCParams.ID))
+ return UpdateResult::NotFound;
+
+ if (Branch.FileID == DecisionRegion->FileID &&
+ Branch.startLoc() >= DecisionStartLoc &&
+ Branch.endLoc() <= DecisionEndLoc)
+ return (insert(Branch) ? UpdateResult::Committed : UpdateResult::Updated);
+
+ for (const auto *R : Expansions) {
+ if (Branch.FileID == R->ExpandedFileID)
+ return (insert(Branch) ? UpdateResult::Committed
+ : UpdateResult::Updated);
+ }
+
+ return UpdateResult::NotFound;
+ }
+
+ bool updateExpansion(const CounterMappingRegion &Expansion) {
+ if (Expansion.FileID == DecisionRegion->FileID &&
+ Expansion.startLoc() >= DecisionStartLoc &&
+ Expansion.endLoc() <= DecisionEndLoc) {
+ Expansions.push_back(&Expansion);
+ return true;
+ }
+
+ for (const auto *R : Expansions) {
+ if (Expansion.FileID == R->ExpandedFileID) {
+ Expansions.push_back(&Expansion);
+ return true;
+ }
+ }
+
+ return false;
+ }
+};
+
Error CoverageMapping::loadFunctionRecord(
const CoverageMappingRecord &Record,
IndexedInstrProfReader &ProfileReader) {
@@ -638,18 +705,11 @@ Error CoverageMapping::loadFunctionRecord(
Record.MappingRegions[0].Count.isZero() && Counts[0] > 0)
return Error::success();
- unsigned NumConds = 0;
- const CounterMappingRegion *MCDCDecision;
- std::vector<const CounterMappingRegion *> MCDCBranches;
-
+ SmallVector<DecisionRow> Decisions;
FunctionRecord Function(OrigFuncName, Record.Filenames);
for (const auto &Region : Record.MappingRegions) {
- // If an MCDCDecisionRegion is seen, track the BranchRegions that follow
- // it according to Region.NumConditions.
if (Region.Kind == CounterMappingRegion::MCDCDecisionRegion) {
- assert(NumConds == 0);
- MCDCDecision = &Region;
- NumConds = Region.MCDCParams.NumConditions;
+ Decisions.emplace_back(Region);
continue;
}
Expected<int64_t> ExecutionCount = Ctx.evaluate(Region.Count);
@@ -664,23 +724,39 @@ Error CoverageMapping::loadFunctionRecord(
}
Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount);
+ if (Region.Kind == CounterMappingRegion::ExpansionRegion) {
+ for (auto &Decision : reverse(Decisions)) {
+ if (Decision.updateExpansion(Region))
+ break;
+ }
+ continue;
+ }
+
+ if (Region.Kind != CounterMappingRegion::MCDCBranchRegion)
+ continue;
+
// If a MCDCDecisionRegion was seen, store the BranchRegions that
// correspond to it in a vector, according to the number of conditions
// recorded for the region (tracked by NumConds).
- if (NumConds > 0 && Region.Kind == CounterMappingRegion::MCDCBranchRegion) {
- MCDCBranches.push_back(&Region);
+ for (int I = Decisions.size() - 1; I >= 0; --I) {
+ auto &Decision = Decisions[I];
// As we move through all of the MCDCBranchRegions that follow the
// MCDCDecisionRegion, decrement NumConds to make sure we account for
// them all before we calculate the bitmap of executed test vectors.
- if (--NumConds == 0) {
+ switch (Decision.updateBranch(Region)) {
+ case DecisionRow::UpdateResult::NotFound:
+ continue;
+ case DecisionRow::UpdateResult::Updated:
+ goto branch_found;
+ case DecisionRow::UpdateResult::Committed:
// Evaluating the test vector bitmap for the decision region entails
// calculating precisely what bits are pertinent to this region alone.
// This is calculated based on the recorded offset into the global
// profile bitmap; the length is calculated based on the recorded
// number of conditions.
Expected<BitVector> ExecutedTestVectorBitmap =
- Ctx.evaluateBitmap(MCDCDecision);
+ Ctx.evaluateBitmap(Decision.DecisionRegion);
if (auto E = ExecutedTestVectorBitmap.takeError()) {
consumeError(std::move(E));
return Error::success();
@@ -690,7 +766,8 @@ Error CoverageMapping::loadFunctionRecord(
// DecisionRegion, all of the information is now available to process.
// This is where the bulk of the MC/DC progressing takes place.
Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
- *MCDCDecision, *ExecutedTestVectorBitmap, MCDCBranches);
+ *Decision.DecisionRegion, *ExecutedTestVectorBitmap,
+ Decision.Branches);
if (auto E = Record.takeError()) {
consumeError(std::move(E));
return Error::success();
@@ -698,9 +775,14 @@ Error CoverageMapping::loadFunctionRecord(
// Save the MC/DC Record so that it can be visualized later.
Function.pushMCDCRecord(*Record);
- MCDCBranches.clear();
+
+ Decisions.erase(Decisions.begin() + I);
+ goto branch_found;
}
}
+ llvm_unreachable("Branch not found in Decisions");
+
+ branch_found:;
}
// Don't create records for (filenames, function) pairs we've already seen.
>From 1564424ec4018fc2a26a3263cbabba2a4a6cbec7 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Tue, 23 Jan 2024 14:48:36 +0900
Subject: [PATCH 2/5] Confirm that all Decisions have not been resolved.
---
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index d8e1af84e898aa..b8bff6df4ab0e9 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -785,6 +785,8 @@ Error CoverageMapping::loadFunctionRecord(
branch_found:;
}
+ assert(Decisions.empty() && "All Decisions have not been resolved");
+
// Don't create records for (filenames, function) pairs we've already seen.
auto FilenamesHash = hash_combine_range(Record.Filenames.begin(),
Record.Filenames.end());
>From f74dec7611aa997bb7973589aef06ef17d0dad11 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Tue, 23 Jan 2024 14:35:09 +0900
Subject: [PATCH 3/5] DecisionRow: Rework to reflect reviews
---
.../ProfileData/Coverage/CoverageMapping.cpp | 63 +++++++++----------
1 file changed, 28 insertions(+), 35 deletions(-)
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index b8bff6df4ab0e9..5aa25c8b8b11b4 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -596,56 +596,49 @@ struct DecisionRow {
: DecisionRegion(&Decision), DecisionStartLoc(Decision.startLoc()),
DecisionEndLoc(Decision.endLoc()) {}
- bool insert(const CounterMappingRegion &Branch) {
- auto ID = Branch.MCDCParams.ID;
- if (ID == 1)
- Branches.insert(Branches.begin(), &Branch);
- else
- Branches.push_back(&Branch);
- IDs.insert(ID);
- return (Branches.size() == DecisionRegion->MCDCParams.NumConditions);
+ bool inDecisionRegion(const CounterMappingRegion &R) {
+ return (R.FileID == DecisionRegion->FileID &&
+ R.startLoc() >= DecisionStartLoc && R.endLoc() <= DecisionEndLoc);
+ }
+
+ bool inExpansions(const CounterMappingRegion &R) {
+ return any_of(Expansions, [&](const auto &Expansion) {
+ return (Expansion->ExpandedFileID == R.FileID);
+ });
}
enum class UpdateResult {
NotFound = 0,
Updated,
- Committed,
+ Completed,
};
UpdateResult updateBranch(const CounterMappingRegion &Branch) {
- if (IDs.contains(Branch.MCDCParams.ID))
- return UpdateResult::NotFound;
+ auto ID = Branch.MCDCParams.ID;
- if (Branch.FileID == DecisionRegion->FileID &&
- Branch.startLoc() >= DecisionStartLoc &&
- Branch.endLoc() <= DecisionEndLoc)
- return (insert(Branch) ? UpdateResult::Committed : UpdateResult::Updated);
+ if (!IDs.contains(ID) &&
+ (inDecisionRegion(Branch) || inExpansions(Branch))) {
+ assert(Branches.size() < DecisionRegion->MCDCParams.NumConditions);
- for (const auto *R : Expansions) {
- if (Branch.FileID == R->ExpandedFileID)
- return (insert(Branch) ? UpdateResult::Committed
- : UpdateResult::Updated);
- }
+ if (ID == 1)
+ Branches.insert(Branches.begin(), &Branch);
+ else
+ Branches.push_back(&Branch);
- return UpdateResult::NotFound;
+ IDs.insert(ID);
+ return (Branches.size() == DecisionRegion->MCDCParams.NumConditions
+ ? UpdateResult::Completed
+ : UpdateResult::Updated);
+ } else
+ return UpdateResult::NotFound;
}
bool updateExpansion(const CounterMappingRegion &Expansion) {
- if (Expansion.FileID == DecisionRegion->FileID &&
- Expansion.startLoc() >= DecisionStartLoc &&
- Expansion.endLoc() <= DecisionEndLoc) {
+ if (inDecisionRegion(Expansion) || inExpansions(Expansion)) {
Expansions.push_back(&Expansion);
return true;
- }
-
- for (const auto *R : Expansions) {
- if (Expansion.FileID == R->ExpandedFileID) {
- Expansions.push_back(&Expansion);
- return true;
- }
- }
-
- return false;
+ } else
+ return false;
}
};
@@ -749,7 +742,7 @@ Error CoverageMapping::loadFunctionRecord(
continue;
case DecisionRow::UpdateResult::Updated:
goto branch_found;
- case DecisionRow::UpdateResult::Committed:
+ case DecisionRow::UpdateResult::Completed:
// Evaluating the test vector bitmap for the decision region entails
// calculating precisely what bits are pertinent to this region alone.
// This is calculated based on the recorded offset into the global
>From 425185ce38676d6b5ccd0e18c70038b28771291b Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Tue, 23 Jan 2024 21:46:06 +0900
Subject: [PATCH 4/5] Add comments to the upper half.
---
.../ProfileData/Coverage/CoverageMapping.cpp | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 5aa25c8b8b11b4..61984adae3f208 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -583,24 +583,37 @@ static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
return MaxBitmapID + (SizeInBits / CHAR_BIT);
}
+/// This holds the DecisionRegion and MCDCBranch(es) under it.
+/// Also traverses Expansion(s).
struct DecisionRow {
+ /// The subject
const CounterMappingRegion *DecisionRegion;
+
+ /// They are reflected from `DecisionRegion` for convenience.
LineColPair DecisionStartLoc;
LineColPair DecisionEndLoc;
+ /// This is passed to `MCDCRecordProcessor`, so this should be compatible to
+ /// `ArrayRef<const CounterMappingRegion *>`.
SmallVector<const CounterMappingRegion *, 6> Branches;
+
+ /// Each `ID` in `Branches` should be unique.
DenseSet<CounterMappingRegion::MCDCConditionID> IDs;
+
+ /// Relevant `Expansion`(s) should be caught to find expanded Branches.
SmallVector<const CounterMappingRegion *> Expansions;
DecisionRow(const CounterMappingRegion &Decision)
: DecisionRegion(&Decision), DecisionStartLoc(Decision.startLoc()),
DecisionEndLoc(Decision.endLoc()) {}
+ /// Determine whether `R` is included in `DecisionRegion`.
bool inDecisionRegion(const CounterMappingRegion &R) {
return (R.FileID == DecisionRegion->FileID &&
R.startLoc() >= DecisionStartLoc && R.endLoc() <= DecisionEndLoc);
}
+ /// Determin whether `R` is pointed by any of Expansions.
bool inExpansions(const CounterMappingRegion &R) {
return any_of(Expansions, [&](const auto &Expansion) {
return (Expansion->ExpandedFileID == R.FileID);
@@ -613,19 +626,26 @@ struct DecisionRow {
Completed,
};
+ /// Add `Branch` into the Decision
UpdateResult updateBranch(const CounterMappingRegion &Branch) {
auto ID = Branch.MCDCParams.ID;
+ assert(ID > 0 && "MCDCBranch.ID should begin with 1");
if (!IDs.contains(ID) &&
(inDecisionRegion(Branch) || inExpansions(Branch))) {
assert(Branches.size() < DecisionRegion->MCDCParams.NumConditions);
+ // Put `ID=1` in front of `Branches` for convenience
+ // even if `Branches` is not topological.
if (ID == 1)
Branches.insert(Branches.begin(), &Branch);
else
Branches.push_back(&Branch);
+ // Mark `ID` as `assigned`.
IDs.insert(ID);
+
+ // `Completed` when `Branches` is full
return (Branches.size() == DecisionRegion->MCDCParams.NumConditions
? UpdateResult::Completed
: UpdateResult::Updated);
@@ -633,6 +653,8 @@ struct DecisionRow {
return UpdateResult::NotFound;
}
+ /// Record `Expansion` if it is dominated to the Decision.
+ /// Each `Expansion` may nest.
bool updateExpansion(const CounterMappingRegion &Expansion) {
if (inDecisionRegion(Expansion) || inExpansions(Expansion)) {
Expansions.push_back(&Expansion);
@@ -702,6 +724,7 @@ Error CoverageMapping::loadFunctionRecord(
FunctionRecord Function(OrigFuncName, Record.Filenames);
for (const auto &Region : Record.MappingRegions) {
if (Region.Kind == CounterMappingRegion::MCDCDecisionRegion) {
+ // Start recording `Region` as the `Decision`
Decisions.emplace_back(Region);
continue;
}
>From 2fda236c5fe68fd16b0f35ff0e6e1719d8157219 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Thu, 25 Jan 2024 20:01:20 +0900
Subject: [PATCH 5/5] Fill up comments and revise
* Introduce `MCDCDecisionRecorder`.
* Sink `DecisionRecord` (was `DecisionRow`) into `MCDCDecisionRecorder` and make it private.
* Rename identifiers.
* Replace `Expansions` with `ExpandedFileIDs`
* Seek `Decisions` in ascent order.
---
.../ProfileData/Coverage/CoverageMapping.cpp | 310 ++++++++++--------
1 file changed, 182 insertions(+), 128 deletions(-)
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 61984adae3f208..da28affa788bb7 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -583,84 +583,169 @@ static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
return MaxBitmapID + (SizeInBits / CHAR_BIT);
}
-/// This holds the DecisionRegion and MCDCBranch(es) under it.
-/// Also traverses Expansion(s).
-struct DecisionRow {
- /// The subject
- const CounterMappingRegion *DecisionRegion;
-
- /// They are reflected from `DecisionRegion` for convenience.
- LineColPair DecisionStartLoc;
- LineColPair DecisionEndLoc;
-
- /// This is passed to `MCDCRecordProcessor`, so this should be compatible to
- /// `ArrayRef<const CounterMappingRegion *>`.
- SmallVector<const CounterMappingRegion *, 6> Branches;
-
- /// Each `ID` in `Branches` should be unique.
- DenseSet<CounterMappingRegion::MCDCConditionID> IDs;
-
- /// Relevant `Expansion`(s) should be caught to find expanded Branches.
- SmallVector<const CounterMappingRegion *> Expansions;
-
- DecisionRow(const CounterMappingRegion &Decision)
- : DecisionRegion(&Decision), DecisionStartLoc(Decision.startLoc()),
- DecisionEndLoc(Decision.endLoc()) {}
-
- /// Determine whether `R` is included in `DecisionRegion`.
- bool inDecisionRegion(const CounterMappingRegion &R) {
- return (R.FileID == DecisionRegion->FileID &&
- R.startLoc() >= DecisionStartLoc && R.endLoc() <= DecisionEndLoc);
- }
+/// Collect Decisions, Branchs, and Expansions and associate them.
+class MCDCDecisionRecorder {
- /// Determin whether `R` is pointed by any of Expansions.
- bool inExpansions(const CounterMappingRegion &R) {
- return any_of(Expansions, [&](const auto &Expansion) {
- return (Expansion->ExpandedFileID == R.FileID);
- });
- }
+private:
+ /// This holds the DecisionRegion and MCDCBranches under it.
+ /// Also traverses Expansion(s).
+ /// The Decision has the number of MCDCBranches and
+ struct DecisionRecord {
+ const CounterMappingRegion *DecisionRegion;
+
+ /// They are reflected from DecisionRegion for convenience.
+ LineColPair DecisionStartLoc;
+ LineColPair DecisionEndLoc;
+
+ /// This is passed to `MCDCRecordProcessor`, so this should be compatible
+ /// to`ArrayRef<const CounterMappingRegion *>`.
+ SmallVector<const CounterMappingRegion *> MCDCBranches;
+
+ /// IDs that are stored in MCDCBranches
+ /// Complete when all IDs (1 to NumConditions) are met.
+ DenseSet<CounterMappingRegion::MCDCConditionID> ConditionIDs;
+
+ /// Set of IDs of Expansion(s) that are relevant to DecisionRegion
+ /// and its children (via expansions).
+ /// FileID pointed by ExpandedFileID is dedicated to the expansion, so
+ /// the location in the expansion doesn't matter.
+ DenseSet<unsigned> ExpandedFileIDs;
+
+ DecisionRecord(const CounterMappingRegion &Decision)
+ : DecisionRegion(&Decision), DecisionStartLoc(Decision.startLoc()),
+ DecisionEndLoc(Decision.endLoc()) {
+ assert(Decision.Kind == CounterMappingRegion::MCDCDecisionRegion);
+ }
- enum class UpdateResult {
- NotFound = 0,
- Updated,
- Completed,
- };
+ /// Determine whether DecisionRecord dominates `R`.
+ bool dominates(const CounterMappingRegion &R) {
+ // Determine whether `R` is included in `DecisionRegion`.
+ if (R.FileID == DecisionRegion->FileID &&
+ R.startLoc() >= DecisionStartLoc && R.endLoc() <= DecisionEndLoc)
+ return true;
+
+ // Determine whether `R` is pointed by any of Expansions.
+ return ExpandedFileIDs.contains(R.FileID);
+ }
+
+ enum Result {
+ NotProcessed = 0, /// Irrelevant to this Decision
+ Processed, /// Added to this Decision
+ Completed, /// Added and filled this Decision
+ };
- /// Add `Branch` into the Decision
- UpdateResult updateBranch(const CounterMappingRegion &Branch) {
- auto ID = Branch.MCDCParams.ID;
- assert(ID > 0 && "MCDCBranch.ID should begin with 1");
+ /// Add Branch into the Decision
+ /// \param Branch expects MCDCBranchRegion
+ /// \returns NotProcessed/Processed/Completed
+ Result addBranch(const CounterMappingRegion &Branch) {
+ assert(Branch.Kind == CounterMappingRegion::MCDCBranchRegion);
- if (!IDs.contains(ID) &&
- (inDecisionRegion(Branch) || inExpansions(Branch))) {
- assert(Branches.size() < DecisionRegion->MCDCParams.NumConditions);
+ auto ConditionID = Branch.MCDCParams.ID;
+ assert(ConditionID > 0 && "ConditionID should begin with 1");
- // Put `ID=1` in front of `Branches` for convenience
- // even if `Branches` is not topological.
- if (ID == 1)
- Branches.insert(Branches.begin(), &Branch);
+ if (ConditionIDs.contains(ConditionID) ||
+ ConditionID > DecisionRegion->MCDCParams.NumConditions)
+ return NotProcessed;
+
+ if (!this->dominates(Branch))
+ return NotProcessed;
+
+ assert(MCDCBranches.size() < DecisionRegion->MCDCParams.NumConditions);
+
+ // Put `ID=1` in front of `MCDCBranches` for convenience
+ // even if `MCDCBranches` is not topological.
+ if (ConditionID == 1)
+ MCDCBranches.insert(MCDCBranches.begin(), &Branch);
else
- Branches.push_back(&Branch);
+ MCDCBranches.push_back(&Branch);
// Mark `ID` as `assigned`.
- IDs.insert(ID);
-
- // `Completed` when `Branches` is full
- return (Branches.size() == DecisionRegion->MCDCParams.NumConditions
- ? UpdateResult::Completed
- : UpdateResult::Updated);
- } else
- return UpdateResult::NotFound;
- }
+ ConditionIDs.insert(ConditionID);
+
+ // `Completed` when `MCDCBranches` is full
+ return (MCDCBranches.size() == DecisionRegion->MCDCParams.NumConditions
+ ? Completed
+ : Processed);
+ }
+
+ /// Record Expansion if it is relevant to this Decision.
+ /// Each `Expansion` may nest.
+ /// \returns true if recorded.
+ bool recordExpansion(const CounterMappingRegion &Expansion) {
+ if (!this->dominates(Expansion))
+ return false;
- /// Record `Expansion` if it is dominated to the Decision.
- /// Each `Expansion` may nest.
- bool updateExpansion(const CounterMappingRegion &Expansion) {
- if (inDecisionRegion(Expansion) || inExpansions(Expansion)) {
- Expansions.push_back(&Expansion);
+ ExpandedFileIDs.insert(Expansion.ExpandedFileID);
return true;
- } else
+ }
+ };
+
+private:
+ /// Decisions in progress
+ /// DecisionRecord is added for each MCDCDecisionRegion.
+ /// DecisionRecord is removed when Decision is completed.
+ SmallVector<DecisionRecord> Decisions;
+
+public:
+ ~MCDCDecisionRecorder() {
+ assert(Decisions.empty() && "All Decisions have not been resolved");
+ }
+
+ /// Register Region and start recording if it is MCDCDecisionRegion.
+ /// \param Region to be inspected
+ /// \returns true if recording started.
+ bool registerDecision(const CounterMappingRegion &Region) {
+ if (Region.Kind != CounterMappingRegion::MCDCDecisionRegion)
return false;
+
+ // Start recording Region to create DecisionRecord
+ Decisions.emplace_back(Region);
+ return true;
+ }
+
+ using DecisionAndBranches =
+ std::pair<const CounterMappingRegion *, /// Decision
+ SmallVector<const CounterMappingRegion *> /// Branches
+ >;
+
+ /// If Region is ExpansionRegion, record it.
+ /// If Region is MCDCBranchRegion, add it to DecisionRecord.
+ /// \param Region to be inspected
+ /// \returns DecisionsAndBranches if DecisionRecord completed.
+ /// Or returns nullopt.
+ std::optional<DecisionAndBranches>
+ processRegion(const CounterMappingRegion &Region) {
+
+ // Record ExpansionRegion.
+ if (Region.Kind == CounterMappingRegion::ExpansionRegion) {
+ for (auto &Decision : reverse(Decisions)) {
+ if (Decision.recordExpansion(Region))
+ break;
+ }
+ return std::nullopt; // It doesn't complete.
+ }
+
+ // Do nothing unless MCDCBranchRegion.
+ if (Region.Kind != CounterMappingRegion::MCDCBranchRegion)
+ return std::nullopt;
+
+ // Seek each Decision and apply Region to it.
+ for (auto DecisionIter = Decisions.begin(), DecisionEnd = Decisions.end();
+ DecisionIter != DecisionEnd; ++DecisionIter)
+ switch (DecisionIter->addBranch(Region)) {
+ case DecisionRecord::NotProcessed:
+ continue;
+ case DecisionRecord::Processed:
+ return std::nullopt;
+ case DecisionRecord::Completed:
+ DecisionAndBranches Result =
+ std::make_pair(DecisionIter->DecisionRegion,
+ std::move(DecisionIter->MCDCBranches));
+ Decisions.erase(DecisionIter); // No longer used.
+ return Result;
+ }
+
+ llvm_unreachable("Branch not found in Decisions");
}
};
@@ -720,14 +805,13 @@ Error CoverageMapping::loadFunctionRecord(
Record.MappingRegions[0].Count.isZero() && Counts[0] > 0)
return Error::success();
- SmallVector<DecisionRow> Decisions;
+ MCDCDecisionRecorder MCDCDecisions;
FunctionRecord Function(OrigFuncName, Record.Filenames);
for (const auto &Region : Record.MappingRegions) {
- if (Region.Kind == CounterMappingRegion::MCDCDecisionRegion) {
- // Start recording `Region` as the `Decision`
- Decisions.emplace_back(Region);
+ // MCDCDecisionRegion should be handled first since it overlaps with
+ // others inside.
+ if (MCDCDecisions.registerDecision(Region))
continue;
- }
Expected<int64_t> ExecutionCount = Ctx.evaluate(Region.Count);
if (auto E = ExecutionCount.takeError()) {
consumeError(std::move(E));
@@ -740,69 +824,39 @@ Error CoverageMapping::loadFunctionRecord(
}
Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount);
- if (Region.Kind == CounterMappingRegion::ExpansionRegion) {
- for (auto &Decision : reverse(Decisions)) {
- if (Decision.updateExpansion(Region))
- break;
- }
+ auto Result = MCDCDecisions.processRegion(Region);
+ if (!Result) // Any Decision doesn't complete.
continue;
- }
- if (Region.Kind != CounterMappingRegion::MCDCBranchRegion)
- continue;
-
- // If a MCDCDecisionRegion was seen, store the BranchRegions that
- // correspond to it in a vector, according to the number of conditions
- // recorded for the region (tracked by NumConds).
- for (int I = Decisions.size() - 1; I >= 0; --I) {
- auto &Decision = Decisions[I];
-
- // As we move through all of the MCDCBranchRegions that follow the
- // MCDCDecisionRegion, decrement NumConds to make sure we account for
- // them all before we calculate the bitmap of executed test vectors.
- switch (Decision.updateBranch(Region)) {
- case DecisionRow::UpdateResult::NotFound:
- continue;
- case DecisionRow::UpdateResult::Updated:
- goto branch_found;
- case DecisionRow::UpdateResult::Completed:
- // Evaluating the test vector bitmap for the decision region entails
- // calculating precisely what bits are pertinent to this region alone.
- // This is calculated based on the recorded offset into the global
- // profile bitmap; the length is calculated based on the recorded
- // number of conditions.
- Expected<BitVector> ExecutedTestVectorBitmap =
- Ctx.evaluateBitmap(Decision.DecisionRegion);
- if (auto E = ExecutedTestVectorBitmap.takeError()) {
- consumeError(std::move(E));
- return Error::success();
- }
-
- // Since the bitmap identifies the executed test vectors for an MC/DC
- // DecisionRegion, all of the information is now available to process.
- // This is where the bulk of the MC/DC progressing takes place.
- Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
- *Decision.DecisionRegion, *ExecutedTestVectorBitmap,
- Decision.Branches);
- if (auto E = Record.takeError()) {
- consumeError(std::move(E));
- return Error::success();
- }
-
- // Save the MC/DC Record so that it can be visualized later.
- Function.pushMCDCRecord(*Record);
+ auto MCDCDecision = Result->first;
+ auto &MCDCBranches = Result->second;
+
+ // Evaluating the test vector bitmap for the decision region entails
+ // calculating precisely what bits are pertinent to this region alone.
+ // This is calculated based on the recorded offset into the global
+ // profile bitmap; the length is calculated based on the recorded
+ // number of conditions.
+ Expected<BitVector> ExecutedTestVectorBitmap =
+ Ctx.evaluateBitmap(MCDCDecision);
+ if (auto E = ExecutedTestVectorBitmap.takeError()) {
+ consumeError(std::move(E));
+ return Error::success();
+ }
- Decisions.erase(Decisions.begin() + I);
- goto branch_found;
- }
+ // Since the bitmap identifies the executed test vectors for an MC/DC
+ // DecisionRegion, all of the information is now available to process.
+ // This is where the bulk of the MC/DC progressing takes place.
+ Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
+ *MCDCDecision, *ExecutedTestVectorBitmap, MCDCBranches);
+ if (auto E = Record.takeError()) {
+ consumeError(std::move(E));
+ return Error::success();
}
- llvm_unreachable("Branch not found in Decisions");
- branch_found:;
+ // Save the MC/DC Record so that it can be visualized later.
+ Function.pushMCDCRecord(*Record);
}
- assert(Decisions.empty() && "All Decisions have not been resolved");
-
// Don't create records for (filenames, function) pairs we've already seen.
auto FilenamesHash = hash_combine_range(Record.Filenames.begin(),
Record.Filenames.end());
More information about the llvm-commits
mailing list