Skip to content

Commit 4e48db5

Browse files
committed
[Profiler] Improve region termination
We can terminate all the regions up to the last AST node in the stack, since regions without AST nodes are refinements of the region with the AST node, and should be terminated the same. This avoids leaving some regions that extend past e.g the `return` of a function. The region in the test case that changes here is: ``` [[@line+9]]:28 -> [[@line+12]]:4 : (0 - 1) ``` this was extending past the return. Now it is: ``` [[@line]]:6 -> [[@line+4]]:11 : (0 - 1) ``` Apologies, I also refactored the test case at the same time which makes the difference harder to see, but the main point is that this region now terminates at the return, the same as the others.
1 parent bb48233 commit 4e48db5

File tree

2 files changed

+24
-22
lines changed

2 files changed

+24
-22
lines changed

lib/SIL/IR/SILProfiler.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,9 +1057,18 @@ struct CoverageMapping : public ASTWalker {
10571057

10581058
/// Mark \c S as a terminator, starting a zero region.
10591059
void terminateRegion(ASTNode S) {
1060-
SourceMappingRegion &Region = getRegion();
1061-
if (!Region.hasEndLoc())
1062-
Region.setEndLoc(getEndLoc(S));
1060+
assert(!RegionStack.empty() && "Cannot terminate non-existant region");
1061+
1062+
// Walk up the region stack and cut short regions until we reach a region
1063+
// for an AST node. This ensures we correctly handle new regions that have
1064+
// been introduced as a result of replacing the count, e.g if errors have
1065+
// been thrown.
1066+
for (auto &Region : llvm::reverse(RegionStack)) {
1067+
if (!Region.hasEndLoc())
1068+
Region.setEndLoc(getEndLoc(S));
1069+
if (Region.getNode())
1070+
break;
1071+
}
10631072
replaceCount(CounterExpr::Zero(), /*Start*/ llvm::None);
10641073
}
10651074

test/Profiler/coverage_do.swift

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,19 @@
1818
// CHECK-NEXT: increment_profiler_counter 1
1919

2020
// CHECK-LABEL: sil_coverage_map {{.*}} "$s11coverage_do3fooyyF"
21-
// CHECK-NEXT: [[@LINE+11]]:12 -> [[@LINE+18]]:2 : 0
22-
// CHECK-NEXT: [[@LINE+11]]:9 -> [[@LINE+15]]:4 : 0
23-
// CHECK-NEXT: [[@LINE+11]]:8 -> [[@LINE+11]]:17 : 0
24-
// CHECK-NEXT: [[@LINE+10]]:18 -> [[@LINE+10]]:28 : 1
25-
// CHECK-NEXT: [[@LINE+9]]:28 -> [[@LINE+12]]:4 : (0 - 1)
26-
// CHECK-NEXT: [[@LINE+9]]:8 -> [[@LINE+9]]:17 : (0 - 1)
27-
// CHECK-NEXT: [[@LINE+8]]:18 -> [[@LINE+8]]:29 : 2
28-
// CHECK-NEXT: [[@LINE+7]]:29 -> [[@LINE+8]]:11 : ((0 - 1) - 2)
29-
// CHECK-NEXT: [[@LINE+8]]:4 -> [[@LINE+10]]:2 : 2
30-
// CHECK-NEXT: [[@LINE+8]]:6 -> [[@LINE+8]]:8 : 2
31-
// CHECK-NEXT: [[@LINE+7]]:8 -> [[@LINE+8]]:2 : 2
32-
func foo() {
33-
x: do {
34-
if .random() { return }
35-
if .random() { break x }
21+
func foo() { // CHECK-NEXT: [[@LINE]]:12 -> [[@LINE+11]]:2 : 0
22+
x: do { // CHECK-NEXT: [[@LINE]]:9 -> [[@LINE+8]]:4 : 0
23+
if .random() { // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE]]:17 : 0
24+
return // CHECK-NEXT: [[@LINE-1]]:18 -> [[@LINE+1]]:6 : 1
25+
} // CHECK-NEXT: [[@LINE]]:6 -> [[@LINE+4]]:11 : (0 - 1)
26+
if .random() { // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE]]:17 : (0 - 1)
27+
break x // CHECK-NEXT: [[@LINE-1]]:18 -> [[@LINE+1]]:6 : 2
28+
} // CHECK-NEXT: [[@LINE]]:6 -> [[@LINE+1]]:11 : ((0 - 1) - 2)
3629
return
37-
}
38-
do {}
39-
}
40-
// CHECK-NEXT: }
30+
} // CHECK-NEXT: [[@LINE]]:4 -> [[@LINE+2]]:2 : 2
31+
do {} // CHECK-NEXT: [[@LINE]]:6 -> [[@LINE]]:8 : 2
32+
} // CHECK-NEXT: [[@LINE-1]]:8 -> [[@LINE]]:2 : 2
33+
// CHECK-NEXT: }
4134

4235
// CHECK-LABEL: sil_coverage_map {{.*}} "$s11coverage_do4foobyyF"
4336
func foob() {

0 commit comments

Comments
 (0)