-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Coverage] Ignore unused functions if the count is 0. #107661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-pgo Author: Zequan Wu (ZequanWu) ChangesRelax the condition to ignore the case when count is 0. This fixes a bug on 381e9d2. This was reported at https://discourse.llvm.org/t/coverage-from-multiple-test-executables/81024/. Full diff: https://github.com/llvm/llvm-project/pull/107661.diff 2 Files Affected:
diff --git a/compiler-rt/test/profile/instrprof-merging-2.cpp b/compiler-rt/test/profile/instrprof-merging-2.cpp
new file mode 100644
index 00000000000000..4c2579804774f1
--- /dev/null
+++ b/compiler-rt/test/profile/instrprof-merging-2.cpp
@@ -0,0 +1,54 @@
+// UNSUPPORTED: target={{.*windows.*}}
+
+// RUN: split-file %s %t
+// RUN: %clangxx_profgen -fcoverage-mapping %t/test1.cpp -o %t/test1.exe
+// RUN: %clangxx_profgen -fcoverage-mapping %t/test2.cpp -o %t/test2.exe
+// RUN: env LLVM_PROFILE_FILE=%t/test1.profraw %run %t/test1.exe
+// RUN: env LLVM_PROFILE_FILE=%t/test2.profraw %run %t/test2.exe
+// RUN: llvm-profdata merge %t/test1.profraw %t/test2.profraw -o %t/merged.profdata
+// RUN: llvm-cov show -instr-profile=%t/merged.profdata -object %t/test1.exe %t/test2.exe | FileCheck %s
+// RUN: llvm-cov show -instr-profile=%t/merged.profdata -object %t/test2.exe %t/test1.exe | FileCheck %s
+
+// CHECK: |struct Test {
+// CHECK-NEXT: 1| int getToTest() {
+// CHECK-NEXT: 2| for (int i = 0; i < 1; i++) {
+// CHECK-NEXT: 1| if (false) {
+// CHECK-NEXT: 0| return 1;
+// CHECK-NEXT: 0| }
+// CHECK-NEXT: 1| }
+// CHECK-NEXT: 1| if (true) {
+// CHECK-NEXT: 1| return 1;
+// CHECK-NEXT: 1| }
+// CHECK-NEXT: 0| return 1;
+// CHECK-NEXT: 1| }
+// CHECK-NEXT: |};
+// CHECK-NEXT: |
+
+#--- test.h
+struct Test {
+ int getToTest() {
+ for (int i = 0; i < 1; i++) {
+ if (false) {
+ return 1;
+ }
+ }
+ if (true) {
+ return 1;
+ }
+ return 1;
+ }
+};
+
+#--- test1.cpp
+#include "test.h"
+int main() {
+ Test t;
+ t.getToTest();
+ return 0;
+}
+
+#--- test2.cpp
+#include "test.h"
+int main() {
+ return 0;
+}
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 21ce0ac17d6186..eeef78a822008b 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -851,7 +851,7 @@ Error CoverageMapping::loadFunctionRecord(
// won't (in which case we don't unintuitively report functions as uncovered
// when they have non-zero counts in the profile).
if (Record.MappingRegions.size() == 1 &&
- Record.MappingRegions[0].Count.isZero() && Counts[0] > 0)
+ Record.MappingRegions[0].Count.isZero())
return Error::success();
MCDCDecisionRecorder MCDCDecisions;
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/4778 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/4780 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/175/builds/4881 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/185/builds/4870 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/2/builds/6294 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/6574 Here is the relevant piece of the build log for the reference |
Reverts #107661 Breaks llvm-project/llvm/unittests/ProfileData/CoverageMappingTest.cpp
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/33/builds/2684 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/8314 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/6891 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/7101 Here is the relevant piece of the build log for the reference |
Relax the condition to ignore the case when count is 0.
This fixes a bug on 381e9d2. This was reported at https://discourse.llvm.org/t/coverage-from-multiple-test-executables/81024/.