[PATCH] D36014: [llvm-cov] Ignore unclosed line segments when setting line counts
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 3 11:16:28 PDT 2017
Vedant Kumar via Phabricator <reviews at reviews.llvm.org> writes:
> vsk created this revision.
>
> This patch makes a slight change to the way llvm-cov determines line
> execution counts. If there are multiple line segments on a line, the
> line count is the max count among the regions which start *and* end on
> the line. This avoids an issue posed by deferred regions which start on
> the same line as a terminated region, e.g:
>
> if (false)
> return; //< The line count should be 0, even though a new region
> //< starts at the semi-colon.
> foo();
>
> Another change is that counts from line segments which don't correspond
> to region entries are considered. This enables the first change, and
> corrects an outstanding issue (see the showLineExecutionCounts.cpp test
> change).
LGTM.
> This is related to https://reviews.llvm.org/D35925.
>
> Testing: check-profile, llvm-cov lit tests
>
>
> https://reviews.llvm.org/D36014
>
> Files:
> test/tools/llvm-cov/Inputs/deferred-regions.covmapping
> test/tools/llvm-cov/Inputs/deferred-regions.profdata
> test/tools/llvm-cov/deferred-region.cpp
> test/tools/llvm-cov/showLineExecutionCounts.cpp
> tools/llvm-cov/SourceCoverageView.cpp
> tools/llvm-cov/SourceCoverageView.h
>
> Index: tools/llvm-cov/SourceCoverageView.h
> ===================================================================
> --- tools/llvm-cov/SourceCoverageView.h
> +++ tools/llvm-cov/SourceCoverageView.h
> @@ -67,25 +67,15 @@
> /// \brief Coverage statistics for a single line.
> struct LineCoverageStats {
> uint64_t ExecutionCount;
> - unsigned RegionCount;
> + bool HasMultipleRegions;
> bool Mapped;
>
> - LineCoverageStats() : ExecutionCount(0), RegionCount(0), Mapped(false) {}
> + LineCoverageStats(ArrayRef<const coverage::CoverageSegment *> LineSegments,
> + const coverage::CoverageSegment *WrappedSegment);
>
> bool isMapped() const { return Mapped; }
>
> - bool hasMultipleRegions() const { return RegionCount > 1; }
> -
> - void addRegionStartCount(uint64_t Count) {
> - // The max of all region starts is the most interesting value.
> - addRegionCount(RegionCount ? std::max(ExecutionCount, Count) : Count);
> - ++RegionCount;
> - }
> -
> - void addRegionCount(uint64_t Count) {
> - Mapped = true;
> - ExecutionCount = Count;
> - }
> + bool hasMultipleRegions() const { return HasMultipleRegions; }
> };
>
> /// \brief A file manager that handles format-aware file creation.
> Index: tools/llvm-cov/SourceCoverageView.cpp
> ===================================================================
> --- tools/llvm-cov/SourceCoverageView.cpp
> +++ tools/llvm-cov/SourceCoverageView.cpp
> @@ -83,6 +83,41 @@
> llvm_unreachable("Unknown coverage output format!");
> }
>
> +LineCoverageStats::LineCoverageStats(
> + ArrayRef<const coverage::CoverageSegment *> LineSegments,
> + const coverage::CoverageSegment *WrappedSegment) {
> + // Find the minimum number of regions which start in this line.
> + unsigned MinRegionCount = 0;
> + auto isStartOfRegion = [](const coverage::CoverageSegment *S) {
> + return S->HasCount && S->IsRegionEntry;
> + };
> + for (unsigned I = 0; I < LineSegments.size() && MinRegionCount < 2; ++I)
> + if (isStartOfRegion(LineSegments[I]))
> + ++MinRegionCount;
> +
> + ExecutionCount = 0;
> + HasMultipleRegions = MinRegionCount > 1;
> + Mapped = (WrappedSegment && WrappedSegment->HasCount) || (MinRegionCount > 0);
> +
> + if (!Mapped)
> + return;
> +
> + // Pick the max count among regions which start and end on this line, to
> + // avoid erroneously using the wrapped count, and to avoid picking region
> + // counts which come from deferred regions.
> + if (LineSegments.size() > 1) {
> + for (unsigned I = 0; I < LineSegments.size() - 1; ++I)
> + ExecutionCount = std::max(ExecutionCount, LineSegments[I]->Count);
> + return;
> + }
> +
> + // Just pick the maximum count.
> + if (WrappedSegment && WrappedSegment->HasCount)
> + ExecutionCount = WrappedSegment->Count;
> + if (!LineSegments.empty())
> + ExecutionCount = std::max(ExecutionCount, LineSegments[0]->Count);
> +}
> +
> unsigned SourceCoverageView::getFirstUncoveredLineNo() {
> auto CheckIfUncovered = [](const coverage::CoverageSegment &S) {
> return S.HasCount && S.Count == 0;
> @@ -207,17 +242,11 @@
> while (NextSegment != EndSegment && NextSegment->Line == LI.line_number())
> LineSegments.push_back(&*NextSegment++);
>
> - // Calculate a count to be for the line as a whole.
> - LineCoverageStats LineCount;
> - if (WrappedSegment && WrappedSegment->HasCount)
> - LineCount.addRegionCount(WrappedSegment->Count);
> - for (const auto *S : LineSegments)
> - if (S->HasCount && S->IsRegionEntry)
> - LineCount.addRegionStartCount(S->Count);
> -
> renderLinePrefix(OS, ViewDepth);
> if (getOptions().ShowLineNumbers)
> renderLineNumberColumn(OS, LI.line_number());
> +
> + LineCoverageStats LineCount{LineSegments, WrappedSegment};
> if (getOptions().ShowLineStats)
> renderLineCoverageColumn(OS, LineCount);
>
> Index: test/tools/llvm-cov/showLineExecutionCounts.cpp
> ===================================================================
> --- test/tools/llvm-cov/showLineExecutionCounts.cpp
> +++ test/tools/llvm-cov/showLineExecutionCounts.cpp
> @@ -6,7 +6,7 @@
> int main() { // TEXT: [[@LINE]]| 161|int main(
> int x = 0; // TEXT: [[@LINE]]| 161| int x
> // TEXT: [[@LINE]]| 161|
> - if (x) { // TEXT: [[@LINE]]| 0| if (x)
> + if (x) { // TEXT: [[@LINE]]| 161| if (x)
> x = 0; // TEXT: [[@LINE]]| 0| x = 0
> } else { // TEXT: [[@LINE]]| 161| } else
> x = 1; // TEXT: [[@LINE]]| 161| x = 1
> @@ -50,7 +50,7 @@
> // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre>int main() {
> // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre> int x = 0
> // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre>
> -// HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='uncovered-line'><pre>0</pre></td><td class='code'><pre> if (x) {
> +// HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre> if (x) {
> // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='uncovered-line'><pre>0</pre></td><td class='code'><pre>
> // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre><span class='red'> }</span>
> // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre> x = 1;
> Index: test/tools/llvm-cov/deferred-region.cpp
> ===================================================================
> --- /dev/null
> +++ test/tools/llvm-cov/deferred-region.cpp
> @@ -0,0 +1,79 @@
> +// RUN: llvm-cov show %S/Inputs/deferred-regions.covmapping -instr-profile %S/Inputs/deferred-regions.profdata -show-line-counts-or-regions -dump -filename-equivalence %s 2>&1 | FileCheck %s
> +
> +void foo(int x) {
> + if (x == 0) {
> + return; // CHECK: [[@LINE]]|{{ +}}1|
> + }
> +
> +} // CHECK: [[@LINE]]|{{ +}}2|
> +
> +void bar() {
> + return;
> +
> +} // CHECK: [[@LINE]]|{{ +}}1|
> +
> +void for_loop() {
> + if (false)
> + return; // CHECK: [[@LINE]]|{{ +}}0|
> +
> + for (int i = 0; i < 10; ++i) { // CHECK: [[@LINE]]|{{ +}}2|
> + if (i % 2 == 0)
> + continue; // CHECK: [[@LINE]]|{{ +}}1|
> +
> + if (i % 5 == 0)
> + break; // CHECK: [[@LINE]]|{{ +}}0|
> +
> + int x = i;
> + return; // CHECK: [[@LINE]]|{{ +}}1|
> +
> + } // CHECK: [[@LINE]]|{{ +}}1|
> +}
> +
> +struct Error {};
> +
> +void while_loop() {
> + if (false)
> + return; // CHECK: [[@LINE]]|{{ +}}0|
> +
> + int x = 0;
> + while (++x < 10) { // CHECK: [[@LINE]]|{{ +}}3|
> + if (x == 1)
> + continue; // CHECK: [[@LINE]]|{{ +}}1|
> +
> + while (++x < 4) { // CHECK: [[@LINE]]|{{ +}}1|
> + if (x == 3)
> + break; // CHECK: [[@LINE]]|{{ +}}1|
> + // CHECK: [[@LINE]]|{{ +}}0|
> + while (++x < 5) {} // CHECK: [[@LINE]]|{{ +}}0|
> + } // CHECK: [[@LINE]]|{{ +}}1|
> +
> + if (x == 0)
> + throw Error(); // CHECK: [[@LINE]]|{{ +}}0|
> +
> + while (++x < 9) { // CHECK: [[@LINE]]|{{ +}}6|
> + if (x == 0) // CHECK: [[@LINE]]|{{ +}}5|
> + break; // CHECK: [[@LINE]]|{{ +}}0|
> +
> + }
> + }
> +}
> +
> +void gotos() {
> + if (false)
> + goto out; // CHECK: [[@LINE]]|{{ +}}0|
> +
> + return;
> +
> +out: // CHECK: [[@LINE]]|{{ +}}1|
> + return;
> +}
> +
> +int main() {
> + foo(0);
> + foo(1);
> + bar();
> + for_loop();
> + while_loop();
> + gotos();
> + return 0;
> +}
More information about the llvm-commits
mailing list