Rework. (Also reverts "[Coverage] Move SingleByteCoverage out of CountedRegion")

Author: chapuni

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

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

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);
   }
 };
 

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;

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;
 }

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);
   }

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();