[llvm-branch-commits] [llvm] b50a84e - [Coverage] Let `Decision` take account of expansions (#78969)
Tom Stellard via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Feb 5 11:33:31 PST 2024
Author: NAKAMURA Takumi
Date: 2024-02-05T11:32:51-08:00
New Revision: b50a84e303378df35996d7330aa80aa4ea1f497a
URL: https://github.com/llvm/llvm-project/commit/b50a84e303378df35996d7330aa80aa4ea1f497a
DIFF: https://github.com/llvm/llvm-project/commit/b50a84e303378df35996d7330aa80aa4ea1f497a.diff
LOG: [Coverage] Let `Decision` take account of expansions (#78969)
The current implementation (D138849) assumes `Branch`(es) would follow
after the corresponding `Decision`. It is not true if `Branch`(es) are
forwarded to expanded file ID. As a result, consecutive `Decision`(s)
would be confused with insufficient number of `Branch`(es).
`Expansion` will point `Branch`(es) in other file IDs if `Expansion` is
included in the range of `Decision`.
Fixes #77871
---------
Co-authored-by: Alan Phipps <a-phipps at ti.com>
(cherry picked from commit d912f1f0cb49465b08f82fae89ece222404e5640)
Added:
llvm/test/tools/llvm-cov/Inputs/mcdc-macro.c
llvm/test/tools/llvm-cov/Inputs/mcdc-macro.o
llvm/test/tools/llvm-cov/Inputs/mcdc-macro.proftext
llvm/test/tools/llvm-cov/mcdc-macro.test
Modified:
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
Removed:
################################################################################
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index da8e1d87319dd..a357b4cb49211 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"
@@ -583,6 +584,160 @@ static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
return MaxBitmapID + (SizeInBits / CHAR_BIT);
}
+namespace {
+
+/// Collect Decisions, Branchs, and Expansions and associate them.
+class MCDCDecisionRecorder {
+private:
+ /// This holds the DecisionRegion and MCDCBranches under it.
+ /// Also traverses Expansion(s).
+ /// The Decision has the number of MCDCBranches and will complete
+ /// when it is filled with unique ConditionID of MCDCBranches.
+ 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);
+ }
+
+ /// Determine whether DecisionRecord dominates `R`.
+ bool dominates(const CounterMappingRegion &R) const {
+ // 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
+ /// \param Branch expects MCDCBranchRegion
+ /// \returns NotProcessed/Processed/Completed
+ Result addBranch(const CounterMappingRegion &Branch) {
+ assert(Branch.Kind == CounterMappingRegion::MCDCBranchRegion);
+
+ auto ConditionID = Branch.MCDCParams.ID;
+ assert(ConditionID > 0 && "ConditionID should begin with 1");
+
+ 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
+ MCDCBranches.push_back(&Branch);
+
+ // Mark `ID` as `assigned`.
+ 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;
+
+ ExpandedFileIDs.insert(Expansion.ExpandedFileID);
+ return true;
+ }
+ };
+
+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.
+ void registerDecision(const CounterMappingRegion &Decision) {
+ Decisions.emplace_back(Decision);
+ }
+
+ void recordExpansion(const CounterMappingRegion &Expansion) {
+ any_of(Decisions, [&Expansion](auto &Decision) {
+ return Decision.recordExpansion(Expansion);
+ });
+ }
+
+ using DecisionAndBranches =
+ std::pair<const CounterMappingRegion *, /// Decision
+ SmallVector<const CounterMappingRegion *> /// Branches
+ >;
+
+ /// Add MCDCBranchRegion to DecisionRecord.
+ /// \param Branch to be processed
+ /// \returns DecisionsAndBranches if DecisionRecord completed.
+ /// Or returns nullopt.
+ std::optional<DecisionAndBranches>
+ processBranch(const CounterMappingRegion &Branch) {
+ // Seek each Decision and apply Region to it.
+ for (auto DecisionIter = Decisions.begin(), DecisionEnd = Decisions.end();
+ DecisionIter != DecisionEnd; ++DecisionIter)
+ switch (DecisionIter->addBranch(Branch)) {
+ 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");
+ }
+};
+
+} // namespace
+
Error CoverageMapping::loadFunctionRecord(
const CoverageMappingRecord &Record,
IndexedInstrProfReader &ProfileReader) {
@@ -639,18 +794,13 @@ 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;
-
+ MCDCDecisionRecorder MCDCDecisions;
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.
+ // MCDCDecisionRegion should be handled first since it overlaps with
+ // others inside.
if (Region.Kind == CounterMappingRegion::MCDCDecisionRegion) {
- assert(NumConds == 0);
- MCDCDecision = &Region;
- NumConds = Region.MCDCParams.NumConditions;
+ MCDCDecisions.registerDecision(Region);
continue;
}
Expected<int64_t> ExecutionCount = Ctx.evaluate(Region.Count);
@@ -665,43 +815,47 @@ Error CoverageMapping::loadFunctionRecord(
}
Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount);
- // 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);
-
- // 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) {
- // 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();
- }
+ // Record ExpansionRegion.
+ if (Region.Kind == CounterMappingRegion::ExpansionRegion) {
+ MCDCDecisions.recordExpansion(Region);
+ continue;
+ }
- // 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();
- }
+ // Do nothing unless MCDCBranchRegion.
+ if (Region.Kind != CounterMappingRegion::MCDCBranchRegion)
+ continue;
- // Save the MC/DC Record so that it can be visualized later.
- Function.pushMCDCRecord(*Record);
- MCDCBranches.clear();
- }
+ auto Result = MCDCDecisions.processBranch(Region);
+ if (!Result) // Any Decision doesn't complete.
+ continue;
+
+ 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();
}
+
+ // 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();
+ }
+
+ // Save the MC/DC Record so that it can be visualized later.
+ Function.pushMCDCRecord(*Record);
}
// Don't create records for (filenames, function) pairs we've already seen.
diff --git a/llvm/test/tools/llvm-cov/Inputs/mcdc-macro.c b/llvm/test/tools/llvm-cov/Inputs/mcdc-macro.c
new file mode 100644
index 0000000000000..bd2b979bd257f
--- /dev/null
+++ b/llvm/test/tools/llvm-cov/Inputs/mcdc-macro.c
@@ -0,0 +1,20 @@
+#define C c
+#define D 1
+#define E (C != a) && (C > a)
+#define F E
+
+void __attribute__((noinline)) func1(void) { return; }
+
+void __attribute__((noinline)) func(int a, int b, int c) {
+ if (a && D && E || b)
+ func1();
+ if (b && D)
+ func1();
+ if (a && (b && C) || (D && F))
+ func1();
+}
+
+int main() {
+ func(2, 3, 3);
+ return 0;
+}
diff --git a/llvm/test/tools/llvm-cov/Inputs/mcdc-macro.o b/llvm/test/tools/llvm-cov/Inputs/mcdc-macro.o
new file mode 100644
index 0000000000000..667ccd132d2fb
Binary files /dev/null and b/llvm/test/tools/llvm-cov/Inputs/mcdc-macro.o
diff er
diff --git a/llvm/test/tools/llvm-cov/Inputs/mcdc-macro.proftext b/llvm/test/tools/llvm-cov/Inputs/mcdc-macro.proftext
new file mode 100644
index 0000000000000..35ecc42b5802a
--- /dev/null
+++ b/llvm/test/tools/llvm-cov/Inputs/mcdc-macro.proftext
@@ -0,0 +1,62 @@
+func
+# Func Hash:
+395201011017399473
+# Num Counters:
+22
+# Counter Values:
+1
+1
+0
+0
+1
+1
+1
+1
+1
+1
+1
+1
+1
+1
+0
+1
+1
+1
+0
+0
+0
+0
+# Num Bitmap Bytes:
+$13
+# Bitmap Byte Values:
+0x0
+0x0
+0x0
+0x20
+0x8
+0x0
+0x20
+0x0
+0x0
+0x0
+0x0
+0x0
+0x0
+
+
+func1
+# Func Hash:
+24
+# Num Counters:
+1
+# Counter Values:
+3
+
+main
+# Func Hash:
+24
+# Num Counters:
+1
+# Counter Values:
+1
+
diff --git a/llvm/test/tools/llvm-cov/mcdc-macro.test b/llvm/test/tools/llvm-cov/mcdc-macro.test
new file mode 100644
index 0000000000000..339284bba2c9b
--- /dev/null
+++ b/llvm/test/tools/llvm-cov/mcdc-macro.test
@@ -0,0 +1,99 @@
+// Test visualization of MC/DC constructs for branches in macro expansions.
+
+// RUN: llvm-profdata merge %S/Inputs/mcdc-macro.proftext -o %t.profdata
+// RUN: llvm-cov show --show-expansions --show-branches=count --show-mcdc %S/Inputs/mcdc-macro.o -instr-profile %t.profdata --compilation-dir=%S/Inputs | FileCheck %s
+
+// CHECK: | | | Branch (2:11): [Folded - Ignored]
+// CHECK: | | | Branch (3:11): [True: 1, False: 0]
+// CHECK: | | | Branch (3:23): [True: 1, False: 0]
+// CHECK: | Branch (9:7): [True: 1, False: 0]
+// CHECK-NEXT: | Branch (9:22): [True: 0, False: 0]
+// CHECK-NEXT: ------------------
+// CHECK-NEXT: |---> MC/DC Decision Region (9:7) to (9:23)
+// CHECK-NEXT: |
+// CHECK-NEXT: | Number of Conditions: 5
+// CHECK-NEXT: | Condition C1 --> (9:7)
+// CHECK-NEXT: | Condition C2 --> (9:22)
+// CHECK-NEXT: | Condition C3 --> (2:11)
+// CHECK-NEXT: | Condition C4 --> (3:11)
+// CHECK-NEXT: | Condition C5 --> (3:23)
+// CHECK-NEXT: |
+// CHECK-NEXT: | Executed MC/DC Test Vectors:
+// CHECK-NEXT: |
+// CHECK-NEXT: | C1, C2, C3, C4, C5 Result
+// CHECK-NEXT: | 1 { T, -, C, T, T = T }
+// CHECK-NEXT: |
+// CHECK-NEXT: | C1-Pair: not covered
+// CHECK-NEXT: | C2-Pair: not covered
+// CHECK-NEXT: | C3-Pair: constant folded
+// CHECK-NEXT: | C4-Pair: not covered
+// CHECK-NEXT: | C5-Pair: not covered
+// CHECK-NEXT: | MC/DC Coverage for Decision: 0.00%
+// CHECK-NEXT: |
+// CHECK-NEXT: ------------------
+
+// CHECK: | | | Branch (2:11): [Folded - Ignored]
+// CHECK: | Branch (11:7): [True: 1, False: 0]
+// CHECK-NEXT: ------------------
+// CHECK-NEXT: |---> MC/DC Decision Region (11:7) to (11:13)
+// CHECK-NEXT: |
+// CHECK-NEXT: | Number of Conditions: 2
+// CHECK-NEXT: | Condition C1 --> (11:7)
+// CHECK-NEXT: | Condition C2 --> (2:11)
+// CHECK-NEXT: |
+// CHECK-NEXT: | Executed MC/DC Test Vectors:
+// CHECK-NEXT: |
+// CHECK-NEXT: | C1, C2 Result
+// CHECK-NEXT: | 1 { T, C = T }
+// CHECK-NEXT: |
+// CHECK-NEXT: | C1-Pair: not covered
+// CHECK-NEXT: | C2-Pair: constant folded
+// CHECK-NEXT: | MC/DC Coverage for Decision: 0.00%
+// CHECK-NEXT: |
+// CHECK-NEXT: ------------------
+
+// CHECK: | | | Branch (1:11): [True: 1, False: 0]
+// CHECK: | | | Branch (2:11): [Folded - Ignored]
+// CHECK: | | | | | Branch (3:11): [True: 0, False: 0]
+// CHECK: | | | | | Branch (3:23): [True: 0, False: 0]
+// CHECK: | Branch (13:7): [True: 1, False: 0]
+// CHECK-NEXT: | Branch (13:13): [True: 1, False: 0]
+// CHECK-NEXT: ------------------
+// CHECK-NEXT: |---> MC/DC Decision Region (13:7) to (13:32)
+// CHECK-NEXT: |
+// CHECK-NEXT: | Number of Conditions: 6
+// CHECK-NEXT: | Condition C1 --> (13:7)
+// CHECK-NEXT: | Condition C2 --> (13:13)
+// CHECK-NEXT: | Condition C3 --> (1:11)
+// CHECK-NEXT: | Condition C4 --> (2:11)
+// CHECK-NEXT: | Condition C5 --> (3:11)
+// CHECK-NEXT: | Condition C6 --> (3:23)
+// CHECK-NEXT: |
+// CHECK-NEXT: | Executed MC/DC Test Vectors:
+// CHECK-NEXT: |
+// CHECK-NEXT: | C1, C2, C3, C4, C5, C6 Result
+// CHECK-NEXT: | 1 { T, T, T, C, -, - = T }
+// CHECK-NEXT: |
+// CHECK-NEXT: | C1-Pair: not covered
+// CHECK-NEXT: | C2-Pair: not covered
+// CHECK-NEXT: | C3-Pair: not covered
+// CHECK-NEXT: | C4-Pair: constant folded
+// CHECK-NEXT: | C5-Pair: not covered
+// CHECK-NEXT: | C6-Pair: not covered
+// CHECK-NEXT: | MC/DC Coverage for Decision: 0.00%
+// CHECK-NEXT: |
+// CHECK-NEXT: ------------------
+
+Instructions for regenerating the test:
+
+cd %S/Inputs # Or copy mcdc-macro.c into the working directory
+
+clang -fcoverage-mcdc -fprofile-instr-generate -fcoverage-compilation-dir=. \
+ -O3 -mllvm -enable-name-compression=false \
+ -fcoverage-mapping mcdc-macro.c -c
+
+# Instructions for generating proftext
+clang -fprofile-instr-generate mcdc-macro.o
+./a.out
+llvm-profdata merge --sparse -o default.profdata default.profraw
+llvm-profdata merge --text -o mcdc-macro.proftext default.profdata
More information about the llvm-branch-commits
mailing list