[Coverage][Single] Round Counters to boolean after evaluation#110972
Merged
[Coverage][Single] Round Counters to boolean after evaluation#110972
Conversation
`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.
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-pgo Author: NAKAMURA Takumi (chapuni) Changes
Full diff: https://github.com/llvm/llvm-project/pull/110972.diff 6 Files Affected:
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
reviewed
Oct 3, 2024
chapuni
changed the base branch from
users/chapuni/cov/single/refactor
to
main
chapuni
commented
Oct 5, 2024
| Function.pushRegion( | ||
| Region, (SingleByteCoverage && *ExecutionCount ? 1 : *ExecutionCount), | ||
| (SingleByteCoverage && *AltExecutionCount ? 1 : *AltExecutionCount), | ||
| SingleByteCoverage); |
chapuni
changed the title
ellishg
reviewed
Oct 9, 2024
Contributor
ellishg
left a comment
ellishg
left a comment
There was a problem hiding this comment.
Looks ok to me, but can you add a test?
Contributor
Author
Contributor
Author
|
@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 That said, I can postpend committing this after practical changes in singlebytecoverage. They will be good testcases. |
Contributor
Adding tests later should be fine. Thanks! |
And reformat. NFC.
ellishg
approved these changes
Nov 20, 2024
This was referenced Nov 21, 2024
Conflicts: llvm/test/tools/llvm-cov/branch-macros.cpp
…ingle/merge Conflicts: llvm/test/tools/llvm-cov/Inputs/showLineExecutionCounts.cpp
chapuni
added a commit
that referenced
this pull request
Dec 23, 2024
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 Buildbot has detected a new failure on builder 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rounding in merging segments has been done after #75425.
Depends on: #113114