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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
