Skip to content

Commit 4cd3aad

Browse files
jckarterjckarter
authored
Merge pull request #5825 from jckarter/revert-case-body-reemission
Revert "Simpler implementation for multiple matching patterns in case with variable bindings"
2 parents 064426a + 9348059 commit 4cd3aad

File tree

2 files changed

+107
-62
lines changed

2 files changed

+107
-62
lines changed

‎lib/SILGen/SILGenPattern.cpp

Lines changed: 69 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,15 +1017,15 @@ void PatternMatchEmission::emitWildcardDispatch(ClauseMatrix &clauses,
10171017
// Bind the rest of the patterns.
10181018
bindIrrefutablePatterns(clauses[row], args, !hasGuard, hasMultipleItems);
10191019

1020-
SGF.usingImplicitVariablesForPattern(clauses[row].getCasePattern(), dyn_cast<CaseStmt>(stmt), [&]{
1021-
// Emit the guard branch, if it exists.
1022-
if (guardExpr) {
1020+
// Emit the guard branch, if it exists.
1021+
if (guardExpr) {
1022+
SGF.usingImplicitVariablesForPattern(clauses[row].getCasePattern(), dyn_cast<CaseStmt>(stmt), [&]{
10231023
this->emitGuardBranch(guardExpr, guardExpr, failure);
1024-
}
1024+
});
1025+
}
10251026

1026-
// Enter the row.
1027-
CompletionHandler(*this, args, clauses[row]);
1028-
});
1027+
// Enter the row.
1028+
CompletionHandler(*this, args, clauses[row]);
10291029
assert(!SGF.B.hasValidInsertionPoint());
10301030
}
10311031

@@ -1168,6 +1168,13 @@ void PatternMatchEmission::bindVariable(SILLocation loc, VarDecl *var,
11681168
CanType formalValueType,
11691169
bool isIrrefutable,
11701170
bool hasMultipleItems) {
1171+
// If this binding is one of multiple patterns, each individual binding
1172+
// will just be let, and then the chosen value will get forwarded into
1173+
// a var box in the final shared case block.
1174+
bool forcedLet = hasMultipleItems && !var->isLet();
1175+
if (forcedLet)
1176+
var->setLet(true);
1177+
11711178
// Initialize the variable value.
11721179
InitializationPtr init = SGF.emitInitializationForVarDecl(var);
11731180

@@ -1177,6 +1184,9 @@ void PatternMatchEmission::bindVariable(SILLocation loc, VarDecl *var,
11771184
} else {
11781185
std::move(rv).copyInto(SGF, loc, init.get());
11791186
}
1187+
1188+
if (forcedLet)
1189+
var->setLet(false);
11801190
}
11811191

