Skip to content

Commit 9508fdc

Browse files
gregomnigregomni
committed
Simpler implementation for multiple matching patterns in case with variable bindings
Backed off of the implementation with a shared body block, which got messy really fast with multiple patterns containing address-only values to pass in, as it led to separated stack alloc/cleanup requirements and passing stack alloc’d values as block args, aliasing of addresses for stack cleanup verifying, etc. A mess. This now does the conservative thing of repeating the body in each matching pattern block.
1 parent 42d3b18 commit 9508fdc

File tree

2 files changed

+62
-100
lines changed

2 files changed

+62
-100
lines changed

‎lib/SILGen/SILGenPattern.cpp

Lines changed: 25 additions & 69 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-
// Emit the guard branch, if it exists.
1021-
if (guardExpr) {
1022-
SGF.usingImplicitVariablesForPattern(clauses[row].getCasePattern(), dyn_cast<CaseStmt>(stmt), [&]{
1020+
SGF.usingImplicitVariablesForPattern(clauses[row].getCasePattern(), dyn_cast<CaseStmt>(stmt), [&]{
1021+
// Emit the guard branch, if it exists.
1022+
if (guardExpr) {
10231023
this->emitGuardBranch(guardExpr, guardExpr, failure);
1024-
});
1025-
}
1024+
}
10261025

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

@@ -1168,13 +1168,6 @@ 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-
11781171
// Initialize the variable value.
11791172
InitializationPtr init = SGF.emitInitializationForVarDecl(var);
11801173

@@ -1184,9 +1177,6 @@ void PatternMatchEmission::bindVariable(SILLocation loc, VarDecl *var,
11841177
} else {
11851178
std::move(rv).copyInto(SGF, loc, init.get());
11861179
}
1187-
1188-
if (forcedLet)
1189-
var->setLet(false);
11901180
}
11911181

11921182
/// Evaluate a guard expression and, if it returns false, branch to
@@ -2099,35 +2089,36 @@ void SILGenFunction::usingImplicitVariablesForPattern(Pattern *pattern, CaseStmt
20992089

21002090
ArrayRef<CaseLabelItem> labelItems = stmt->getCaseLabelItems();
21012091
auto expectedPattern = labelItems[0].getPattern();
2102-
2092+
21032093
if (labelItems.size() <= 1 || pattern == expectedPattern) {
21042094
f();
21052095
return;
21062096
}
21072097

21082098
// Remap vardecls that the case body is expecting to the pattern var locations
21092099
// for the given pattern, emit whatever, and switch back.
2110-
SmallVector<VarDecl *, 4> Vars;
2111-
expectedPattern->collectVariables(Vars);
2100+
SmallVector<VarDecl *, 4> expectedVars;
2101+
SmallVector<VarLoc, 4> savedVarLocs;
2102+
expectedPattern->collectVariables(expectedVars);
2103+
2104+
for (auto expected : expectedVars)
2105+
savedVarLocs.push_back(VarLocs[expected]);
21122106

2113-
auto variableSwapper = [&] {
2114-
pattern->forEachVariable([&](VarDecl *VD) {
2115-
if (!VD->hasName())
2107+
pattern->forEachVariable([&](VarDecl *VD) {
2108+
if (!VD->hasName())
2109+
return;
2110+
for (auto expected : expectedVars) {
2111+
if (expected->hasName() && expected->getName() == VD->getName()) {
2112+
VarLocs[expected] = VarLocs[VD];
21162113
return;
2117-
for (auto expected : Vars) {
2118-
if (expected->hasName() && expected->getName() == VD->getName()) {
2119-
auto swap = VarLocs[expected];
2120-
VarLocs[expected] = VarLocs[VD];
2121-
VarLocs[VD] = swap;
2122-
return;
2123-
}
21242114
}
2125-
});
2126-
};
2115+
}
2116+
});
21272117

2128-
variableSwapper();
21292118
f();
2130-
variableSwapper();
2119+
2120+
for (unsigned index = 0; index < savedVarLocs.size(); index++)
2121+
VarLocs[expectedVars[index]] = savedVarLocs[index];
21312122
}
21322123

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

‎test/SILGen/switch_var.swift

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -471,15 +471,14 @@ func test_multiple_patterns1() {
471471
// CHECK: cond_br {{%.*}}, [[FIRST_MATCH_CASE:bb[0-9]+]], [[FIRST_FAIL:bb[0-9]+]]
472472
// CHECK: [[FIRST_MATCH_CASE]]:
473473
// CHECK: debug_value [[FIRST_X:%.*]] :
474-
// CHECK: br [[CASE_BODY:bb[0-9]+]]([[FIRST_X]] : $Int)
474+
// CHECK: [[B1_FUNC:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
475+
// CHECK: apply [[B1_FUNC]]([[FIRST_X]])
475476
// CHECK: [[FIRST_FAIL]]:
476477
// CHECK: cond_br {{%.*}}, [[SECOND_MATCH_CASE:bb[0-9]+]], [[SECOND_FAIL:bb[0-9]+]]
477478
// CHECK: [[SECOND_MATCH_CASE]]:
478479
// CHECK: debug_value [[SECOND_X:%.*]] :
479-
// CHECK: br [[CASE_BODY]]([[SECOND_X]] : $Int)
480-
// CHECK: [[CASE_BODY]]([[BODY_VAR:%.*]] : $Int):
481-
// CHECK: [[A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
482-
// CHECK: apply [[A]]([[BODY_VAR]])
480+
// CHECK: [[B2_FUNC:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
481+
// CHECK: apply [[B2_FUNC]]([[SECOND_X]])
483482
a(x: x)
484483
default:
485484
// CHECK: [[SECOND_FAIL]]:
@@ -502,16 +501,15 @@ func test_multiple_patterns2() {
502501
// CHECK: apply {{%.*}}([[FIRST_X]], [[T1]])
503502
// CHECK: cond_br {{%.*}}, [[FIRST_MATCH_CASE:bb[0-9]+]], [[FIRST_FAIL:bb[0-9]+]]
504503
// CHECK: [[FIRST_MATCH_CASE]]:
505-
// CHECK: br [[CASE_BODY:bb[0-9]+]]([[FIRST_X]] : $Int)
504+
// CHECK: [[A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
505+
// CHECK: apply [[A]]([[FIRST_X]])
506506
// CHECK: [[FIRST_FAIL]]:
507507
// CHECK: debug_value [[SECOND_X:%.*]] :
508508
// CHECK: apply {{%.*}}([[SECOND_X]], [[T2]])
509509
// CHECK: cond_br {{%.*}}, [[SECOND_MATCH_CASE:bb[0-9]+]], [[SECOND_FAIL:bb[0-9]+]]
510510
// CHECK: [[SECOND_MATCH_CASE]]:
511-
// CHECK: br [[CASE_BODY]]([[SECOND_X]] : $Int)
512-
// CHECK: [[CASE_BODY]]([[BODY_VAR:%.*]] : $Int):
513511
// CHECK: [[A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
514-
// CHECK: apply [[A]]([[BODY_VAR]])
512+
// CHECK: apply [[A]]([[SECOND_X]])
515513
a(x: x)
516514
default:
517515
// CHECK: [[SECOND_FAIL]]:
@@ -535,22 +533,22 @@ func test_multiple_patterns3() {
535533
// CHECK: [[A]]({{%.*}} : $(Int, Double)):
536534
// CHECK: [[A_X:%.*]] = tuple_extract
537535
// CHECK: [[A_N:%.*]] = tuple_extract
538-
// CHECK: br [[CASE_BODY:bb[0-9]+]]([[A_X]] : $Int, [[A_N]] : $Double)
536+
// CHECK: [[FUNC_A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
537+
// CHECK: apply [[FUNC_A]]([[A_X]])
539538

540539
// CHECK: [[B]]({{%.*}} : $(Double, Int)):
541540
// CHECK: [[B_N:%.*]] = tuple_extract
542541
// CHECK: [[B_X:%.*]] = tuple_extract
543-
// CHECK: br [[CASE_BODY]]([[B_X]] : $Int, [[B_N]] : $Double)
542+
// CHECK: [[FUNC_B:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
543+
// CHECK: apply [[FUNC_B]]([[B_X]])
544544

545545
// CHECK: [[C]]({{%.*}} : $(Int, Int, Double)):
546546
// CHECK: [[C__:%.*]] = tuple_extract
547547
// CHECK: [[C_X:%.*]] = tuple_extract
548548
// CHECK: [[C_N:%.*]] = tuple_extract
549-
// CHECK: br [[CASE_BODY]]([[C_X]] : $Int, [[C_N]] : $Double)
549+
// CHECK: [[FUNC_C:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
550+
// CHECK: apply [[FUNC_C]]([[C_X]])
550551

551-
// CHECK: [[CASE_BODY]]([[BODY_X:%.*]] : $Int, [[BODY_N:%.*]] : $Double):
552-
// CHECK: [[FUNC_A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
553-
// CHECK: apply [[FUNC_A]]([[BODY_X]])
554552
a(x: x)
555553
}
556554
}
@@ -574,24 +572,25 @@ func test_multiple_patterns4() {
574572
// CHECK: [[A]]({{%.*}} : $(Int, Double)):
575573
// CHECK: [[A_X:%.*]] = tuple_extract
576574
// CHECK: [[A_N:%.*]] = tuple_extract
577-
// CHECK: br [[CASE_BODY:bb[0-9]+]]([[A_X]] : $Int)
575+
// CHECK: [[FUNC_A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
576+
// CHECK: apply [[FUNC_A]]([[A_X]])
578577

579578
// CHECK: [[B]]({{%.*}} : $(Double, Int)):
580579
// CHECK: [[B_N:%.*]] = tuple_extract
581580
// CHECK: [[B_X:%.*]] = tuple_extract
582-
// CHECK: br [[CASE_BODY]]([[B_X]] : $Int)
581+
// CHECK: [[FUNC_B:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
582+
// CHECK: apply [[FUNC_B]]([[B_X]])
583583

584584
// CHECK: [[C]]({{%.*}} : $(Int, Int, Double)):
585-
// CHECK: br [[CASE_BODY]]([[Y_X]] : $Int)
585+
// CHECK: [[FUNC_C:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
586+
// CHECK: apply [[FUNC_C]]([[Y_X]])
586587

587588
// CHECK: [[Z]]({{%.*}} : $(Int, Foo)):
588589
// CHECK: [[Z_X:%.*]] = tuple_extract
589590
// CHECK: [[Z_F:%.*]] = tuple_extract
590-
// CHECK: br [[CASE_BODY]]([[Z_X]] : $Int)
591+
// CHECK: [[FUNC_Z:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
592+
// CHECK: apply [[FUNC_Z]]([[Z_X]])
591593

592-
// CHECK: [[CASE_BODY]]([[BODY_X:%.*]] : $Int):
593-
// CHECK: [[FUNC_A:%.*]] = function_ref @_TF10switch_var1aFT1xSi_T_
594-
// CHECK: apply [[FUNC_A]]([[BODY_X]])
595594
a(x: x)
596595
}
597596
}
@@ -612,25 +611,32 @@ func test_multiple_patterns5() {
612611
// CHECK: [[A]]({{%.*}} : $(Int, Double)):
613612
// CHECK: [[A_X:%.*]] = tuple_extract
614613
// CHECK: [[A_N:%.*]] = tuple_extract
615-
// CHECK: br [[CASE_BODY:bb[0-9]+]]([[A_X]] : $Int)
614+
// CHECK: [[A_BOX:%.*]] = project_box
615+
// CHECK: store [[A_X]] to [[A_BOX]]
616+
// CHECK: [[FUNC_A:%.*]] = function_ref @_TF10switch_var3aaaFT1xRSi_T_
617+
// CHECK: apply [[FUNC_A]]([[A_BOX]])
616618

617619
// CHECK: [[B]]({{%.*}} : $(Double, Int)):
618620
// CHECK: [[B_N:%.*]] = tuple_extract
619621
// CHECK: [[B_X:%.*]] = tuple_extract
620-
// CHECK: br [[CASE_BODY]]([[B_X]] : $Int)
622+
// CHECK: [[B_BOX:%.*]] = project_box
623+
// CHECK: store [[B_X]] to [[B_BOX]]
624+
// CHECK: [[FUNC_B:%.*]] = function_ref @_TF10switch_var3aaaFT1xRSi_T_
625+
// CHECK: apply [[FUNC_B]]([[B_BOX]])
621626

622627
// CHECK: [[C]]({{%.*}} : $(Int, Int, Double)):
623-
// CHECK: br [[CASE_BODY]]([[Y_X]] : $Int)
628+
// CHECK: [[Y_BOX:%.*]] = project_box
629+
// CHECK: store [[Y_X]] to [[Y_BOX]]
630+
// CHECK: [[FUNC_C:%.*]] = function_ref @_TF10switch_var3aaaFT1xRSi_T_
631+
// CHECK: apply [[FUNC_C]]([[Y_BOX]])
624632

625633
// CHECK: [[Z]]({{%.*}} : $(Int, Foo)):
626634
// CHECK: [[Z_X:%.*]] = tuple_extract
627635
// CHECK: [[Z_F:%.*]] = tuple_extract
628-
// CHECK: br [[CASE_BODY]]([[Z_X]] : $Int)
629-
630-
// CHECK: [[CASE_BODY]]([[BODY_X:%.*]] : $Int):
631-
// CHECK: store [[BODY_X]] to [[BOX_X:%.*]] : $*Int
632-
// CHECK: [[FUNC_AAA:%.*]] = function_ref @_TF10switch_var3aaaFT1xRSi_T_
633-
// CHECK: apply [[FUNC_AAA]]([[BOX_X]])
636+
// CHECK: [[Z_BOX:%.*]] = project_box
637+
// CHECK: store [[Z_X]] to [[Z_BOX]]
638+
// CHECK: [[FUNC_Z:%.*]] = function_ref @_TF10switch_var3aaaFT1xRSi_T_
639+
// CHECK: apply [[FUNC_Z]]([[Z_BOX]])
634640
aaa(x: &x)
635641
}
636642
}

0 commit comments

Comments
 (0)