llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: NAKAMURA Takumi (chapuni) <details> <summary>Changes</summary> Also, Let NumConditions uint16_t --- Full diff: https://github.com/llvm/llvm-project/pull/81257.diff 9 Files Affected: - (modified) clang/lib/CodeGen/CodeGenPGO.cpp (+4-4) - (modified) clang/lib/CodeGen/CodeGenPGO.h (+1-1) - (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+15-15) - (modified) clang/lib/CodeGen/CoverageMappingGen.h (+2-2) - (modified) llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h (+5-5) - (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+13-12) - (modified) llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp (+9-8) - (modified) llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp (+3-3) - (modified) llvm/unittests/ProfileData/CoverageMappingTest.cpp (+9-9) ``````````diff diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 5d7c3847745762..9025889f443b88 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -1033,7 +1033,7 @@ void CodeGenPGO::emitCounterRegionMapping(const Decl *D) { std::string CoverageMapping; llvm::raw_string_ostream OS(CoverageMapping); - RegionCondIDMap.reset(new llvm::DenseMap<const Stmt *, unsigned>); + RegionCondIDMap.reset(new llvm::DenseMap<const Stmt *, int16_t>); CoverageMappingGen MappingGen( *CGM.getCoverageMapping(), CGM.getContext().getSourceManager(), CGM.getLangOpts(), RegionCounterMap.get(), RegionMCDCBitmapMap.get(), @@ -1198,8 +1198,8 @@ void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy &Builder, const Expr *S, return; // Extract the ID of the condition we are setting in the bitmap. - unsigned CondID = ExprMCDCConditionIDMapIterator->second; - assert(CondID > 0 && "Condition has no ID!"); + auto CondID = ExprMCDCConditionIDMapIterator->second; + assert(CondID >= 0 && "Condition has no ID!"); auto *I8PtrTy = llvm::PointerType::getUnqual(CGM.getLLVMContext()); @@ -1208,7 +1208,7 @@ void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy &Builder, const Expr *S, // the resulting value is used to update the boolean expression's bitmap. llvm::Value *Args[5] = {llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy), Builder.getInt64(FunctionHash), - Builder.getInt32(CondID - 1), + Builder.getInt32(CondID), MCDCCondBitmapAddr.getPointer(), Val}; Builder.CreateCall( CGM.getIntrinsic(llvm::Intrinsic::instrprof_mcdc_condbitmap_update), diff --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h index 6596b6c3527764..5f2941cfb2e95e 100644 --- a/clang/lib/CodeGen/CodeGenPGO.h +++ b/clang/lib/CodeGen/CodeGenPGO.h @@ -37,7 +37,7 @@ class CodeGenPGO { uint64_t FunctionHash; std::unique_ptr<llvm::DenseMap<const Stmt *, unsigned>> RegionCounterMap; std::unique_ptr<llvm::DenseMap<const Stmt *, unsigned>> RegionMCDCBitmapMap; - std::unique_ptr<llvm::DenseMap<const Stmt *, unsigned>> RegionCondIDMap; + std::unique_ptr<llvm::DenseMap<const Stmt *, int16_t>> RegionCondIDMap; std::unique_ptr<llvm::DenseMap<const Stmt *, uint64_t>> StmtCountMap; std::unique_ptr<llvm::InstrProfRecord> ProfRecord; std::vector<uint64_t> RegionCounts; diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 0c43317642bca4..d918acd951dd8c 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -587,8 +587,8 @@ struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder { struct MCDCCoverageBuilder { struct DecisionIDPair { - MCDCConditionID TrueID = 0; - MCDCConditionID FalseID = 0; + MCDCConditionID TrueID = -1; + MCDCConditionID FalseID = -1; }; /// The AST walk recursively visits nested logical-AND or logical-OR binary @@ -684,11 +684,11 @@ struct MCDCCoverageBuilder { llvm::SmallVector<DecisionIDPair> DecisionStack; llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDs; llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap; - MCDCConditionID NextID = 1; + MCDCConditionID NextID = 0; bool NotMapped = false; /// Represent a sentinel value of [0,0] for the bottom of DecisionStack. - static constexpr DecisionIDPair DecisionStackSentinel{0, 0}; + static constexpr DecisionIDPair DecisionStackSentinel{-1, -1}; /// Is this a logical-AND operation? bool isLAnd(const BinaryOperator *E) const { @@ -705,12 +705,12 @@ struct MCDCCoverageBuilder { /// Return whether the build of the control flow map is at the top-level /// (root) of a logical operator nest in a boolean expression prior to the /// assignment of condition IDs. - bool isIdle() const { return (NextID == 1 && !NotMapped); } + bool isIdle() const { return (NextID == 0 && !NotMapped); } /// Return whether any IDs have been assigned in the build of the control /// flow map, indicating that the map is being generated for this boolean /// expression. - bool isBuilding() const { return (NextID > 1); } + bool isBuilding() const { return (NextID > 0); } /// Set the given condition's ID. void setCondID(const Expr *Cond, MCDCConditionID ID) { @@ -721,7 +721,7 @@ struct MCDCCoverageBuilder { MCDCConditionID getCondID(const Expr *Cond) const { auto I = CondIDs.find(CodeGenFunction::stripCond(Cond)); if (I == CondIDs.end()) - return 0; + return -1; else return I->second; } @@ -788,15 +788,15 @@ struct MCDCCoverageBuilder { // Reset state if not doing mapping. if (NotMapped) { NotMapped = false; - assert(NextID == 1); + assert(NextID == 0); return 0; } // Set number of conditions and reset. - unsigned TotalConds = NextID - 1; + unsigned TotalConds = NextID; // Reset ID back to beginning. - NextID = 1; + NextID = 0; return TotalConds; } @@ -865,8 +865,8 @@ 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 = -1, MCDCConditionID TrueID = -1, + MCDCConditionID FalseID = -1) { if (StartLoc && !FalseCount) { MostRecentLocation = *StartLoc; @@ -892,7 +892,7 @@ struct CounterCoverageMappingBuilder return RegionStack.size() - 1; } - size_t pushRegion(unsigned BitmapIdx, unsigned Conditions, + size_t pushRegion(unsigned BitmapIdx, uint16_t Conditions, std::optional<SourceLocation> StartLoc = std::nullopt, std::optional<SourceLocation> EndLoc = std::nullopt) { @@ -2134,8 +2134,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 + 1 << "," << R.MCDCParams.TrueID + 1; + OS << "," << R.MCDCParams.FalseID + 1 << "] "; } if (R.Kind == CounterMappingRegion::ExpansionRegion) diff --git a/clang/lib/CodeGen/CoverageMappingGen.h b/clang/lib/CodeGen/CoverageMappingGen.h index 62cea173c9fc93..915099f8331923 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.h +++ b/clang/lib/CodeGen/CoverageMappingGen.h @@ -151,7 +151,7 @@ class CoverageMappingGen { const LangOptions &LangOpts; llvm::DenseMap<const Stmt *, unsigned> *CounterMap; llvm::DenseMap<const Stmt *, unsigned> *MCDCBitmapMap; - llvm::DenseMap<const Stmt *, unsigned> *CondIDMap; + llvm::DenseMap<const Stmt *, int16_t> *CondIDMap; public: CoverageMappingGen(CoverageMappingModuleGen &CVM, SourceManager &SM, @@ -163,7 +163,7 @@ class CoverageMappingGen { const LangOptions &LangOpts, llvm::DenseMap<const Stmt *, unsigned> *CounterMap, llvm::DenseMap<const Stmt *, unsigned> *MCDCBitmapMap, - llvm::DenseMap<const Stmt *, unsigned> *CondIDMap) + llvm::DenseMap<const Stmt *, int16_t> *CondIDMap) : CVM(CVM), SM(SM), LangOpts(LangOpts), CounterMap(CounterMap), MCDCBitmapMap(MCDCBitmapMap), CondIDMap(CondIDMap) {} diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h index 88ec60c7aa33c6..959edea1bbaedd 100644 --- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h +++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h @@ -249,17 +249,17 @@ struct CounterMappingRegion { MCDCBranchRegion }; - using MCDCConditionID = unsigned int; + using MCDCConditionID = int16_t; struct MCDCParameters { /// Byte Index of Bitmap Coverage Object for a Decision Region. unsigned BitmapIdx = 0; /// Number of Conditions used for a Decision Region. - unsigned NumConditions = 0; + uint16_t NumConditions = 0; /// 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 = -1, TrueID = -1, FalseID = -1; }; /// Primary Counter that is also used for Branch Regions (TrueCount). @@ -345,8 +345,8 @@ struct CounterMappingRegion { unsigned LineEnd, unsigned ColumnEnd) { return CounterMappingRegion(Count, FalseCount, MCDCParams, FileID, 0, LineStart, ColumnStart, LineEnd, ColumnEnd, - MCDCParams.ID == 0 ? BranchRegion - : MCDCBranchRegion); + MCDCParams.ID < 0 ? BranchRegion + : MCDCBranchRegion); } static CounterMappingRegion diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index eb0996e33b70dc..5b631bd7c27c2a 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -283,25 +283,26 @@ class MCDCRecordProcessor { // 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, + void buildTestVector(MCDCRecord::TestVector &TV, + CounterMappingRegion::MCDCConditionID ID, unsigned Index) { const CounterMappingRegion *Branch = Map[ID]; - TV[ID - 1] = MCDCRecord::MCDC_False; - if (Branch->MCDCParams.FalseID > 0) + TV[ID] = 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) + Index |= 1 << ID; + TV[ID] = MCDCRecord::MCDC_True; + if (Branch->MCDCParams.TrueID >= 0) buildTestVector(TV, Branch->MCDCParams.TrueID, Index); else recordTestVector(TV, Index, MCDCRecord::MCDC_True); // Reset back to DontCare. - TV[ID - 1] = MCDCRecord::MCDC_DontCare; + TV[ID] = MCDCRecord::MCDC_DontCare; } /// Walk the bits in the bitmap. A bit set to '1' indicates that the test @@ -311,7 +312,7 @@ class MCDCRecordProcessor { // We start at the root node (ID == 1) with all values being DontCare. // `Index` encodes the bitmask of true values and is initially 0. MCDCRecord::TestVector TV(NumConditions, MCDCRecord::MCDC_DontCare); - buildTestVector(TV, 1, 0); + buildTestVector(TV, 0, 0); } // Find an independence pair for each condition: @@ -372,7 +373,7 @@ class MCDCRecordProcessor { // from being measured. for (const auto *B : Branches) { Map[B->MCDCParams.ID] = B; - PosToID[I] = B->MCDCParams.ID - 1; + PosToID[I] = B->MCDCParams.ID; CondLoc[I] = B->startLoc(); Folded[I++] = (B->Count.isZero() && B->FalseCount.isZero()); } @@ -562,10 +563,10 @@ class MCDCDecisionRecorder { assert(Branch.Kind == CounterMappingRegion::MCDCBranchRegion); auto ConditionID = Branch.MCDCParams.ID; - assert(ConditionID > 0 && "ConditionID should begin with 1"); + assert(ConditionID >= 0 && "ConditionID should be positive"); if (ConditionIDs.contains(ConditionID) || - ConditionID > DecisionRegion->MCDCParams.NumConditions) + ConditionID >= DecisionRegion->MCDCParams.NumConditions) return NotProcessed; if (!this->dominates(Branch)) @@ -575,7 +576,7 @@ class MCDCDecisionRecorder { // Put `ID=1` in front of `MCDCBranches` for convenience // even if `MCDCBranches` is not topological. - if (ConditionID == 1) + if (ConditionID == 0) MCDCBranches.insert(MCDCBranches.begin(), &Branch); else MCDCBranches.push_back(&Branch); diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp index ac8e6b56379f21..df12d2fec3050a 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp @@ -244,7 +244,7 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray( unsigned LineStart = 0; for (size_t I = 0; I < NumRegions; ++I) { Counter C, C2; - uint64_t BIDX = 0, NC = 0, ID = 0, TID = 0, FID = 0; + uint64_t BIDX = 0, NC = 0, ID1 = 0, TID1 = 0, FID1 = 0; CounterMappingRegion::RegionKind Kind = CounterMappingRegion::CodeRegion; // Read the combined counter + region kind. @@ -302,18 +302,18 @@ Error RawCoverageMappingReader::readMappingRegionsSubArray( return Err; if (auto Err = readCounter(C2)) return Err; - if (auto Err = readIntMax(ID, std::numeric_limits<unsigned>::max())) + if (auto Err = readIntMax(ID1, std::numeric_limits<int16_t>::max())) return Err; - if (auto Err = readIntMax(TID, std::numeric_limits<unsigned>::max())) + if (auto Err = readIntMax(TID1, std::numeric_limits<int16_t>::max())) return Err; - if (auto Err = readIntMax(FID, std::numeric_limits<unsigned>::max())) + if (auto Err = readIntMax(FID1, std::numeric_limits<int16_t>::max())) return Err; break; case CounterMappingRegion::MCDCDecisionRegion: Kind = CounterMappingRegion::MCDCDecisionRegion; if (auto Err = readIntMax(BIDX, std::numeric_limits<unsigned>::max())) return Err; - if (auto Err = readIntMax(NC, std::numeric_limits<unsigned>::max())) + if (auto Err = readIntMax(NC, std::numeric_limits<int16_t>::max())) return Err; break; default: @@ -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<uint16_t>(NC), + static_cast<int16_t>(int(ID1) - 1), + static_cast<int16_t>(int(TID1) - 1), + static_cast<int16_t>(int(FID1) - 1)}, 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..b9257a1c95e7d8 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp @@ -251,9 +251,9 @@ void CoverageMappingWriter::write(raw_ostream &OS) { 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.ID + 1), OS); + encodeULEB128(unsigned(I->MCDCParams.TrueID + 1), OS); + encodeULEB128(unsigned(I->MCDCParams.FalseID + 1), 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..cce271d523960e 100644 --- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp +++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp @@ -192,7 +192,7 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> { addCMR(Counter::getZero(), File, LS, CS, LE, CE, true); } - void addMCDCDecisionCMR(unsigned Mask, unsigned NC, StringRef File, + void addMCDCDecisionCMR(unsigned Mask, uint16_t NC, StringRef File, unsigned LS, unsigned CS, unsigned LE, unsigned CE) { auto &Regions = InputFunctions.back().Regions; unsigned FileID = getFileIndexForFunction(File); @@ -201,8 +201,8 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> { CE)); } - void addMCDCBranchCMR(Counter C1, Counter C2, unsigned ID, unsigned TrueID, - unsigned FalseID, StringRef File, unsigned LS, + void addMCDCBranchCMR(Counter C1, Counter C2, int16_t ID, int16_t TrueID, + int16_t FalseID, StringRef File, unsigned LS, unsigned CS, unsigned LE, unsigned CE) { auto &Regions = InputFunctions.back().Regions; unsigned FileID = getFileIndexForFunction(File); @@ -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), 0, 1, -1, "file", 7, 2, 7, 3); - addMCDCBranchCMR(Counter::getCounter(2), Counter::getCounter(3), 2, 0, 0, + addMCDCBranchCMR(Counter::getCounter(2), Counter::getCounter(3), 1, -1, -1, "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), 0, 1, -1, + "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), 1, -1, -1, + "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/81257 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
