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`. + SmallVector MCDCBranches; + + /// IDs that are stored in MCDCBranches + /// Complete when all IDs (1 to NumConditions) are met. + DenseSet 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 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 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 /// Branches + >; + + /// Add MCDCBranchRegion to DecisionRecord. + /// \param Branch to be processed + /// \returns DecisionsAndBranches if DecisionRecord completed. + /// Or returns nullopt. + std::optional + 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 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 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 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 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 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 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/lib/ProfileData/Coverage/CoverageMappingWriter.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp index 1c7d8a8909c48..27727f216b051 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp @@ -167,7 +167,15 @@ void CoverageMappingWriter::write(raw_ostream &OS) { return LHS.FileID < RHS.FileID; if (LHS.startLoc() != RHS.startLoc()) return LHS.startLoc() < RHS.startLoc(); - return LHS.Kind < RHS.Kind; + + // Put `Decision` before `Expansion`. + auto getKindKey = [](CounterMappingRegion::RegionKind Kind) { + return (Kind == CounterMappingRegion::MCDCDecisionRegion + ? 2 * CounterMappingRegion::ExpansionRegion - 1 + : 2 * Kind); + }; + + return getKindKey(LHS.Kind) < getKindKey(RHS.Kind); }); // Write out the fileid -> filename mapping. 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 differ 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 diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp index 23f66a0232ddb..2849781a9dc43 100644 --- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp +++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp @@ -890,6 +890,42 @@ TEST_P(CoverageMappingTest, non_code_region_bitmask) { ASSERT_EQ(1U, Names.size()); } +// Test the order of MCDCDecision before Expansion +TEST_P(CoverageMappingTest, decision_before_expansion) { + startFunction("foo", 0x1234); + addCMR(Counter::getCounter(0), "foo", 3, 23, 5, 2); + + // This(4:11) was put after Expansion(4:11) before the fix + addMCDCDecisionCMR(0, 2, "foo", 4, 11, 4, 20); + + addExpansionCMR("foo", "A", 4, 11, 4, 12); + 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); + addCMR(Counter::getCounter(1), "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; + + writeAndReadCoverageRegions(); + + const auto &OutputRegions = OutputFunctions.back().Regions; + + size_t N = ArrayRef(InputRegions).size(); + ASSERT_EQ(N, OutputRegions.size()); + for (size_t I = 0; I < N; ++I) { + ASSERT_EQ(InputRegions[I].Kind, OutputRegions[I].Kind); + ASSERT_EQ(InputRegions[I].FileID, OutputRegions[I].FileID); + ASSERT_EQ(InputRegions[I].ExpandedFileID, OutputRegions[I].ExpandedFileID); + ASSERT_EQ(InputRegions[I].startLoc(), OutputRegions[I].startLoc()); + ASSERT_EQ(InputRegions[I].endLoc(), OutputRegions[I].endLoc()); + } +} + TEST_P(CoverageMappingTest, strip_filename_prefix) { ProfileWriter.addRecord({"file1:func", 0x1234, {0}}, Err);