[PATCH] D18831: [Coverage] Use the count value of the outer region for an expansion region.

[Igor Kudrin via llvm-commits]

ikudrin created this revision.
ikudrin added reviewers: bogner, davidxl, vsk.
ikudrin added a subscriber: llvm-commits.

Because an expansion region lays inside some outer code region, it can just borrow
the count value of that region.

http://reviews.llvm.org/D18831

Files:
  llvm/trunk/lib/ProfileData/CoverageMapping.cpp
  llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp

Index: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
===================================================================
--- llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
+++ llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
@@ -510,6 +510,28 @@
   EXPECT_EQ(CoverageSegment(1, 10, false), Segments[1]);
 }
 
+TEST_P(MaybeSparseCoverageMappingTest,
+       expansion_region_uses_count_of_outer_region) {
+  InstrProfRecord Record("func", 0x1234, {10, 20});
+  ProfileWriter.addRecord(std::move(Record));
+  readProfCounts();
+
+  startFunction("func", 0x1234);
+  addCMR(Counter::getCounter(1), "include", 1, 1, 1, 10);
+  addCMR(Counter::getCounter(0), "file", 1, 1, 5, 5);
+  addExpansionCMR("file", "include", 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, 10, true), Segments[0]);
+  EXPECT_EQ(CoverageSegment(3, 1, 10, true), Segments[1]);
+  EXPECT_EQ(CoverageSegment(3, 5, 10, false), Segments[2]);
+  EXPECT_EQ(CoverageSegment(5, 5, false), Segments[3]);
+}
+
 INSTANTIATE_TEST_CASE_P(MaybeSparse, MaybeSparseCoverageMappingTest,
                         ::testing::Bool());
 
Index: llvm/trunk/lib/ProfileData/CoverageMapping.cpp
===================================================================
--- llvm/trunk/lib/ProfileData/CoverageMapping.cpp
+++ llvm/trunk/lib/ProfileData/CoverageMapping.cpp
@@ -324,7 +324,7 @@
       if (ReferenceRegion && ReferenceRegion->startLoc() == Region.startLoc() &&
           ReferenceRegion->endLoc() == Region.endLoc()) {
         if (Region.Kind == coverage::CounterMappingRegion::CodeRegion)
-          Count = Count.getValueOr(0) + Region.ExecutionCount;
+          Count = Count.getValue() + Region.ExecutionCount;
         continue;
       }
       // Add the previous region with the combined counter to the stack.
@@ -335,10 +335,17 @@
              ActiveRegions.back().first->endLoc() <= Region.startLoc())
         popRegion();
       ReferenceRegion = &Region;
-      if (Region.Kind != coverage::CounterMappingRegion::SkippedRegion)
+      switch (Region.Kind) {
+      case coverage::CounterMappingRegion::CodeRegion:
         Count = Region.ExecutionCount;
-      else
+        break;
+      case coverage::CounterMappingRegion::ExpansionRegion:
+        // Preserve the count of the outer region.
+        break;
+      case coverage::CounterMappingRegion::SkippedRegion:
         Count = None;
+        break;
+      }
     }
     // Add the last region to the stack.
     if (ReferenceRegion)
@@ -392,13 +399,23 @@
 
 /// Sort a nested sequence of regions from a single file.
 template <class It> static void sortNestedRegions(It First, It Last) {
-  std::sort(First, Last,
-            [](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(
+      First, Last, [](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 region,
+        // prefer CodeRegion to ExpansionRegion
+        // and ExpansionRegion to SkippedRegion.
+        static_assert(coverage::CounterMappingRegion::CodeRegion <
+                              coverage::CounterMappingRegion::ExpansionRegion &&
+                          coverage::CounterMappingRegion::ExpansionRegion <
+                              coverage::CounterMappingRegion::SkippedRegion,
+                      "Not expected order of region kind values");
+        return LHS.Kind < RHS.Kind;
+      });
 }
 
 static bool isExpansion(const CountedRegion &R, unsigned FileID) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D18831.52813.patch
Type: text/x-patch
Size: 4129 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160406/cab4f791/attachment.bin>