Skip to content

Commit 87604fe

Browse files
authored
Merge pull request #23161 from gottesmm/pr-a5f2826a8beda8910e2b0dc7efba52dce1379f8d
[silgenpattern] Only emit a shared case if we have a fallthrough/mult…
2 parents 404cf73 + 9f0375e commit 87604fe

File tree

4 files changed

+57
-46
lines changed

4 files changed

+57
-46
lines changed

lib/SILGen/SILGenPattern.cpp

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2318,8 +2318,9 @@ void PatternMatchEmission::initSharedCaseBlockDest(CaseStmt *caseBlock,
23182318

23192319
auto *block = SGF.createBasicBlock();
23202320
result.first->second.first = block;
2321-
2322-
// Add args for any pattern variables
2321+
2322+
// If we do not have any bound decls, we do not need to setup any phi
2323+
// arguments for the shared case block. Just bail early.
23232324
if (!caseBlock->hasBoundDecls()) {
23242325
return;
23252326
}
@@ -2355,6 +2356,7 @@ void PatternMatchEmission::emitAddressOnlyAllocations() {
23552356
for (auto &entry : SharedCases) {
23562357
CaseStmt *caseBlock = entry.first;
23572358

2359+
// If we do not have any bound decls, we can bail early.
23582360
if (!caseBlock->hasBoundDecls()) {
23592361
continue;
23602362
}
@@ -2429,11 +2431,9 @@ void PatternMatchEmission::emitSharedCaseBlocks() {
24292431
SGF.B.setInsertionPoint(predBB);
24302432

24312433
} else {
2432-
// FIXME: Figure out why this is necessary.
2433-
if (caseBB->pred_empty()) {
2434-
SGF.eraseBasicBlock(caseBB);
2435-
continue;
2436-
}
2434+
// If we did not need a shared case block, we shouldn't have emitted one.
2435+
assert(!caseBB->pred_empty() &&
2436+
"Shared case block without predecessors?!");
24372437

24382438
// Otherwise, move the block to after the first predecessor.
24392439
auto predBB = *caseBB->pred_begin();
@@ -2651,32 +2651,36 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF,
26512651
auto caseBlock = row.getClientData<CaseStmt>();
26522652
SGF.emitProfilerIncrement(caseBlock);
26532653

2654-
// Certain case statements can be entered along multiple paths, either
2655-
// because they have multiple labels or because of fallthrough. When we
2656-
// need multiple entrance path, we factor the paths with a shared block.
2657-
if (!caseBlock->hasBoundDecls()) {
2658-
// Don't emit anything yet, we emit it at the cleanup level of the switch
2659-
// statement.
2660-
JumpDest sharedDest = emission.getSharedCaseBlockDest(caseBlock);
2661-
SGF.Cleanups.emitBranchAndCleanups(sharedDest, caseBlock);
2662-
return;
2663-
}
2664-
2665-
// If we don't have a fallthrough or a multi-pattern 'case', we can just
2666-
// emit the body inline and save some dead blocks. Emit the statement here.
2654+
// Certain case statements can be entered along multiple paths, either because
2655+
// they have multiple labels or because of fallthrough. When we need multiple
2656+
// entrance path, we factor the paths with a shared block. If we don't have a
2657+
// fallthrough or a multi-pattern 'case', we can just emit the body inline and
2658+
// save some dead blocks. Emit the statement here and bail early.
26672659
if (!row.hasFallthroughTo() && caseBlock->getCaseLabelItems().size() == 1) {
26682660
emission.emitCaseBody(caseBlock);
26692661
return;
26702662
}
26712663

2664+
// Ok, at this point we know that we have a multiple entrance block. Grab our
2665+
// shared destination in preperation for branching to it.
2666+
//
2667+
// NOTE: We do not emit anything yet, since we will emit the shared block
2668+
// later.
26722669
JumpDest sharedDest = emission.getSharedCaseBlockDest(caseBlock);
26732670

2674-
// Generate the arguments from this row's pattern in the case block's
2675-
// expected order, and keep those arguments from being cleaned up, as
2676-
// we're passing the +1 along to the shared case block dest. (The
2677-
// cleanups still happen, as they are threaded through here messily,
2678-
// but the explicit retains here counteract them, and then the
2679-
// retain/release pair gets optimized out.)
2671+
// If we do not have any bound decls, we do not need to setup any
2672+
// variables. Just jump to the shared destination.
2673+
if (!caseBlock->hasBoundDecls()) {
2674+
JumpDest sharedDest = emission.getSharedCaseBlockDest(caseBlock);
2675+
SGF.Cleanups.emitBranchAndCleanups(sharedDest, caseBlock);
2676+
return;
2677+
}
2678+
2679+
// Generate the arguments from this row's pattern in the case block's expected
2680+
// order, and keep those arguments from being cleaned up, as we're passing the
2681+
// +1 along to the shared case block dest. (The cleanups still happen, as they
2682+
// are threaded through here messily, but the explicit retains here counteract
2683+
// them, and then the retain/release pair gets optimized out.)
26802684
ArrayRef<CaseLabelItem> labelItems = caseBlock->getCaseLabelItems();
26812685
SmallVector<SILValue, 4> args;
26822686
SmallVector<VarDecl *, 4> expectedVarOrder;
@@ -2746,9 +2750,10 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
27462750
clauseRows.reserve(S->getRawCases().size());
27472751
bool hasFallthrough = false;
27482752
for (auto caseBlock : S->getCases()) {
2749-
if (!caseBlock->hasBoundDecls() ||
2750-
caseBlock->getCaseLabelItems().size() > 1 ||
2751-
hasFallthrough) {
2753+
// If the previous block falls through into this block or we have multiple
2754+
// case label itmes, create a shared case block to generate the shared
2755+
// block.
2756+
if (hasFallthrough || caseBlock->getCaseLabelItems().size() > 1) {
27522757
emission.initSharedCaseBlockDest(caseBlock, hasFallthrough);
27532758
}
27542759

test/SILGen/indirect_enum.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,9 @@ func switchTreeB<T>(_ x: TreeB<T>) {
229229
switch x {
230230

231231
// CHECK: bb{{.*}}:
232+
// CHECK: function_ref @$s13indirect_enum1ayyF
232233
// CHECK: destroy_addr [[SCRATCH]]
233234
// CHECK: dealloc_stack [[SCRATCH]]
234-
// CHECK: function_ref @$s13indirect_enum1ayyF
235235
// CHECK: br [[OUTER_CONT:bb[0-9]+]]
236236
case .Nil:
237237
a()
@@ -305,9 +305,9 @@ func switchTreeB<T>(_ x: TreeB<T>) {
305305
// CHECK: br [[INNER_CONT:bb[0-9]+]]
306306

307307
// CHECK: [[INNER_CONT]]:
308+
// CHECK: function_ref @$s13indirect_enum1dyyF
308309
// CHECK: destroy_addr [[SCRATCH]]
309310
// CHECK: dealloc_stack [[SCRATCH]]
310-
// CHECK: function_ref @$s13indirect_enum1dyyF
311311
// CHECK: br [[OUTER_CONT]]
312312
default:
313313
d()

test/SILGen/switch.swift

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -304,11 +304,13 @@ func test_isa_1(p: P) {
304304
case is X:
305305
// CHECK: [[IS_X]]:
306306
// CHECK-NEXT: load [trivial] [[TMPBUF]]
307+
// CHECK-NEXT: // function_ref
308+
// CHECK-NEXT: [[FUNC:%.*]] = function_ref @$s6switch1ayyF
309+
// CHECK-NEXT: apply [[FUNC]]()
307310
// CHECK-NEXT: dealloc_stack [[TMPBUF]]
308311
// CHECK-NEXT: destroy_addr [[PTMPBUF]]
309312
// CHECK-NEXT: dealloc_stack [[PTMPBUF]]
310313
a()
311-
// CHECK: function_ref @$s6switch1ayyF
312314
// CHECK: br [[CONT:bb[0-9]+]]
313315

314316
// CHECK: [[IS_NOT_X]]:
@@ -423,9 +425,9 @@ func test_isa_class_1(x: B) {
423425

424426
// CHECK: [[YES_CASE1]]:
425427
case is D1 where runced():
428+
// CHECK: function_ref @$s6switch1ayyF
426429
// CHECK: destroy_value [[CAST_D1_COPY]]
427430
// CHECK: end_borrow [[CAST_D1]]
428-
// CHECK: function_ref @$s6switch1ayyF
429431
// CHECK: br [[CONT:bb[0-9]+]]
430432
a()
431433

@@ -443,8 +445,8 @@ func test_isa_class_1(x: B) {
443445
case is D2:
444446
// CHECK: [[IS_D2]]([[CAST_D2:%.*]] : @guaranteed $D2):
445447
// CHECK: [[CAST_D2_COPY:%.*]] = copy_value [[CAST_D2]]
446-
// CHECK: destroy_value [[CAST_D2_COPY]]
447448
// CHECK: function_ref @$s6switch1byyF
449+
// CHECK: destroy_value [[CAST_D2_COPY]]
448450
// CHECK: br [[CONT]]
449451
b()
450452

@@ -458,8 +460,8 @@ func test_isa_class_1(x: B) {
458460
// CHECK: cond_br {{%.*}}, [[CASE3:bb[0-9]+]], [[NO_CASE3:bb[0-9]+]]
459461

460462
// CHECK: [[CASE3]]:
461-
// CHECK: destroy_value [[CAST_E_COPY]]
462463
// CHECK: function_ref @$s6switch1cyyF
464+
// CHECK: destroy_value [[CAST_E_COPY]]
463465
// CHECK: br [[CONT]]
464466
c()
465467

@@ -478,9 +480,10 @@ func test_isa_class_1(x: B) {
478480
case is C:
479481
// CHECK: [[IS_C]]([[CAST_C:%.*]] : @guaranteed $C):
480482
// CHECK: [[CAST_C_COPY:%.*]] = copy_value [[CAST_C]]
483+
// CHECK: function_ref @$s6switch1dyyF
484+
// CHECK-NEXT: apply
481485
// CHECK: destroy_value [[CAST_C_COPY]]
482486
// CHECK: end_borrow [[CAST_C]]
483-
// CHECK: function_ref @$s6switch1dyyF
484487
// CHECK: br [[CONT]]
485488
d()
486489

@@ -799,25 +802,28 @@ func test_union_addr_only_1(u: MaybeAddressOnlyPair) {
799802
// CHECK: [[IS_LEFT]]:
800803
// CHECK: [[P:%.*]] = unchecked_take_enum_data_addr [[ENUM_ADDR]] : $*MaybeAddressOnlyPair, #MaybeAddressOnlyPair.Left!enumelt.1
801804
case .Left(_):
805+
// CHECK: [[FUNC:%.*]] = function_ref @$s6switch1byyF
806+
// CHECK-NEXT: apply [[FUNC]](
802807
// CHECK: destroy_addr [[P]]
803-
// CHECK: function_ref @$s6switch1byyF
804808
// CHECK: br [[CONT]]
805809
b()
806810

807811
// CHECK: [[IS_RIGHT]]:
808812
// CHECK: [[STR_ADDR:%.*]] = unchecked_take_enum_data_addr [[ENUM_ADDR]] : $*MaybeAddressOnlyPair, #MaybeAddressOnlyPair.Right!enumelt.1
809813
// CHECK: [[STR:%.*]] = load [take] [[STR_ADDR]]
810814
case .Right(_):
815+
// CHECK: [[FUNC:%.*]] = function_ref @$s6switch1cyyF
816+
// CHECK: apply [[FUNC]](
811817
// CHECK: destroy_value [[STR]] : $String
812-
// CHECK: function_ref @$s6switch1cyyF
813818
// CHECK: br [[CONT]]
814819
c()
815820

816821
// CHECK: [[IS_BOTH]]:
817822
// CHECK: [[P_STR_TUPLE:%.*]] = unchecked_take_enum_data_addr [[ENUM_ADDR]] : $*MaybeAddressOnlyPair, #MaybeAddressOnlyPair.Both!enumelt.1
818823
case .Both(_):
824+
// CHECK: [[FUNC:%.*]] = function_ref @$s6switch1dyyF
825+
// CHECK-NEXT: apply [[FUNC]](
819826
// CHECK: destroy_addr [[P_STR_TUPLE]]
820-
// CHECK: function_ref @$s6switch1dyyF
821827
// CHECK: br [[CONT]]
822828
d()
823829
}

test/SILGen/switch_var.swift

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -431,16 +431,17 @@ func test_let() {
431431
case bars():
432432
// CHECK: [[YES_CASE3]]:
433433
// CHECK: destroy_value [[VAL_COPY_3]]
434+
// CHECK: [[FUNC:%.*]] = function_ref @$s10switch_var1cyyF
435+
// CHECK-NEXT: apply [[FUNC]](
434436
// CHECK: destroy_value [[VAL]]
435-
// CHECK: function_ref @$s10switch_var1cyyF
436437
// CHECK: br [[CONT]]
437438
c()
438-
// CHECK: [[NO_CASE3]]:
439-
// CHECK: destroy_value [[VAL_COPY_3]]
440439

441440
case _:
442-
// CHECK: destroy_value [[VAL]]
441+
// CHECK: [[NO_CASE3]]:
442+
// CHECK: destroy_value [[VAL_COPY_3]]
443443
// CHECK: function_ref @$s10switch_var1dyyF
444+
// CHECK: destroy_value [[VAL]]
444445
// CHECK: br [[CONT]]
445446
d()
446447
}
@@ -504,18 +505,17 @@ func test_mixed_let_var() {
504505
case bars():
505506
// CHECK: [[CASE3]]:
506507
// CHECK: destroy_value [[VAL_COPY]]
507-
// CHECK: destroy_value [[VAL]]
508508
// CHECK: [[FUNC:%.*]] = function_ref @$s10switch_var1cyyF : $@convention(thin) () -> ()
509509
// CHECK: apply [[FUNC]]()
510+
// CHECK: destroy_value [[VAL]]
510511
// CHECK: br [[CONT]]
511512
c()
512513

513514
// CHECK: [[NOCASE3]]:
514515
// CHECK: destroy_value [[VAL_COPY]]
515-
516-
// CHECK: destroy_value [[VAL]]
517516
// CHECK: [[D_FUNC:%.*]] = function_ref @$s10switch_var1dyyF : $@convention(thin) () -> ()
518517
// CHECK: apply [[D_FUNC]]()
518+
// CHECK: destroy_value [[VAL]]
519519
// CHECK: br [[CONT]]
520520
case _:
521521
d()

0 commit comments

Comments
 (0)