diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 3b711c05e9275..93c3c31e71fa8 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 DecisionStack; + llvm::SmallVector DecisionStack; MCDC::State &MCDCState; llvm::DenseMap &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(&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 c6fbdb512b807..e3b394287f335 100644 --- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h +++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h @@ -38,7 +38,6 @@ #include #include #include -#include #include #include diff --git a/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h b/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h index d7520fa082424..61272174fef82 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 #include namespace llvm::coverage::mcdc { /// The ID for MCDCBranch. using ConditionID = unsigned int; +using ConditionIDs = std::array; 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 80b80f7a26f45..9adeceb1faee2 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 BranchParamsMap; + llvm::DenseMap 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 d528d9aa95648..de7be523ef33c 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( coveragemap_error::malformed, "MCDCConditionID shouldn't be zero"); - Params = mcdc::BranchParameters{static_cast(ID), - static_cast(TID), - static_cast(FID)}; + Params = mcdc::BranchParameters{ + static_cast(ID), + {static_cast(FID), static_cast(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 3267afdbe15c2..6125cce0fa4cd 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(BranchParams.ID), OS); - encodeULEB128(static_cast(BranchParams.TrueID), OS); - encodeULEB128(static_cast(BranchParams.FalseID), OS); + encodeULEB128(static_cast(BranchParams.Conds[true]), OS); + encodeULEB128(static_cast(BranchParams.Conds[false]), OS); } break; case CounterMappingRegion::MCDCDecisionRegion: diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp index 6f6718fbd9459..db6689bc58839 100644 --- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp +++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp @@ -200,14 +200,13 @@ struct CoverageMappingTest : ::testing::TestWithParam> { 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;