11821192
/// Evaluate a guard expression and, if it returns false, branch to
@@ -2092,36 +2102,35 @@ void SILGenFunction::usingImplicitVariablesForPattern(Pattern *pattern, CaseStmt
20922102

20932103
ArrayRef<CaseLabelItem> labelItems = stmt->getCaseLabelItems();
20942104
auto expectedPattern = labelItems[0].getPattern();
2095-
2105+
20962106
if (labelItems.size() <= 1 || pattern == expectedPattern) {
20972107
f();
20982108
return;
20992109
}
21002110

21012111
// Remap vardecls that the case body is expecting to the pattern var locations
21022112
// for the given pattern, emit whatever, and switch back.
2103-
SmallVector<VarDecl *, 4> expectedVars;
2104-
SmallVector<VarLoc, 4> savedVarLocs;
2105-
expectedPattern->collectVariables(expectedVars);
2106-
2107-
for (auto expected : expectedVars)
2108-
savedVarLocs.push_back(VarLocs[expected]);
2113+
SmallVector<VarDecl *, 4> Vars;
2114+
expectedPattern->collectVariables(Vars);
21092115

2110-
pattern->forEachVariable([&](VarDecl *VD) {
2111-
if (!VD->hasName())
2112-
return;
2113-
for (auto expected : expectedVars) {
2114-
if (expected->hasName() && expected->getName() == VD->getName()) {
2115-
VarLocs[expected] = VarLocs[VD];
2116+
auto variableSwapper = [&] {
2117+
pattern->forEachVariable([&](VarDecl *VD) {
2118+
if (!VD->hasName())
21162119
return;
2120+
for (auto expected : Vars) {
2121+
if (expected->hasName() && expected->getName() == VD->getName()) {
2122+
auto swap = VarLocs[expected];
2123+
VarLocs[expected] = VarLocs[VD];
2124+
VarLocs[VD] = swap;
2125+
return;
2126+
}
21172127
}
2118-
}
2119-
});
2128+
});
2129+
};
21202130

2131+
variableSwapper();
21212132
f();
2122-
2123-
for (unsigned index = 0; index < savedVarLocs.size(); index++)
2124-
VarLocs[expectedVars[index]] = savedVarLocs[index];
2133+
variableSwapper();
21252134
}
21262135

21272136
void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
@@ -2148,6 +2157,41 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
21482157
JumpDest sharedDest = emission.getSharedCaseBlockDest(caseBlock,
21492158
row.hasFallthroughTo());
21502159
Cleanups.emitBranchAndCleanups(sharedDest, caseBlock);
2160+
} else if (caseBlock->getCaseLabelItems().size() > 1) {
2161+
JumpDest sharedDest = emission.getSharedCaseBlockDest(caseBlock,
2162+
row.hasFallthroughTo());
2163+
2164+
// Generate the arguments from this row's pattern in the case block's expected order,
2165+
// and keep those arguments from being cleaned up, as we're passing the +1 along to
2166+
// the shared case block dest. (The cleanups still happen, as they are threaded through
2167+
// here messily, but the explicit retains here counteract them, and then the
2168+
// retain/release pair gets optimized out.)
2169+
ArrayRef<CaseLabelItem> labelItems = caseBlock->getCaseLabelItems();
2170+
SmallVector<SILValue, 4> args;
2171+
SmallVector<VarDecl *, 4> expectedVarOrder;
2172+
SmallVector<VarDecl *, 4> Vars;
2173+
labelItems[0].getPattern()->collectVariables(expectedVarOrder);
2174+
row.getCasePattern()->collectVariables(Vars);
2175+
2176+
for (auto expected : expectedVarOrder) {
2177+
if (!expected->hasName())
2178+
continue;
2179+
for (auto var : Vars) {
2180+
if (var->hasName() && var->getName() == expected->getName()) {
2181+
auto value = VarLocs[var].value;
2182+
2183+
for (auto cmv : argArray) {
2184+
if (cmv.getValue() == value) {
2185+
value = B.emitCopyValueOperation(CurrentSILLoc, value);
2186+
break;
2187+
}
2188+
}
2189+
args.push_back(value);
2190+
break;
2191+
}
2192+
}
2193+
}
2194+
Cleanups.emitBranchAndCleanups(sharedDest, caseBlock, args);
21512195
} else {
21522196
// However, if we don't have a fallthrough or a multi-pattern 'case', we
21532197
// can just emit the body inline and save some dead blocks.

‎test/SILGen/switch_var.swift

Lines changed: 38 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -529,14 +529,15 @@ func test_multiple_patterns1() {
529529
// CHECK: cond_br {{%.*}}, [[FIRST_MATCH_CASE:bb[0-9]+]], [[FIRST_FAIL:bb[0-9]+]]
530530
// CHECK: [[FIRST_MATCH_CASE]]:
531531
// CHECK: debug_value [[FIRST_X:%.*]] :
532-
// CHECK: [[B1_FUNC:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
533-
// CHECK: apply [[B1_FUNC]]([[FIRST_X]])
532+
// CHECK: br [[CASE_BODY:bb[0-9]+]]([[FIRST_X]] : $Int)
534533
// CHECK: [[FIRST_FAIL]]:
535534
// CHECK: cond_br {{%.*}}, [[SECOND_MATCH_CASE:bb[0-9]+]], [[SECOND_FAIL:bb[0-9]+]]
536535
// CHECK: [[SECOND_MATCH_CASE]]:
537536
// CHECK: debug_value [[SECOND_X:%.*]] :
538-
// CHECK: [[B2_FUNC:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
539-
// CHECK: apply [[B2_FUNC]]([[SECOND_X]])
537+
// CHECK: br [[CASE_BODY]]([[SECOND_X]] : $Int)
538+
// CHECK: [[CASE_BODY]]([[BODY_VAR:%.*]] : $Int):
539+
// CHECK: [[A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
540+
// CHECK: apply [[A]]([[BODY_VAR]])
540541
a(x: x)
541542
default:
542543
// CHECK: [[SECOND_FAIL]]:
@@ -559,15 +560,16 @@ func test_multiple_patterns2() {
559560
// CHECK: apply {{%.*}}([[FIRST_X]], [[T1]])
560561
// CHECK: cond_br {{%.*}}, [[FIRST_MATCH_CASE:bb[0-9]+]], [[FIRST_FAIL:bb[0-9]+]]
561562
// CHECK: [[FIRST_MATCH_CASE]]:
562-
// CHECK: [[A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
563-
// CHECK: apply [[A]]([[FIRST_X]])
563+
// CHECK: br [[CASE_BODY:bb[0-9]+]]([[FIRST_X]] : $Int)
564564
// CHECK: [[FIRST_FAIL]]:
565565
// CHECK: debug_value [[SECOND_X:%.*]] :
566566
// CHECK: apply {{%.*}}([[SECOND_X]], [[T2]])
567567
// CHECK: cond_br {{%.*}}, [[SECOND_MATCH_CASE:bb[0-9]+]], [[SECOND_FAIL:bb[0-9]+]]
568568
// CHECK: [[SECOND_MATCH_CASE]]:
569+
// CHECK: br [[CASE_BODY]]([[SECOND_X]] : $Int)
570+
// CHECK: [[CASE_BODY]]([[BODY_VAR:%.*]] : $Int):
569571
// CHECK: [[A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
570-
// CHECK: apply [[A]]([[SECOND_X]])
572+
// CHECK: apply [[A]]([[BODY_VAR]])
571573
a(x: x)
572574
default:
573575
// CHECK: [[SECOND_FAIL]]:
@@ -591,22 +593,22 @@ func test_multiple_patterns3() {
591593
// CHECK: [[A]]({{%.*}} : $(Int, Double)):
592594
// CHECK: [[A_X:%.*]] = tuple_extract
593595
// CHECK: [[A_N:%.*]] = tuple_extract
594-
// CHECK: [[FUNC_A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
595-
// CHECK: apply [[FUNC_A]]([[A_X]])
596+
// CHECK: br [[CASE_BODY:bb[0-9]+]]([[A_X]] : $Int, [[A_N]] : $Double)
596597

597598
// CHECK: [[B]]({{%.*}} : $(Double, Int)):
598599
// CHECK: [[B_N:%.*]] = tuple_extract
599600
// CHECK: [[B_X:%.*]] = tuple_extract
600-
// CHECK: [[FUNC_B:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
601-
// CHECK: apply [[FUNC_B]]([[B_X]])
601+
// CHECK: br [[CASE_BODY]]([[B_X]] : $Int, [[B_N]] : $Double)
602602

603603
// CHECK: [[C]]({{%.*}} : $(Int, Int, Double)):
604604
// CHECK: [[C__:%.*]] = tuple_extract
605605
// CHECK: [[C_X:%.*]] = tuple_extract
606606
// CHECK: [[C_N:%.*]] = tuple_extract
607-
// CHECK: [[FUNC_C:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
608-
// CHECK: apply [[FUNC_C]]([[C_X]])
607+
// CHECK: br [[CASE_BODY]]([[C_X]] : $Int, [[C_N]] : $Double)
609608

609+
// CHECK: [[CASE_BODY]]([[BODY_X:%.*]] : $Int, [[BODY_N:%.*]] : $Double):
610+
// CHECK: [[FUNC_A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
611+
// CHECK: apply [[FUNC_A]]([[BODY_X]])
610612
a(x: x)
611613
}
612614
}
@@ -630,25 +632,24 @@ func test_multiple_patterns4() {
630632
// CHECK: [[A]]({{%.*}} : $(Int, Double)):
631633
// CHECK: [[A_X:%.*]] = tuple_extract
632634
// CHECK: [[A_N:%.*]] = tuple_extract
633-
// CHECK: [[FUNC_A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
634-
// CHECK: apply [[FUNC_A]]([[A_X]])
635+
// CHECK: br [[CASE_BODY:bb[0-9]+]]([[A_X]] : $Int)
635636

636637
// CHECK: [[B]]({{%.*}} : $(Double, Int)):
637638
// CHECK: [[B_N:%.*]] = tuple_extract
638639
// CHECK: [[B_X:%.*]] = tuple_extract
639-
// CHECK: [[FUNC_B:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
640-
// CHECK: apply [[FUNC_B]]([[B_X]])
640+
// CHECK: br [[CASE_BODY]]([[B_X]] : $Int)
641641

642642
// CHECK: [[C]]({{%.*}} : $(Int, Int, Double)):
643-
// CHECK: [[FUNC_C:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
644-
// CHECK: apply [[FUNC_C]]([[Y_X]])
643+
// CHECK: br [[CASE_BODY]]([[Y_X]] : $Int)
645644

646645
// CHECK: [[Z]]({{%.*}} : $(Int, Foo)):
647646
// CHECK: [[Z_X:%.*]] = tuple_extract
648647
// CHECK: [[Z_F:%.*]] = tuple_extract
649-
// CHECK: [[FUNC_Z:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
650-
// CHECK: apply [[FUNC_Z]]([[Z_X]])
648+
// CHECK: br [[CASE_BODY]]([[Z_X]] : $Int)
651649

650+
// CHECK: [[CASE_BODY]]([[BODY_X:%.*]] : $Int):
651+
// CHECK: [[FUNC_A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
652+
// CHECK: apply [[FUNC_A]]([[BODY_X]])
652653
a(x: x)
653654
}
654655
}
@@ -669,33 +670,33 @@ func test_multiple_patterns5() {
669670
// CHECK: [[A]]({{%.*}} : $(Int, Double)):
670671
// CHECK: [[A_X:%.*]] = tuple_extract
671672
// CHECK: [[A_N:%.*]] = tuple_extract
672-
// CHECK: [[A_BOX:%.*]] = project_box
673-
// CHECK: store [[A_X]] to [trivial] [[A_BOX]]
674-
// CHECK: [[FUNC_A:%.*]] = function_ref @_TF10switch_var3aaaFT1xRSi_T_
675-
// CHECK: apply [[FUNC_A]]([[A_BOX]])
673+
// CHECK: br [[CASE_BODY:bb[0-9]+]]([[A_X]] : $Int)
676674

677675
// CHECK: [[B]]({{%.*}} : $(Double, Int)):
678676
// CHECK: [[B_N:%.*]] = tuple_extract
679677
// CHECK: [[B_X:%.*]] = tuple_extract
680-
// CHECK: [[B_BOX:%.*]] = project_box
681-
// CHECK: store [[B_X]] to [trivial] [[B_BOX]]
682-
// CHECK: [[FUNC_B:%.*]] = function_ref @_TF10switch_var3aaaFT1xRSi_T_
683-
// CHECK: apply [[FUNC_B]]([[B_BOX]])
678+
// CHECK: br [[CASE_BODY]]([[B_X]] : $Int)
684679

685680
// CHECK: [[C]]({{%.*}} : $(Int, Int, Double)):
686-
// CHECK: [[Y_BOX:%.*]] = project_box
687-
// CHECK: store [[Y_X]] to [trivial] [[Y_BOX]]
688-
// CHECK: [[FUNC_C:%.*]] = function_ref @_TF10switch_var3aaaFT1xRSi_T_
689-
// CHECK: apply [[FUNC_C]]([[Y_BOX]])
681+
// CHECK: br [[CASE_BODY]]([[Y_X]] : $Int)
690682

691683
// CHECK: [[Z]]({{%.*}} : $(Int, Foo)):
692684
// CHECK: [[Z_X:%.*]] = tuple_extract
693685
// CHECK: [[Z_F:%.*]] = tuple_extract
694-
// CHECK: [[Z_BOX:%.*]] = project_box
695-
// CHECK: store [[Z_X]] to [trivial] [[Z_BOX]]
696-
// CHECK: [[FUNC_Z:%.*]] = function_ref @_TF10switch_var3aaaFT1xRSi_T_
697-
// CHECK: apply [[FUNC_Z]]([[Z_BOX]])
686+
// CHECK: br [[CASE_BODY]]([[Z_X]] : $Int)
687+
688+
// CHECK: [[CASE_BODY]]([[BODY_X:%.*]] : $Int):
689+
// CHECK: store [[BODY_X]] to [trivial] [[BOX_X:%.*]] : $*Int
690+
// CHECK: [[FUNC_AAA:%.*]] = function_ref @_TF10switch_var3aaaFT1xRSi_T_
691+
// CHECK: apply [[FUNC_AAA]]([[BOX_X]])
698692
aaa(x: &x)
699693
}
700694
}
701695

696+
// rdar://problem/29252758 -- local decls must not be reemitted.
697+
func test_against_reemission(x: Bar) {
698+
switch x {
699+
case .Y(let a, _), .Z(_, let a):
700+
let b = a
701+
}
702+
}

0 commit comments

Comments
 (0)