[Coverage][Single] Round Counters to boolean after evaluation

[chapuni]

Rounding in merging segments has been done after #75425.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: NAKAMURA Takumi (chapuni)

Changes

• Round Counts as 1/0
• Confirm both ExecutionCount and AltExecutionCount are in range.

---

Full diff: https://github.com/llvm/llvm-project/pull/110972.diff

6 Files Affected:

• (modified) compiler-rt/test/profile/instrprof-block-coverage.c (+1-1)
• (modified) compiler-rt/test/profile/instrprof-entry-coverage.c (+1-1)
• (modified) llvm/include/llvm/ProfileData/InstrProf.h (+4-1)
• (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+3)
• (modified) llvm/lib/ProfileData/InstrProf.cpp (+1-1)
• (modified) llvm/lib/ProfileData/InstrProfReader.cpp (+1)

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

[ellishg]

This is deliberate. Even though we only record boolean coverage in the raw profiles, when we aggregate many raw profiles together we can still get some sense of relative hotness by looking at the counter value. Otherwise we lose information if we treat the counter value in the indexed profile as a boolean.

[chapuni]

I didn't imagine use cases in PGO. I'll leave it unchanged.

In contrast, do you think we could round counters as boolean only in llvm-cov?

[ellishg]

I think that makes sense for frontend coverage since we aren't using those values for optimization.

[chapuni]

Simplified. This no longer depends on #110966.

[chapuni]

The last arg will be pruned after #110966.

[ellishg]

Looks ok to me, but can you add a test?

[chapuni]

@ellishg Yes, it makes sense. I was not aware since #75425 didn't have llvm-cov tests.
I'll do.