Skip to content

Commit bd6aa99

Browse files
hamishknighthamishknight
authored
Merge pull request #60775 from hamishknight/switchboard
2 parents 5574afc + 05e2309 commit bd6aa99

File tree

3 files changed

+180
-49
lines changed

3 files changed

+180
-49
lines changed

‎lib/SIL/IR/SILProfiler.cpp

Lines changed: 59 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,6 @@ struct MapRegionCounters : public ASTWalker {
259259
mapRegion(RWS->getBody());
260260
} else if (auto *FES = dyn_cast<ForEachStmt>(S)) {
261261
mapRegion(FES->getBody());
262-
} else if (auto *SS = dyn_cast<SwitchStmt>(S)) {
263-
mapRegion(SS);
264262
} else if (auto *CS = dyn_cast<CaseStmt>(S)) {
265263
mapRegion(getProfilerStmtForCase(CS));
266264
}
@@ -647,8 +645,6 @@ struct PGOMapping : public ASTWalker {
647645
} else if (auto *FES = dyn_cast<ForEachStmt>(S)) {
648646
setKnownExecutionCount(FES->getBody());
649647
setExecutionCount(FES, parentCount);
650-
} else if (auto *SS = dyn_cast<SwitchStmt>(S)) {
651-
setKnownExecutionCount(SS);
652648
} else if (auto *CS = dyn_cast<CaseStmt>(S)) {
653649
setKnownExecutionCount(getProfilerStmtForCase(CS));
654650
}
@@ -695,6 +691,9 @@ struct PGOMapping : public ASTWalker {
695691
}
696692
};
697693

694+
/// Produce coverage mapping information for a function. This involves taking
695+
/// the counters computed by MapRegionCounters, and annotating the source with
696+
/// regions that are defined in terms of those counters.
698697
struct CoverageMapping : public ASTWalker {
699698
private:
700699
const SourceManager &SM;
@@ -820,17 +819,7 @@ struct CoverageMapping : public ASTWalker {
820819
if (ControlFlowAdjust)
821820
Count = &createCounter(CounterExpr::Sub(*Count, *ControlFlowAdjust));
822821

823-
if (Count->isSemanticallyZero()) {
824-
// If the counter is semantically zero, form an 'incomplete' region with
825-
// no starting location. This prevents forming unreachable regions unless
826-
// there is a following statement or expression to extend the region.
827-
RegionStack.emplace_back(ASTNode(), *Count, None, None);
828-
} else {
829-
// Otherwise, we have a non-zero counter, so form a new region starting
830-
// at the end of the previous scope. This ensures the user covers both
831-
// branches of a condition.
832-
RegionStack.emplace_back(ASTNode(), *Count, getEndLoc(Scope), None);
833-
}
822+
replaceCount(Count, getEndLoc(Scope));
834823
}
835824

836825
/// Push a region covering \c Node onto the stack.
@@ -844,10 +833,24 @@ struct CoverageMapping : public ASTWalker {
844833
});
845834
}
846835

