[PATCH] D102193: Fix branch coverage merging across function instantiations

[Alan Phipps via Phabricator via llvm-commits]

alanphipps created this revision.
alanphipps added reviewers: snidertm, vsk.
alanphipps requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

When I made the upstream commit for branch coverage support in code-coverage in January, I made an incorrect assumption that branch coverage numbers (covered branch and total branches) should be accumulated across function instantiations.  This is not what is done for lines and regions, where the maximum line/region coverage found in a function instantiation across an instantiation group is returned.  For example, if a function template definition has 2 total branches, the branch coverage would be reported as an accumulated total across all instantiations (8 total branches across 4 instantiations).

The correct assumption for the FunctionCoverageSummary::get(const InstantiationGroup &Group, ...) routine is that the summary it returns should agree with the function definition in the source code on lines, regions, and branches.  So we should do the same thing for branch coverage as we do for line and region coverage.  So if a function template definition has 2 total branches, the summary should also reflect branch coverage for 2 total branches.

This change corrects the implementation for the branch coverage summary to do the same thing for branches that is done for lines and regions.  That is, across function instantiations in an instantiation group, the maximum branch coverage found in any of those instantiations is returned, with the total number of branches being the same across instantiations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102193

Files:
  llvm/test/tools/llvm-cov/branch-templates.cpp
  llvm/tools/llvm-cov/CoverageSummaryInfo.cpp
  llvm/tools/llvm-cov/CoverageSummaryInfo.h


Index: llvm/tools/llvm-cov/CoverageSummaryInfo.h
===================================================================
--- llvm/tools/llvm-cov/CoverageSummaryInfo.h
+++ llvm/tools/llvm-cov/CoverageSummaryInfo.h
@@ -123,6 +123,11 @@
     return *this;
   }
 
+  void merge(const BranchCoverageInfo &RHS) {
+    Covered = std::max(Covered, RHS.Covered);
+    NumBranches = std::max(NumBranches, RHS.NumBranches);
+  }
+
   size_t getCovered() const { return Covered; }
 
   size_t getNumBranches() const { return NumBranches; }
Index: llvm/tools/llvm-cov/CoverageSummaryInfo.cpp
===================================================================
--- llvm/tools/llvm-cov/CoverageSummaryInfo.cpp
+++ llvm/tools/llvm-cov/CoverageSummaryInfo.cpp
@@ -100,11 +100,7 @@
   for (const auto &FCS : Summaries.drop_front()) {
     Summary.RegionCoverage.merge(FCS.RegionCoverage);
     Summary.LineCoverage.merge(FCS.LineCoverage);
-
-    // Sum branch coverage across instantiation groups for the summary rather
-    // than "merge" the maximum count. This is a clearer view into whether all
-    // created branches are covered.
-    Summary.BranchCoverage += FCS.BranchCoverage;
+    Summary.BranchCoverage.merge(FCS.BranchCoverage);
   }
   return Summary;
 }
Index: llvm/test/tools/llvm-cov/branch-templates.cpp
===================================================================
--- llvm/test/tools/llvm-cov/branch-templates.cpp
+++ llvm/test/tools/llvm-cov/branch-templates.cpp
@@ -1,9 +1,9 @@
 // RUN: llvm-profdata merge %S/Inputs/branch-templates.proftext -o %t.profdata
 // RUN: llvm-cov show --show-expansions --show-branches=count %S/Inputs/branch-templates.o32l -instr-profile %t.profdata -path-equivalence=/tmp,%S %s | FileCheck %s
 // RUN: llvm-cov report --show-branch-summary %S/Inputs/branch-templates.o32l -instr-profile %t.profdata -show-functions -path-equivalence=/tmp,%S %s | FileCheck %s -check-prefix=REPORT
+// RUN: llvm-cov report --show-branch-summary %S/Inputs/branch-templates.o32l -instr-profile %t.profdata -path-equivalence=/tmp,%S %s | FileCheck %s -check-prefix=REPORTFILE
 
 #include <stdio.h>
-
 template<typename T>
 void unused(T x) {
   return;
@@ -45,3 +45,17 @@
 // REPORT-NEXT: _Z4funcIfEiT_                     5       2  60.00%         7       3  57.14%         2       1  50.00%
 // REPORT-NEXT: ---
 // REPORT-NEXT: TOTAL                            22       7  68.18%        31      11  64.52%        12       6  50.00%
+
+// Make sure the covered branch tally for the function instantiation group is
+// merged to reflect maximum branch coverage of a single instantiation, just
+// like what is done for lines and regions. Also, the total branch tally
+// summary for an instantiation group should agree with the total number of
+// branches in the definition (In this case, 2 and 6 for func<>() and main(),
+// respectively).  This is returned by: FunctionCoverageSummary::get(const
+// InstantiationGroup &Group, ...)
+
+// REPORTFILE: Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
+// REPORTFILE-NEXT: ---
+// REPORTFILE-NEXT: /tmp/branch-templates.cpp          12                 3    75.00%           2                 0   100.00%          17                 4    76.47%           8                 4    50.00%
+// REPORTFILE-NEXT: ---
+// REPORTFILE-NEXT: TOTAL                              12                 3    75.00%           2                 0   100.00%          17                 4    76.47%           8                 4    50.00%


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D102193.344173.patch
Type: text/x-patch
Size: 3623 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210510/13fe53a4/attachment.bin>