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 by sortNestedRegions(), + // we expect the following conditions: + // * 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 need to add counts only for CodeRegions because: + // * ExapnsionRegions will inherit their counts from the outer region; + // * 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 containing the current region. + 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: 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());