Index: lib/ProfileData/CoverageMapping.cpp =================================================================== --- lib/ProfileData/CoverageMapping.cpp +++ lib/ProfileData/CoverageMapping.cpp @@ -334,17 +334,28 @@ /// Sort a nested sequence of regions from a single file. static void sortNestedRegions(MutableArrayRef 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 region, prefer CodeRegion to + // ExpansionRegion, and ExpansionRegion to SkippedRegion. + // That feature is required for correct working of + // combineRegions() and fixExpansionRegions(). + 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. - static ArrayRef + static MutableArrayRef combineRegions(MutableArrayRef Regions) { if (Regions.empty()) return Regions; @@ -360,20 +371,40 @@ 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. + // As long as Regions are sorted with sortNestedRegions(), + // we expect that the following conditions are met: + // * If *I is CodeRegion, *Active is CodeRegion; + // * If *Active is ExpansionRegion, *I cannot be CodeRegion; + // * If *Active is SkippedRegion, *I can only be SkippedRegion. + // We add counts only for CodeRegions because: + // * ExapnsionRegions will inherit their counts from their outer regions; + // * SkippedRegions do not need to be combined at all. + if (I->Kind == coverage::CounterMappingRegion::CodeRegion) { + assert(Active->Kind == coverage::CounterMappingRegion::CodeRegion); Active->ExecutionCount += I->ExecutionCount; + } } return Regions.drop_back(std::distance(++Active, End)); } + /// Set counts of ExpansionRegions to the value of the outer region. + static void fixExpansionRegions(MutableArrayRef Regions) { + for (auto Begin = Regions.begin(), I = Regions.begin(), End = Regions.end(); + I != End; ++I) { + if (I->Kind != coverage::CounterMappingRegion::ExpansionRegion) + continue; + // Find the nearest region which contains the current. + for (auto Outer = I; Outer != Begin;) { + --Outer; + assert(Outer->startLoc() <= I->startLoc()); + if (Outer->endLoc() >= I->endLoc()) { + I->ExecutionCount = Outer->ExecutionCount; + break; + } + } + } + } + public: /// Build a list of CoverageSegments from a list of Regions. static std::vector @@ -382,9 +413,10 @@ SegmentBuilder Builder(Segments); sortNestedRegions(Regions); - ArrayRef CombinedRegions = combineRegions(Regions); + Regions = combineRegions(Regions); + fixExpansionRegions(Regions); - Builder.buildSegmentsImpl(CombinedRegions); + Builder.buildSegmentsImpl(Regions); return Segments; } }; Index: test/tools/llvm-cov/Inputs/expansion-counts.proftext =================================================================== --- /dev/null +++ test/tools/llvm-cov/Inputs/expansion-counts.proftext @@ -0,0 +1,12 @@ +main +# Func Hash: +1065220 +# Num Counters: +5 +# Counter Values: +1 +2 +2 +2 +2 + Index: test/tools/llvm-cov/expansion-counts.cpp =================================================================== --- /dev/null +++ test/tools/llvm-cov/expansion-counts.cpp @@ -0,0 +1,39 @@ +// Check that expansion regions use counts of their outer regions. + +// RUN: llvm-profdata merge %S/Inputs/expansion-counts.proftext -o %t.profdata +// RUN: llvm-cov show %S/Inputs/expansion-counts.covmapping -instr-profile %t.profdata -filename-equivalence %s | FileCheck %s + +#define DO_LOOP1() \ + for (int K = 0; K < 2; ++K) {} +// CHECK: | [[@LINE-2]]|#define DO_LOOP1() +// CHECK-NEXT: 3| [[@LINE-2]]| for (int K = 0; K < 2; ++K) {} + +#define DO_LOOP2() \ + for (int K = 0; K < 2; ++K) {} +// CHECK: | [[@LINE-2]]|#define DO_LOOP2() +// CHECK-NEXT: 3| [[@LINE-2]]| for (int K = 0; K < 2; ++K) {} + +#define DO_LOOP3() \ + for (int K = 0; K < 2; ++K) {} +// CHECK: | [[@LINE-2]]|#define DO_LOOP3() +// CHECK-NEXT: 6| [[@LINE-2]]| for (int K = 0; K < 2; ++K) {} + +#define JUST_EXPAND_LOOP2() DO_LOOP2() +// CHECK: 1| [[@LINE-1]]|#define JUST_EXPAND_LOOP2() DO_LOOP2() + +#define DO_SOMETHING() \ + { \ + DO_LOOP3(); \ + } +// CHECK: | [[@LINE-4]]|#define DO_SOMETHING() +// CHECK-NEXT: 2| [[@LINE-4]]| { +// CHECK-NEXT: 2| [[@LINE-4]]| DO_LOOP3(); +// CHECK-NEXT: 2| [[@LINE-4]]| } + +int main() { // CHECK: 1| [[@LINE]]|int main() { + DO_LOOP1(); // CHECK-NEXT: 1| [[@LINE]]| DO_LOOP1(); + JUST_EXPAND_LOOP2(); // CHECK-NEXT: 1| [[@LINE]]| JUST_EXPAND_LOOP2(); + 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]]|} Index: unittests/ProfileData/CoverageMappingTest.cpp =================================================================== --- unittests/ProfileData/CoverageMappingTest.cpp +++ unittests/ProfileData/CoverageMappingTest.cpp @@ -512,6 +512,36 @@ 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)); + + 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 DataI = LoadedCoverage->getCoverageForFile("include"); + std::vector SegmentsI(DataI.begin(), DataI.end()); + ASSERT_EQ(2U, SegmentsI.size()); + EXPECT_EQ(CoverageSegment(1, 1, 20, true), SegmentsI[0]); + EXPECT_EQ(CoverageSegment(1, 10, false), SegmentsI[1]); + + // Despite the fact that the count value for the segment in "include" is "20", + // the corresponding segment in "file" should have value "10", inheriting + // it from its outer region. + CoverageData DataF = LoadedCoverage->getCoverageForFile("file"); + std::vector SegmentsF(DataF.begin(), DataF.end()); + ASSERT_EQ(4U, SegmentsF.size()); + EXPECT_EQ(CoverageSegment(1, 1, 10, true), SegmentsF[0]); + EXPECT_EQ(CoverageSegment(3, 1, 10, true), SegmentsF[1]); + EXPECT_EQ(CoverageSegment(3, 5, 10, false), SegmentsF[2]); + EXPECT_EQ(CoverageSegment(5, 5, false), SegmentsF[3]); +} + INSTANTIATE_TEST_CASE_P(MaybeSparse, MaybeSparseCoverageMappingTest, ::testing::Bool());