847-
/// Replace the current region's count by pushing an incomplete region.
848-
void replaceCount(CounterExpr &&Expr, Optional<SourceLoc> Start = None) {
849-
CounterExpr &Counter = createCounter(std::move(Expr));
850-
RegionStack.emplace_back(ASTNode(), Counter, Start, None);
836+
/// Replace the current region at \p Start with a new counter. If \p Start is
837+
/// \c None, or the counter is semantically zero, an 'incomplete' region is
838+
/// formed, which is not recorded unless followed by additional AST nodes.
839+
void replaceCount(CounterExpr *Counter, Optional<SourceLoc> Start) {
840+
// If the counter is semantically zero, form an 'incomplete' region with
841+
// no starting location. This prevents forming unreachable regions unless
842+
// there is a following statement or expression to extend the region.
843+
if (Start && Counter->isSemanticallyZero())
844+
Start = None;
845+
846+
RegionStack.emplace_back(ASTNode(), *Counter, Start, None);
847+
}
848+
849+
/// Replace the current region at \p Start with a new counter. If \p Start is
850+
/// \c None, or the counter is semantically zero, an 'incomplete' region is
851+
/// formed, which is not recorded unless followed by additional AST nodes.
852+
void replaceCount(CounterExpr &&Expr, Optional<SourceLoc> Start) {
853+
replaceCount(&createCounter(std::move(Expr)), Start);
851854
}
852855

853856
/// Get the location for the end of the last token in \c Node.
@@ -909,7 +912,7 @@ struct CoverageMapping : public ASTWalker {
909912
SourceMappingRegion &Region = getRegion();
910913
if (!Region.hasEndLoc())
911914
Region.setEndLoc(getEndLoc(S));
912-
replaceCount(CounterExpr::Zero());
915+
replaceCount(CounterExpr::Zero(), /*Start*/ None);
913916
}
914917

915918
Expr *getConditionNode(StmtCondition SC) {
@@ -976,6 +979,8 @@ struct CoverageMapping : public ASTWalker {
976979
if (S->isImplicit() && S != ImplicitTopLevelBody)
977980
return {true, S};
978981

982+
// If we're in an 'incomplete' region, update it to include this node. This
983+
// ensures we only create the region if needed.
979984
if (!RegionStack.empty())
980985
extendRegion(S);
981986

@@ -986,7 +991,13 @@ struct CoverageMapping : public ASTWalker {
986991
} else if (auto *IS = dyn_cast<IfStmt>(S)) {
987992
if (auto *Cond = getConditionNode(IS->getCond()))
988993
assignCounter(Cond, CounterExpr::Ref(getCurrentCounter()));
994+
995+
// The counter for the if statement itself tracks the number of jumps to
996+
// it by break statements.
989997
assignCounter(IS, CounterExpr::Zero());
998+
999+
// We emit a counter for the then block, and define the else block in
1000+
// terms of it.
9901001
CounterExpr &ThenCounter = assignCounter(IS->getThenStmt());
9911002
if (IS->getElseStmt())
9921003
assignCounter(IS->getElseStmt(),
@@ -996,23 +1007,38 @@ struct CoverageMapping : public ASTWalker {
9961007
assignCounter(GS->getBody());
9971008

9981009
} else if (auto *WS = dyn_cast<WhileStmt>(S)) {
1010+
// The counter for the while statement itself tracks the number of jumps
1011+
// to it by break and continue statements.
9991012
assignCounter(WS, CounterExpr::Zero());
1013+
10001014
if (auto *E = getConditionNode(WS->getCond()))
10011015
assignCounter(E, CounterExpr::Ref(getCurrentCounter()));
10021016
assignCounter(WS->getBody());
10031017

10041018
} else if (auto *RWS = dyn_cast<RepeatWhileStmt>(S)) {
1019+
// The counter for the while statement itself tracks the number of jumps
1020+
// to it by break and continue statements.
10051021
assignCounter(RWS, CounterExpr::Zero());
1022+
10061023
CounterExpr &BodyCounter = assignCounter(RWS->getBody());
10071024
assignCounter(RWS->getCond(), CounterExpr::Ref(BodyCounter));
10081025
RepeatWhileStack.push_back(RWS);
10091026

10101027
} else if (auto *FES = dyn_cast<ForEachStmt>(S)) {
1028+
// The counter for the for statement itself tracks the number of jumps
1029+
// to it by break and continue statements.
10111030
assignCounter(FES, CounterExpr::Zero());
10121031
assignCounter(FES->getBody());
10131032

10141033
} else if (auto *SS = dyn_cast<SwitchStmt>(S)) {
1015-
assignCounter(SS);
1034+
// The counter for the switch statement itself tracks the number of jumps
1035+
// to it by break statements, including the implicit breaks at the end of
1036+
// cases.
1037+
assignCounter(SS, CounterExpr::Zero());
1038+
1039+
assignCounter(SS->getSubjectExpr(),
1040+
CounterExpr::Ref(getCurrentCounter()));
1041+
10161042
// Assign counters for cases so they're available for fallthrough.
10171043
for (CaseStmt *Case : SS->getCases())
10181044
assignCounter(Case);
@@ -1021,7 +1047,10 @@ struct CoverageMapping : public ASTWalker {
10211047
if (caseStmt->getParentKind() == CaseParentKind::Switch)
10221048
pushRegion(S);
10231049
} else if (auto *DS = dyn_cast<DoStmt>(S)) {
1050+
// The counter for the do statement itself tracks the number of jumps
1051+
// to it by break statements.
10241052
assignCounter(DS, CounterExpr::Zero());
1053+
10251054
assignCounter(DS->getBody(), CounterExpr::Ref(getCurrentCounter()));
10261055

10271056
} else if (auto *DCS = dyn_cast<DoCatchStmt>(S)) {
@@ -1075,7 +1104,8 @@ struct CoverageMapping : public ASTWalker {
10751104
Stmt *BreakTarget = BS->getTarget();
10761105
if (auto *RWS = dyn_cast<RepeatWhileStmt>(BreakTarget)) {
10771106
subtractFromCounter(RWS->getCond(), getCurrentCounter());
1078-
} else if (!isa<SwitchStmt>(BreakTarget)) {
1107+
} else {
1108+
// Update the exit counter for the target.
10791109
addToCounter(BS->getTarget(), getCurrentCounter());
10801110
}
10811111

@@ -1093,9 +1123,12 @@ struct CoverageMapping : public ASTWalker {
10931123
replaceCount(CounterExpr::Ref(getCounter(S)), getEndLoc(S));
10941124

10951125
} else if (auto caseStmt = dyn_cast<CaseStmt>(S)) {
1096-
if (caseStmt->getParentKind() == CaseParentKind::Switch)
1126+
if (caseStmt->getParentKind() == CaseParentKind::Switch) {
1127+
// The end of a case block is an implicit break, update the exit
1128+
// counter to reflect this.
1129+
addToCounter(caseStmt->getParentStmt(), getCurrentCounter());
10971130
popRegions(S);
1098-
1131+
}
10991132
} else if (auto *DCS = dyn_cast<DoCatchStmt>(S)) {
11001133
assert(DoCatchStack.back() == DCS && "Malformed do-catch stack");
11011134
DoCatchStack.pop_back();
@@ -1125,6 +1158,8 @@ struct CoverageMapping : public ASTWalker {
11251158
if (isa<AbstractClosureExpr>(E) && !Parent.isNull())
11261159
return {false, E};
11271160

1161+
// If we're in an 'incomplete' region, update it to include this node. This
1162+
// ensures we only create the region if needed.
11281163
if (!RegionStack.empty())
11291164
extendRegion(E);
11301165

‎lib/SILGen/SILGenPattern.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2814,7 +2814,6 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
28142814
emission.emitAddressOnlyAllocations();
28152815

28162816
SILBasicBlock *contBB = createBasicBlock();
2817-
emitProfilerIncrement(S);
28182817
JumpDest contDest(contBB, Cleanups.getCleanupsDepth(), CleanupLocation(S));
28192818

28202819
LexicalScope switchScope(*this, CleanupLocation(S));

‎test/Profiler/coverage_switch.swift

Lines changed: 121 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,61 @@
11
// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -profile-generate -profile-coverage-mapping -emit-sorted-sil -emit-sil -module-name coverage_switch %s | %FileCheck %s
22
// RUN: %target-swift-frontend -profile-generate -profile-coverage-mapping -emit-ir %s
33

4+
// CHECK-LABEL: sil hidden @$s15coverage_switch2f1yys5Int32VF : $@convention(thin) (Int32) -> ()
5+
// CHECK: string_literal
6+
// CHECK-NEXT: integer_literal $Builtin.Int64, 0
7+
// CHECK-NEXT: integer_literal $Builtin.Int32, 4
8+
// CHECK-NEXT: integer_literal $Builtin.Int32, 0
9+
// CHECK-NEXT: int_instrprof_increment
10+
11+
// CHECK: integer_literal $Builtin.Int32, 1
12+
// CHECK: cmp_eq_Int32
13+
// CHECK: cond_br {{%[0-9]+}}, [[CASE1:bb[0-9]+]], [[NOTCASE1:bb[0-9]+]]
14+
15+
// CHECK: [[NOTCASE1]]
16+
// CHECK: integer_literal $Builtin.Int32, 2
17+
// CHECK: cmp_eq_Int32
18+
// CHECK: cond_br {{%[0-9]+}}, [[CASE2:bb[0-9]+]], [[NOTCASE2:bb[0-9]+]]
19+
20+
// CHECK: [[NOTCASE2]]
21+
// CHECK-NEXT: string_literal
22+
// CHECK-NEXT: integer_literal $Builtin.Int64, 0
23+
// CHECK-NEXT: integer_literal $Builtin.Int32, 4
24+
// CHECK-NEXT: integer_literal $Builtin.Int32, 3
25+
// CHECK-NEXT: int_instrprof_increment
26+
// CHECK-NEXT: br [[DEFAULT:bb[0-9]+]]
27+
28+
// CHECK: [[CASE2]]
29+
// CHECK-NEXT: string_literal
30+
// CHECK-NEXT: integer_literal $Builtin.Int64, 0
31+
// CHECK-NEXT: integer_literal $Builtin.Int32, 4
32+
// CHECK-NEXT: integer_literal $Builtin.Int32, 2
33+
// CHECK-NEXT: int_instrprof_increment
34+
// CHECK-NEXT: br [[DEFAULT]]
35+
36+
// CHECK: [[DEFAULT]]
37+
// CHECK: function_ref @$s15coverage_switch2f1yys5Int32VF : $@convention(thin) (Int32) -> ()
38+
39+
// CHECK: [[CASE1]]
40+
// CHECK-NEXT: string_literal
41+
// CHECK-NEXT: integer_literal $Builtin.Int64, 0
42+
// CHECK-NEXT: integer_literal $Builtin.Int32, 4
43+
// CHECK-NEXT: integer_literal $Builtin.Int32, 1
44+
// CHECK-NEXT: int_instrprof_increment
45+
446
// CHECK-LABEL: sil_coverage_map {{.*}}// coverage_switch.f1
5-
func f1(_ x : Int32) {
6-
switch (x) {
7-
case 1: // CHECK: [[@LINE]]:3 -> [[@LINE+1]]:10 : 2
47+
func f1(_ x : Int32) { // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE+11]]:2 : 0
48+
switch (x) { // CHECK-NEXT: [[@LINE]]:10 -> [[@LINE]]:13 : 0
49+
case 1: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:10 : 1
850
break
9-
case 2: // CHECK: [[@LINE]]:3 -> [[@LINE+1]]:16 : 3
51+
case 2: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:16 : 2
1052
fallthrough
11-
default: // CHECK: [[@LINE]]:3 -> [[@LINE+1]]:14 : (3 + 4)
53+
default: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:14 : (2 + 3)
1254
f1(x - 1)
13-
} // CHECK: [[@LINE]]:4 -> [[@LINE+3]]:2 : 1
55+
} // CHECK-NEXT: [[@LINE]]:4 -> [[@LINE+3]]:2 : ((1 + 2) + 3)
1456

1557
var y = x
16-
}
58+
} // CHECK-NEXT: }
1759

1860
enum Algebraic {
1961
case Type1(Int32, Int32)
@@ -25,41 +67,96 @@ enum Algebraic {
2567
func nop() {}
2668

2769
// CHECK-LABEL: sil_coverage_map {{.*}}// coverage_switch.f2
28-
func f2(_ x : Algebraic) -> Int32 {
29-
switch(x) {
30-
case let .Type1(y, z): // CHECK: [[@LINE]]:3 -> [[@LINE+1]]:10 : 2
70+
func f2(_ x : Algebraic) -> Int32 { // CHECK-NEXT: [[@LINE]]:35 -> [[@LINE+15]]:2 : 0
71+
switch(x) { // CHECK-NEXT: [[@LINE]]:9 -> [[@LINE]]:12 : 0
72+
case let .Type1(y, z): // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:10 : 1
3173
nop()
32-
case .Type2(let b): // CHECK: [[@LINE]]:3 -> [[@LINE+2]]:16 : 3
74+
case .Type2(let b): // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+2]]:16 : 2
3375
nop()
3476
fallthrough
35-
case .Type3: // CHECK: [[@LINE]]:3 -> [[@LINE+3]]:6 : (3 + 4)
36-
if (false) { // CHECK: [[@LINE]]:16 -> [[@LINE+2]]:6 : 5
37-
fallthrough
38-
}
39-
case .Type4: // CHECK: [[@LINE]]:3 -> [[@LINE+1]]:10 : (5 + 6)
77+
case .Type3: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+3]]:6 : (2 + 3)
78+
if (false) { // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE]]:15 : (2 + 3)
79+
fallthrough // CHECK-NEXT: [[@LINE-1]]:16 -> [[@LINE+1]]:6 : 4
80+
} // CHECK-NEXT: [[@LINE]]:6 -> [[@LINE]]:6 : ((2 + 3) - 4)
81+
case .Type4: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:10 : (4 + 5)
4082
break
41-
} // CHECK: [[@LINE]]:4 -> [[@LINE+1]]:11 : 1
83+
} // CHECK-NEXT: [[@LINE]]:4 -> [[@LINE+1]]:11 : (((1 + 2) + 3) + 5)
4284
return 0
43-
}
85+
} // CHECK-NEXT: }
4486

4587
public enum Simple {
4688
case First, Second
4789
}
4890

4991
// CHECK-LABEL: sil_coverage_map {{.*}}// coverage_switch.f3
50-
func f3(_ x : Simple) -> Int32 {
51-
switch (x) {
52-
case .First: // CHECK: [[@LINE]]:3 -> [[@LINE+1]]:13 : 2
92+
func f3(_ x : Simple) -> Int32 { // CHECK-NEXT: [[@LINE]]:32 -> [[@LINE+9]]:2 : 0
93+
switch (x) { // CHECK-NEXT: [[@LINE]]:10 -> [[@LINE]]:13 : 0
94+
case .First: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:13 : 1
5395
return 1
54-
case .Second: // CHECK: [[@LINE]]:3 -> [[@LINE+1]]:10 : 3
96+
case .Second: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:10 : 2
5597
break
56-
} // CHECK: [[@LINE]]:4 -> [[@LINE+2]]:11 : 1
98+
} // CHECK: [[@LINE]]:4 -> [[@LINE+2]]:11 : 2
5799

58100
return 0
59-
}
101+
} // CHECK-NEXT: }
60102

61103
f1(3)
62104
f2(Algebraic.Type1(1, 1))
63105
f2(Algebraic.Type2(false))
64106
f2(Algebraic.Type3)
65107
f3(Simple.Second)
108+
109+
// CHECK-LABEL: sil_coverage_map {{.*}}// coverage_switch.f4
110+
func f4(_ x: Int) throws -> Int { // CHECK-NEXT: [[@LINE]]:33 -> [[@LINE+21]]:2 : 0
111+
y: do { // CHECK-NEXT: [[@LINE]]:9 -> [[@LINE+18]]:4 : 0
112+
switch x { // CHECK-NEXT: [[@LINE]]:12 -> [[@LINE]]:13 : 0
113+
case 1, 2, 3: // CHECK-NEXT: [[@LINE]]:5 -> [[@LINE+4]]:18 : 1
114+
if .random() { // CHECK-NEXT: [[@LINE]]:10 -> [[@LINE]]:19 : 1
115+
return 5 // CHECK-NEXT: [[@LINE-1]]:20 -> [[@LINE+1]]:8 : 2
116+
} // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE+1]]:18 : (1 - 2)
117+
fallthrough
118+
case 4: // CHECK-NEXT: [[@LINE]]:5 -> [[@LINE+4]]:8 : ((1 + 3) - 2)
119+
if .random() { // CHECK-NEXT: [[@LINE]]:10 -> [[@LINE]]:19 : ((1 + 3) - 2)
120+
struct E : Error {} // CHECK-NEXT: [[@LINE-1]]:20 -> [[@LINE+2]]:8 : 4
121+
throw E()
122+
} // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE]]:8 : (((1 + 3) - 2) - 4)
123+
default: // CHECK-NEXT: [[@LINE]]:5 -> [[@LINE+3]]:8 : 5
124+
if .random() { // CHECK-NEXT: [[@LINE]]:10 -> [[@LINE]]:19 : 5
125+
break y // CHECK-NEXT: [[@LINE-1]]:20 -> [[@LINE+1]]:8 : 6
126+
} // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE]]:8 : (5 - 6)
127+
}
128+
f1(0) // CHECK-NEXT: [[@LINE-1]]:6 -> [[@LINE+1]]:4 : (((((1 + 3) + 5) - 2) - 4) - 6)
129+
}
130+
return 1 // CHECK-NEXT: [[@LINE-1]]:4 -> [[@LINE]]:11 : ((((1 + 3) + 5) - 2) - 4)
131+
} // CHECK-NEXT: }
132+
133+
// CHECK-LABEL: sil_coverage_map {{.*}}// coverage_switch.f5
134+
func f5(_ x: Simple) -> Int { // CHECK-NEXT: [[@LINE]]:29 -> [[@LINE+7]]:2 : 0
135+
switch x { // CHECK-NEXT: [[@LINE]]:10 -> [[@LINE]]:11 : 0
136+
case .First: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:13 : 1
137+
return 0
138+
case .Second: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:13 : 2
139+
return 1
140+
}
141+
} // CHECK-NEXT: }
142+
143+
// CHECK-LABEL: sil_coverage_map {{.*}}// coverage_switch.f6
144+
func f6(_ x: Simple) -> Int { // CHECK-NEXT: [[@LINE]]:29 -> [[@LINE+5]]:2 : 0
145+
switch x { // CHECK-NEXT: [[@LINE]]:10 -> [[@LINE]]:11 : 0
146+
case .First: return 0 // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE]]:25 : 1
147+
case .Second: return 1 // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE]]:25 : 2
148+
}
149+
} // CHECK-NEXT: }
150+
151+
// CHECK-LABEL: sil_coverage_map {{.*}}// coverage_switch.f7
152+
func f7() -> Int { // CHECK-NEXT: [[@LINE]]:18 -> [[@LINE+10]]:2 : 0
153+
switch .random() ? Simple.First : .Second {
154+
// CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE-1]]:44 : 0
155+
// CHECK-NEXT: [[@LINE-2]]:22 -> [[@LINE-2]]:34 : 1
156+
// CHECK-NEXT: [[@LINE-3]]:37 -> [[@LINE-3]]:44 : (0 - 1)
157+
case .First: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:13 : 2
158+
return 0
159+
case .Second: // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE+1]]:13 : 3
160+
return 1
161+
}
162+
} // CHECK-NEXT: }

0 commit comments

Comments
 (0)