https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/110972
>From aacb50ddf87d96b4a0644c7ef5d0a86dc94f069b Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Wed, 2 Oct 2024 23:25:52 +0900 Subject: [PATCH 1/2] [Coverage] Make SingleByteCoverage work consistent to merging - Round `Counts` as 1/0 - Confirm both `ExecutionCount` and `AltExecutionCount` are in range. --- compiler-rt/test/profile/instrprof-block-coverage.c | 2 +- compiler-rt/test/profile/instrprof-entry-coverage.c | 2 +- llvm/include/llvm/ProfileData/InstrProf.h | 5 ++++- llvm/lib/ProfileData/Coverage/CoverageMapping.cpp | 3 +++ llvm/lib/ProfileData/InstrProf.cpp | 2 +- llvm/lib/ProfileData/InstrProfReader.cpp | 1 + 6 files changed, 11 insertions(+), 4 deletions(-) diff --git a/compiler-rt/test/profile/instrprof-block-coverage.c b/compiler-rt/test/profile/instrprof-block-coverage.c index 829d5af8dc3f9e..8d924e1cac64d8 100644 --- a/compiler-rt/test/profile/instrprof-block-coverage.c +++ b/compiler-rt/test/profile/instrprof-block-coverage.c @@ -49,4 +49,4 @@ int main(int argc, char *argv[]) { // CHECK-ERROR-NOT: warning: {{.*}}: Found inconsistent block coverage -// COUNTS: Maximum function count: 4 +// COUNTS: Maximum function count: 1 diff --git a/compiler-rt/test/profile/instrprof-entry-coverage.c b/compiler-rt/test/profile/instrprof-entry-coverage.c index 1c6816ba01964b..b93a4e0c43ccd6 100644 --- a/compiler-rt/test/profile/instrprof-entry-coverage.c +++ b/compiler-rt/test/profile/instrprof-entry-coverage.c @@ -36,4 +36,4 @@ int main(int argc, char *argv[]) { // CHECK-DAG: foo // CHECK-DAG: bar -// COUNTS: Maximum function count: 2 +// COUNTS: Maximum function count: 1 diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h index b0b2258735e2ae..df9e76966bf42b 100644 --- a/llvm/include/llvm/ProfileData/InstrProf.h +++ b/llvm/include/llvm/ProfileData/InstrProf.h @@ -830,6 +830,7 @@ struct InstrProfValueSiteRecord { /// Profiling information for a single function. struct InstrProfRecord { std::vector<uint64_t> Counts; + bool SingleByteCoverage = false; std::vector<uint8_t> BitmapBytes; InstrProfRecord() = default; @@ -839,13 +840,15 @@ struct InstrProfRecord { : Counts(std::move(Counts)), BitmapBytes(std::move(BitmapBytes)) {} InstrProfRecord(InstrProfRecord &&) = default; InstrProfRecord(const InstrProfRecord &RHS) - : Counts(RHS.Counts), BitmapBytes(RHS.BitmapBytes), + : Counts(RHS.Counts), SingleByteCoverage(RHS.SingleByteCoverage), + BitmapBytes(RHS.BitmapBytes), ValueData(RHS.ValueData ? std::make_unique<ValueProfData>(*RHS.ValueData) : nullptr) {} InstrProfRecord &operator=(InstrProfRecord &&) = default; InstrProfRecord &operator=(const InstrProfRecord &RHS) { Counts = RHS.Counts; + SingleByteCoverage = RHS.SingleByteCoverage; BitmapBytes = RHS.BitmapBytes; if (!RHS.ValueData) { ValueData = nullptr; diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index a02136d5b0386d..bc765c59381718 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -874,6 +874,9 @@ Error CoverageMapping::loadFunctionRecord( consumeError(std::move(E)); return Error::success(); } + assert(!SingleByteCoverage || + (0 <= *ExecutionCount && *ExecutionCount <= 1 && + 0 <= *AltExecutionCount && *AltExecutionCount <= 1)); Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount); // Record ExpansionRegion. diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index b9937c9429b77d..0f6677b4d35718 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -952,7 +952,7 @@ void InstrProfRecord::merge(InstrProfRecord &Other, uint64_t Weight, Value = getInstrMaxCountValue(); Overflowed = true; } - Counts[I] = Value; + Counts[I] = (SingleByteCoverage && Value != 0 ? 1 : Value); if (Overflowed) Warn(instrprof_error::counter_overflow); } diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp index b90617c74f6d13..a07d7f573275ba 100644 --- a/llvm/lib/ProfileData/InstrProfReader.cpp +++ b/llvm/lib/ProfileData/InstrProfReader.cpp @@ -743,6 +743,7 @@ Error RawInstrProfReader<IntPtrT>::readRawCounts( Record.Counts.clear(); Record.Counts.reserve(NumCounters); + Record.SingleByteCoverage = hasSingleByteCoverage(); for (uint32_t I = 0; I < NumCounters; I++) { const char *Ptr = CountersStart + CounterBaseOffset + I * getCounterTypeSize(); >From b9bbc7cac3076594cd326ffa7f2d4fc4a92fabb9 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Sat, 5 Oct 2024 10:43:26 +0900 Subject: [PATCH 2/2] Rework. (Also reverts "[Coverage] Move SingleByteCoverage out of CountedRegion") --- .../test/profile/instrprof-block-coverage.c | 2 +- .../test/profile/instrprof-entry-coverage.c | 2 +- .../ProfileData/Coverage/CoverageMapping.h | 27 +++++++------ llvm/include/llvm/ProfileData/InstrProf.h | 5 +-- .../ProfileData/Coverage/CoverageMapping.cpp | 40 +++++++------------ llvm/lib/ProfileData/InstrProf.cpp | 2 +- llvm/lib/ProfileData/InstrProfReader.cpp | 1 - 7 files changed, 33 insertions(+), 46 deletions(-) diff --git a/compiler-rt/test/profile/instrprof-block-coverage.c b/compiler-rt/test/profile/instrprof-block-coverage.c index 8d924e1cac64d8..829d5af8dc3f9e 100644 --- a/compiler-rt/test/profile/instrprof-block-coverage.c +++ b/compiler-rt/test/profile/instrprof-block-coverage.c @@ -49,4 +49,4 @@ int main(int argc, char *argv[]) { // CHECK-ERROR-NOT: warning: {{.*}}: Found inconsistent block coverage -// COUNTS: Maximum function count: 1 +// COUNTS: Maximum function count: 4 diff --git a/compiler-rt/test/profile/instrprof-entry-coverage.c b/compiler-rt/test/profile/instrprof-entry-coverage.c index b93a4e0c43ccd6..1c6816ba01964b 100644 --- a/compiler-rt/test/profile/instrprof-entry-coverage.c +++ b/compiler-rt/test/profile/instrprof-entry-coverage.c @@ -36,4 +36,4 @@ int main(int argc, char *argv[]) { // CHECK-DAG: foo // CHECK-DAG: bar -// COUNTS: Maximum function count: 1 +// COUNTS: Maximum function count: 2 diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h index 77c447efe1a7c9..fa07b3a9e8b14f 100644 --- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h +++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h @@ -359,15 +359,19 @@ struct CountedRegion : public CounterMappingRegion { uint64_t ExecutionCount; uint64_t FalseExecutionCount; bool Folded; + bool HasSingleByteCoverage; - CountedRegion(const CounterMappingRegion &R, uint64_t ExecutionCount) + CountedRegion(const CounterMappingRegion &R, uint64_t ExecutionCount, + bool HasSingleByteCoverage) : CounterMappingRegion(R), ExecutionCount(ExecutionCount), - FalseExecutionCount(0), Folded(false) {} + FalseExecutionCount(0), Folded(false), + HasSingleByteCoverage(HasSingleByteCoverage) {} CountedRegion(const CounterMappingRegion &R, uint64_t ExecutionCount, - uint64_t FalseExecutionCount) + uint64_t FalseExecutionCount, bool HasSingleByteCoverage) : CounterMappingRegion(R), ExecutionCount(ExecutionCount), - FalseExecutionCount(FalseExecutionCount), Folded(false) {} + FalseExecutionCount(FalseExecutionCount), Folded(false), + HasSingleByteCoverage(HasSingleByteCoverage) {} }; /// MCDC Record grouping all information together. @@ -698,12 +702,9 @@ struct FunctionRecord { std::vector<MCDCRecord> MCDCRecords; /// The number of times this function was executed. uint64_t ExecutionCount = 0; - bool SingleByteCoverage; - FunctionRecord(StringRef Name, ArrayRef<StringRef> Filenames, - bool SingleByteCoverage) - : Name(Name), Filenames(Filenames.begin(), Filenames.end()), - SingleByteCoverage(SingleByteCoverage) {} + FunctionRecord(StringRef Name, ArrayRef<StringRef> Filenames) + : Name(Name), Filenames(Filenames.begin(), Filenames.end()) {} FunctionRecord(FunctionRecord &&FR) = default; FunctionRecord &operator=(FunctionRecord &&) = default; @@ -713,10 +714,11 @@ struct FunctionRecord { } void pushRegion(CounterMappingRegion Region, uint64_t Count, - uint64_t FalseCount) { + uint64_t FalseCount, bool HasSingleByteCoverage) { if (Region.Kind == CounterMappingRegion::BranchRegion || Region.Kind == CounterMappingRegion::MCDCBranchRegion) { - CountedBranchRegions.emplace_back(Region, Count, FalseCount); + CountedBranchRegions.emplace_back(Region, Count, FalseCount, + HasSingleByteCoverage); // If both counters are hard-coded to zero, then this region represents a // constant-folded branch. if (Region.Count.isZero() && Region.FalseCount.isZero()) @@ -725,7 +727,8 @@ struct FunctionRecord { } if (CountedRegions.empty()) ExecutionCount = Count; - CountedRegions.emplace_back(Region, Count, FalseCount); + CountedRegions.emplace_back(Region, Count, FalseCount, + HasSingleByteCoverage); } }; diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h index df9e76966bf42b..b0b2258735e2ae 100644 --- a/llvm/include/llvm/ProfileData/InstrProf.h +++ b/llvm/include/llvm/ProfileData/InstrProf.h @@ -830,7 +830,6 @@ struct InstrProfValueSiteRecord { /// Profiling information for a single function. struct InstrProfRecord { std::vector<uint64_t> Counts; - bool SingleByteCoverage = false; std::vector<uint8_t> BitmapBytes; InstrProfRecord() = default; @@ -840,15 +839,13 @@ struct InstrProfRecord { : Counts(std::move(Counts)), BitmapBytes(std::move(BitmapBytes)) {} InstrProfRecord(InstrProfRecord &&) = default; InstrProfRecord(const InstrProfRecord &RHS) - : Counts(RHS.Counts), SingleByteCoverage(RHS.SingleByteCoverage), - BitmapBytes(RHS.BitmapBytes), + : Counts(RHS.Counts), BitmapBytes(RHS.BitmapBytes), ValueData(RHS.ValueData ? std::make_unique<ValueProfData>(*RHS.ValueData) : nullptr) {} InstrProfRecord &operator=(InstrProfRecord &&) = default; InstrProfRecord &operator=(const InstrProfRecord &RHS) { Counts = RHS.Counts; - SingleByteCoverage = RHS.SingleByteCoverage; BitmapBytes = RHS.BitmapBytes; if (!RHS.ValueData) { ValueData = nullptr; diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index bc765c59381718..9a03dede84b6df 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -856,7 +856,7 @@ Error CoverageMapping::loadFunctionRecord( return Error::success(); MCDCDecisionRecorder MCDCDecisions; - FunctionRecord Function(OrigFuncName, Record.Filenames, SingleByteCoverage); + FunctionRecord Function(OrigFuncName, Record.Filenames); for (const auto &Region : Record.MappingRegions) { // MCDCDecisionRegion should be handled first since it overlaps with // others inside. @@ -874,10 +874,10 @@ Error CoverageMapping::loadFunctionRecord( consumeError(std::move(E)); return Error::success(); } - assert(!SingleByteCoverage || - (0 <= *ExecutionCount && *ExecutionCount <= 1 && - 0 <= *AltExecutionCount && *AltExecutionCount <= 1)); - Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount); + Function.pushRegion( + Region, (SingleByteCoverage && *ExecutionCount ? 1 : *ExecutionCount), + (SingleByteCoverage && *AltExecutionCount ? 1 : *AltExecutionCount), + SingleByteCoverage); // Record ExpansionRegion. if (Region.Kind == CounterMappingRegion::ExpansionRegion) { @@ -1273,8 +1273,7 @@ class SegmentBuilder { /// Combine counts of regions which cover the same area. static ArrayRef<CountedRegion> - combineRegions(MutableArrayRef<CountedRegion> Regions, - bool SingleByteCoverage) { + combineRegions(MutableArrayRef<CountedRegion> Regions) { if (Regions.empty()) return Regions; auto Active = Regions.begin(); @@ -1301,7 +1300,9 @@ class SegmentBuilder { // We add counts of the regions of the same kind as the active region // to handle the both situations. if (I->Kind == Active->Kind) { - if (SingleByteCoverage) + assert(I->HasSingleByteCoverage == Active->HasSingleByteCoverage && + "Regions are generated in different coverage modes"); + if (I->HasSingleByteCoverage) Active->ExecutionCount = Active->ExecutionCount || I->ExecutionCount; else Active->ExecutionCount += I->ExecutionCount; @@ -1313,14 +1314,12 @@ class SegmentBuilder { public: /// Build a sorted list of CoverageSegments from a list of Regions. static std::vector<CoverageSegment> - buildSegments(MutableArrayRef<CountedRegion> Regions, - bool SingleByteCoverage) { + buildSegments(MutableArrayRef<CountedRegion> Regions) { std::vector<CoverageSegment> Segments; SegmentBuilder Builder(Segments); sortNestedRegions(Regions); - ArrayRef<CountedRegion> CombinedRegions = - combineRegions(Regions, SingleByteCoverage); + ArrayRef<CountedRegion> CombinedRegions = combineRegions(Regions); LLVM_DEBUG({ dbgs() << "Combined regions:\n"; @@ -1407,14 +1406,10 @@ CoverageData CoverageMapping::getCoverageForFile(StringRef Filename) const { // the filename, we may get back some records that are not in the file. ArrayRef<unsigned> RecordIndices = getImpreciseRecordIndicesForFilename(Filename); - std::optional<bool> SingleByteCoverage; for (unsigned RecordIndex : RecordIndices) { const FunctionRecord &Function = Functions[RecordIndex]; auto MainFileID = findMainViewFileID(Filename, Function); auto FileIDs = gatherFileIDs(Filename, Function); - assert(!SingleByteCoverage || - *SingleByteCoverage == Function.SingleByteCoverage); - SingleByteCoverage = Function.SingleByteCoverage; for (const auto &CR : Function.CountedRegions) if (FileIDs.test(CR.FileID)) { Regions.push_back(CR); @@ -1432,8 +1427,7 @@ CoverageData CoverageMapping::getCoverageForFile(StringRef Filename) const { } LLVM_DEBUG(dbgs() << "Emitting segments for file: " << Filename << "\n"); - FileCoverage.Segments = - SegmentBuilder::buildSegments(Regions, *SingleByteCoverage); + FileCoverage.Segments = SegmentBuilder::buildSegments(Regions); return FileCoverage; } @@ -1489,8 +1483,7 @@ CoverageMapping::getCoverageForFunction(const FunctionRecord &Function) const { LLVM_DEBUG(dbgs() << "Emitting segments for function: " << Function.Name << "\n"); - FunctionCoverage.Segments = - SegmentBuilder::buildSegments(Regions, Function.SingleByteCoverage); + FunctionCoverage.Segments = SegmentBuilder::buildSegments(Regions); return FunctionCoverage; } @@ -1500,12 +1493,8 @@ CoverageData CoverageMapping::getCoverageForExpansion( CoverageData ExpansionCoverage( Expansion.Function.Filenames[Expansion.FileID]); std::vector<CountedRegion> Regions; - std::optional<bool> SingleByteCoverage; for (const auto &CR : Expansion.Function.CountedRegions) if (CR.FileID == Expansion.FileID) { - assert(!SingleByteCoverage || - *SingleByteCoverage == Expansion.Function.SingleByteCoverage); - SingleByteCoverage = Expansion.Function.SingleByteCoverage; Regions.push_back(CR); if (isExpansion(CR, Expansion.FileID)) ExpansionCoverage.Expansions.emplace_back(CR, Expansion.Function); @@ -1517,8 +1506,7 @@ CoverageData CoverageMapping::getCoverageForExpansion( LLVM_DEBUG(dbgs() << "Emitting segments for expansion of file " << Expansion.FileID << "\n"); - ExpansionCoverage.Segments = - SegmentBuilder::buildSegments(Regions, *SingleByteCoverage); + ExpansionCoverage.Segments = SegmentBuilder::buildSegments(Regions); return ExpansionCoverage; } diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index 0f6677b4d35718..b9937c9429b77d 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -952,7 +952,7 @@ void InstrProfRecord::merge(InstrProfRecord &Other, uint64_t Weight, Value = getInstrMaxCountValue(); Overflowed = true; } - Counts[I] = (SingleByteCoverage && Value != 0 ? 1 : Value); + Counts[I] = Value; if (Overflowed) Warn(instrprof_error::counter_overflow); } diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp index a07d7f573275ba..b90617c74f6d13 100644 --- a/llvm/lib/ProfileData/InstrProfReader.cpp +++ b/llvm/lib/ProfileData/InstrProfReader.cpp @@ -743,7 +743,6 @@ Error RawInstrProfReader<IntPtrT>::readRawCounts( Record.Counts.clear(); Record.Counts.reserve(NumCounters); - Record.SingleByteCoverage = hasSingleByteCoverage(); for (uint32_t I = 0; I < NumCounters; I++) { const char *Ptr = CountersStart + CounterBaseOffset + I * getCounterTypeSize(); _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits