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

chapuni · 2024-10-03T09:05:39Z

Rounding in merging segments has been done after #75425.

Depends on: #113114

`SingleByteCoverage` is not per-region attribute at least. At the moment, this change moves it into `FunctionRecord`.

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

llvmbot · 2024-10-03T09:06:14Z

@llvm/pr-subscribers-clang

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

llvm/lib/ProfileData/InstrProf.cpp

…tedRegion")

chapuni

Simplified. This no longer depends on #110966.

chapuni · 2024-10-05T08:26:05Z

llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

+    Function.pushRegion(
+        Region, (SingleByteCoverage && *ExecutionCount ? 1 : *ExecutionCount),
+        (SingleByteCoverage && *AltExecutionCount ? 1 : *AltExecutionCount),
+        SingleByteCoverage);


The last arg will be pruned after #110966.

I'll rewind this entirely with #110966 to make the viewer responsible to round counters.

ellishg

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

chapuni · 2024-10-09T00:28:29Z

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

…matches

chapuni · 2024-11-18T09:15:16Z

@ellishg Sorry for the delay. I was supposing tests easier.

The current implementation of singlebytecoverage doesn't emit any expressions, so I cannot create testcases to trigger my changes.

Even w/o any additional tests, my change doesn't break anything. So I can assume other llvm-cov tests may cover this.

That said, I can postpend committing this after practical changes in singlebytecoverage. They will be good testcases.

ellishg · 2024-11-18T17:31:35Z

@ellishg Sorry for the delay. I was supposing tests easier.

The current implementation of singlebytecoverage doesn't emit any expressions, so I cannot create testcases to trigger my changes.

Even w/o any additional tests, my change doesn't break anything. So I can assume other llvm-cov tests may cover this.

That said, I can postpend committing this after practical changes in singlebytecoverage. They will be good testcases.

Adding tests later should be fine. Thanks!

And reformat. NFC.

…ingle/merge

Conflicts: llvm/test/tools/llvm-cov/branch-macros.cpp

…ingle/merge Conflicts: llvm/test/tools/llvm-cov/Inputs/showLineExecutionCounts.cpp

Restructure some tests to split into `%.test` and Inputs/%.c*`. Add test actions with `yaml2obj` for single byte coverage. `FileCheck` lines are: - Relax to accept both counter values and single values `1`. A few line counting are greater than `1` due to `llvm-profdata merge`. They will be fixed by #110972. - Suppress matching with `--check-prefixes=CHECK,BRCOV`, since the current implementation of single byte doesn't emit any branch coverages. They will be integrated to `CHECK` finally. - Some tests are not unified but use dedicated `CHECK` lines for single byte, since old format is different (esp. "partial fold"). They will be unified when `Inputs` will be regenerated. Introduce llvm/test/tools/llvm-cov/Inputs/yaml.makefile for convenience. `%-single.yaml` and `%-single.proftext` are generated by it. It could be used to regenerate other files. https://discourse.llvm.org/t/rfc-integrating-singlebytecoverage-with-branch-coverage/82492

Conflicts: clang/test/CoverageMapping/single-byte-counters.cpp llvm/test/tools/llvm-cov/Inputs/branch-logical-mixed.cpp llvm/test/tools/llvm-cov/branch-macros.test llvm/test/tools/llvm-cov/showLineExecutionCounts.test

llvm-ci · 2024-12-23T23:16:07Z

LLVM Buildbot has detected a new failure on builder openmp-s390x-linux running on systemz-1 while building llvm at step 6 "test-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/6149

Here is the relevant piece of the build log for the reference

Step 6 (test-openmp) failure: test (failure)
******************** TEST 'libomp :: tasking/issue-94260-2.c' FAILED ********************
Exit Code: -11

Command Output (stdout):
--
# RUN: at line 1
/home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./bin/clang -fopenmp   -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test -L /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -fno-omit-frame-pointer -mbackchain -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/ompt /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic && /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# executed command: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./bin/clang -fopenmp -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test -L /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -fno-omit-frame-pointer -mbackchain -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/ompt /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic
# executed command: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11

--

********************

chapuni added 2 commits October 3, 2024 15:47

[Coverage] Move SingleByteCoverage out of CountedRegion

178b57c

`SingleByteCoverage` is not per-region attribute at least. At the moment, this change moves it into `FunctionRecord`.

[Coverage] Make SingleByteCoverage work consistent to merging

aacb50d

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

chapuni requested review from ellishg, gulfemsavrun and ornata October 3, 2024 09:05

llvmbot added compiler-rt PGO Profile Guided Optimizations labels Oct 3, 2024

ellishg reviewed Oct 3, 2024

View reviewed changes

llvm/lib/ProfileData/InstrProf.cpp Show resolved Hide resolved

Rework. (Also reverts "[Coverage] Move SingleByteCoverage out of Coun…

b9bbc7c

…tedRegion")

chapuni changed the base branch from users/chapuni/cov/single/refactor to main October 5, 2024 04:04

chapuni commented Oct 5, 2024

View reviewed changes

chapuni changed the title ~~[Coverage] Make SingleByteCoverage work consistent to merging~~ [Coverage][Single] Round Counters to boolean after evaluation Oct 5, 2024

ellishg reviewed Oct 9, 2024

View reviewed changes

ellishg mentioned this pull request Oct 9, 2024

[Coverage] Move SingleByteCoverage out of CountedRegion #110966

Merged

clang/test/CoverageMapping/single-byte-counters.cpp: Rewrite counter …

52f072e

…matches

chapuni added 5 commits November 20, 2024 23:39

test/llvm-cov: Transform %.c* tests to {%.test, Inputs/%.c*}

97a4a8f

And reformat. NFC.

Introduce test/llvm-cov/Inputs/yaml.makefile for convenience.

c50c492

Add tests for SingleByteCoverage

d7c5b44

Merge branch 'users/chapuni/cov/single/test' into users/chapuni/cov/s…

6675226

…ingle/merge

Fix a test to fix linecount=1

5fc3408

llvmbot added the clang Clang issues not falling into any other category label Nov 20, 2024

chapuni changed the base branch from main to users/chapuni/cov/single/test November 20, 2024 15:39

ellishg approved these changes Nov 20, 2024

View reviewed changes

threads.c: Fixup on the clean testdir

eb7fff9

This was referenced Nov 21, 2024

Introduce SingleByteCoverage tests (w/yaml2obj) #113114

Merged

Single byte coverage with branch coverage #113115

Draft

chapuni added 6 commits November 21, 2024 23:27

Rename threads.c to threads.test since it is no longer C source file.

7543095

llvm/test/tools/llvm-cov/Inputs: Avoid wildcards rm -rf %t*.dir

00ac90d

Merge branch 'main' into users/chapuni/cov/single/test

fcb3ee8

Conflicts: llvm/test/tools/llvm-cov/branch-macros.cpp

Use [[#min(C,n)]] for tests

5fa862a

Merge branch 'users/chapuni/cov/single/test' into users/chapuni/cov/s…

68d7b3b

…ingle/merge Conflicts: llvm/test/tools/llvm-cov/Inputs/showLineExecutionCounts.cpp

Update tests

24457a7

Base automatically changed from users/chapuni/cov/single/test to main December 23, 2024 21:40

Merge branch 'main' into users/chapuni/cov/single/merge

9a20404

Conflicts: clang/test/CoverageMapping/single-byte-counters.cpp llvm/test/tools/llvm-cov/Inputs/branch-logical-mixed.cpp llvm/test/tools/llvm-cov/branch-macros.test llvm/test/tools/llvm-cov/showLineExecutionCounts.test

chapuni merged commit 275a277 into main Dec 23, 2024
5 of 8 checks passed

chapuni deleted the users/chapuni/cov/single/merge branch December 23, 2024 23:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

chapuni commented Oct 3, 2024 •

edited

Loading

Uh oh!

llvmbot commented Oct 3, 2024 •

edited

Loading

Uh oh!

Uh oh!

chapuni left a comment

Uh oh!

chapuni Oct 5, 2024

Uh oh!

chapuni Dec 23, 2024

Uh oh!

ellishg left a comment

Uh oh!

chapuni commented Oct 9, 2024

Uh oh!

chapuni commented Nov 18, 2024

Uh oh!

ellishg commented Nov 18, 2024

Uh oh!

Uh oh!

llvm-ci commented Dec 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

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

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

Uh oh!

Conversation

chapuni commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

chapuni left a comment

Choose a reason for hiding this comment

Uh oh!

chapuni Oct 5, 2024

Choose a reason for hiding this comment

Uh oh!

chapuni Dec 23, 2024

Choose a reason for hiding this comment

Uh oh!

ellishg left a comment

Choose a reason for hiding this comment

Uh oh!

chapuni commented Oct 9, 2024

Uh oh!

chapuni commented Nov 18, 2024

Uh oh!

ellishg commented Nov 18, 2024

Uh oh!

Uh oh!

llvm-ci commented Dec 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

chapuni commented Oct 3, 2024 •

edited

Loading

llvmbot commented Oct 3, 2024 •

edited

Loading