[Coverage] Combine counts of expansion regions if there are no code regions for the same area.

Differential Revision: http://reviews.llvm.org/D18831


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@268620 91177308-0d34-0410-b5e6-96231b3b80d8

lib/ProfileData/Coverage/CoverageMapping.cpp

@@ -334,13 +334,25 @@ class SegmentBuilder {
 
   /// Sort a nested sequence of regions from a single file.
   static void sortNestedRegions(MutableArrayRef<CountedRegion> Regions) {
-    std::sort(Regions.begin(), Regions.end(),
-              [](const CountedRegion &LHS, const CountedRegion &RHS) {
-                if (LHS.startLoc() == RHS.startLoc())
-                  // When LHS completely contains RHS, we sort LHS first.
-                  return RHS.endLoc() < LHS.endLoc();
-                return LHS.startLoc() < RHS.startLoc();
-              });
+    std::sort(Regions.begin(), Regions.end(), [](const CountedRegion &LHS,
+                                                 const CountedRegion &RHS) {
+      if (LHS.startLoc() != RHS.startLoc())
+        return LHS.startLoc() < RHS.startLoc();
+      if (LHS.endLoc() != RHS.endLoc())
+        // When LHS completely contains RHS, we sort LHS first.
+        return RHS.endLoc() < LHS.endLoc();
+      // If LHS and RHS cover the same area, we need to sort them according
+      // to their kinds so that the most suitable region will become "active"
+      // in combineRegions(). Because we accumulate counter values only from
+      // regions of the same kind as the first region of the area, prefer
+      // CodeRegion to ExpansionRegion and ExpansionRegion to SkippedRegion.
+      static_assert(coverage::CounterMappingRegion::CodeRegion <
+                            coverage::CounterMappingRegion::ExpansionRegion &&
+                        coverage::CounterMappingRegion::ExpansionRegion <
+                            coverage::CounterMappingRegion::SkippedRegion,
+                    "Unexpected order of region kind values");
+      return LHS.Kind < RHS.Kind;
+    });
   }
 
   /// Combine counts of regions which cover the same area.
@@ -360,15 +372,18 @@ class SegmentBuilder {
         continue;
       }
       // Merge duplicate region.
-      if (I->Kind != coverage::CounterMappingRegion::CodeRegion)
-        // Add counts only from CodeRegions.
-        continue;
-      if (Active->Kind == coverage::CounterMappingRegion::SkippedRegion)
-        // We have to overwrite SkippedRegions because of special handling
-        // of them in startSegment().
-        *Active = *I;
-      else
-        // Otherwise, just append the count.
+      // If CodeRegions and ExpansionRegions cover the same area, it's probably
+      // a macro which is fully expanded to another macro. In that case, we need
+      // to accumulate counts only from CodeRegions, or else the area will be
+      // counted twice.
+      // On the other hand, a macro may have a nested macro in its body. If the
+      // outer macro is used several times, the ExpansionRegion for the nested
+      // macro will also be added several times. These ExpansionRegions cover
+      // the same source locations and have to be combined to reach the correct
+      // value for that area.
+      // We add counts of the regions of the same kind as the active region
+      // to handle the both situations.
+      if (I->Kind == Active->Kind)
         Active->ExecutionCount += I->ExecutionCount;
     }
     return Regions.drop_back(std::distance(++Active, End));

test/tools/llvm-cov/Inputs/combine_expansions.proftext

@@ -0,0 +1,8 @@
+main
+# Func Hash:
+0
+# Num Counters:
+1
+# Counter Values:
+1
+

test/tools/llvm-cov/combine_expansions.cpp

@@ -0,0 +1,26 @@
+// Check that we combine expansion regions.
+
+// RUN: llvm-profdata merge %S/Inputs/combine_expansions.proftext -o %t.profdata
+// RUN: llvm-cov show %S/Inputs/combine_expansions.covmapping -instr-profile %t.profdata -filename-equivalence %s | FileCheck %s
+
+#define SIMPLE_OP \
+  ++x
+// CHECK:       | [[@LINE-2]]|#define SIMPLE_OP
+// CHECK-NEXT: 2| [[@LINE-2]]|  ++x
+
+#define DO_SOMETHING \
+  {                  \
+    int x = 0;       \
+    SIMPLE_OP;       \
+  }
+// CHECK:       | [[@LINE-5]]|#define DO_SOMETHING
+// CHECK-NEXT: 2| [[@LINE-5]]|  {
+// CHECK-NEXT: 2| [[@LINE-5]]|    int x = 0;
+// CHECK-NEXT: 2| [[@LINE-5]]|    SIMPLE_OP;
+// CHECK-NEXT: 2| [[@LINE-5]]|  }
+
+int main() {    // CHECK:      1| [[@LINE]]|int main() {
+  DO_SOMETHING; // CHECK-NEXT: 1| [[@LINE]]|  DO_SOMETHING;
+  DO_SOMETHING; // CHECK-NEXT: 1| [[@LINE]]|  DO_SOMETHING;
+  return 0;     // CHECK-NEXT: 1| [[@LINE]]|  return 0;
+}               // CHECK-NEXT: 1| [[@LINE]]|}

unittests/ProfileData/CoverageMappingTest.cpp

@@ -422,6 +422,8 @@ TEST_P(MaybeSparseCoverageMappingTest,
   EXPECT_EQ(CoverageSegment(9, 9, false), Segments[3]);
 }
 
+// If CodeRegions and ExpansionRegions cover the same area,
+// only counts of CodeRegions should be used.
 TEST_P(MaybeSparseCoverageMappingTest, dont_combine_expansions) {
   InstrProfRecord Record1("func", 0x1234, {10, 20});
   InstrProfRecord Record2("func", 0x1234, {0, 0});
@@ -444,6 +446,29 @@ TEST_P(MaybeSparseCoverageMappingTest, dont_combine_expansions) {
   ASSERT_EQ(CoverageSegment(9, 9, false), Segments[3]);
 }
 
+// If an area is covered only by ExpansionRegions, they should be combinated.
+TEST_P(MaybeSparseCoverageMappingTest, combine_expansions) {
+  InstrProfRecord Record("func", 0x1234, {2, 3, 7});
+  NoError(ProfileWriter.addRecord(std::move(Record)));
+
+  startFunction("func", 0x1234);
+  addCMR(Counter::getCounter(1), "include1", 1, 1, 1, 10);
+  addCMR(Counter::getCounter(2), "include2", 1, 1, 1, 10);
+  addCMR(Counter::getCounter(0), "file", 1, 1, 5, 5);
+  addExpansionCMR("file", "include1", 3, 1, 3, 5);
+  addExpansionCMR("file", "include2", 3, 1, 3, 5);
+
+  loadCoverageMapping();
+
+  CoverageData Data = LoadedCoverage->getCoverageForFile("file");
+  std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
+  ASSERT_EQ(4U, Segments.size());
+  EXPECT_EQ(CoverageSegment(1, 1, 2, true), Segments[0]);
+  EXPECT_EQ(CoverageSegment(3, 1, 10, true), Segments[1]);
+  EXPECT_EQ(CoverageSegment(3, 5, 2, false), Segments[2]);
+  EXPECT_EQ(CoverageSegment(5, 5, false), Segments[3]);
+}
+
 TEST_P(MaybeSparseCoverageMappingTest, strip_filename_prefix) {
   InstrProfRecord Record("file1:func", 0x1234, {0});
   NoError(ProfileWriter.addRecord(std::move(Record)));