Skip to content

Commit 6df106e

Browse files
swift-ciswift-ci
authored
Merge pull request #15248 from gottesmm/emit_bind_optional_fixes
2 parents b15fb39 + dde4f58 commit 6df106e

17 files changed

+391
-180
lines changed

‎lib/SILGen/SILGenConvert.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -391,9 +391,8 @@ SILGenFunction::emitOptionalToOptional(SILLocation loc,
391391
finalResult = B.createOwnedPHIArgument(resultTL.getLoweredType());
392392
}
393393

394-
SEBuilder.addCase(
395-
getASTContext().getOptionalSomeDecl(), isPresentBB, contBB,
396-
[&](ManagedValue input, SwitchCaseFullExpr &scope) {
394+
SEBuilder.addOptionalSomeCase(
395+
isPresentBB, contBB, [&](ManagedValue input, SwitchCaseFullExpr &&scope) {
397396
// If we have an address only type, we want to match the old behavior of
398397
// transforming the underlying type instead of the optional type. This
399398
// ensures that we use the more efficient non-generic code paths when
@@ -420,9 +419,9 @@ SILGenFunction::emitOptionalToOptional(SILLocation loc,
420419
return scope.exitAndBranch(loc);
421420
});
422421

423-
SEBuilder.addCase(
424-
getASTContext().getOptionalNoneDecl(), isNotPresentBB, contBB,
425-
[&](ManagedValue input, SwitchCaseFullExpr &scope) {
422+
SEBuilder.addOptionalNoneCase(
423+
isNotPresentBB, contBB,
424+
[&](ManagedValue input, SwitchCaseFullExpr &&scope) {
426425
if (!(resultTL.isAddressOnly() && silConv.useLoweredAddresses())) {
427426
SILValue none =
428427
B.createManagedOptionalNone(loc, resultTy).forward(*this);

‎lib/SILGen/SILGenDecl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,7 @@ void EnumElementPatternInitialization::emitEnumMatch(
779779
// path, causing a use (the destroy on the negative path) to be created that
780780
// does not dominate its definition (in the positive path).
781781
auto handler = [&SGF, &loc, &failureDest](ManagedValue mv,
782-
SwitchCaseFullExpr &expr) {
782+
SwitchCaseFullExpr &&expr) {
783783
expr.exit();
784784
SGF.Cleanups.emitBranchAndCleanups(failureDest, loc);
785785
};
@@ -804,7 +804,7 @@ void EnumElementPatternInitialization::emitEnumMatch(
804804
switchBuilder.addCase(
805805
eltDecl, someBlock, contBlock,
806806
[&SGF, &loc, &eltDecl, &subInit, &value](ManagedValue mv,
807-
SwitchCaseFullExpr &expr) {
807+
SwitchCaseFullExpr &&expr) {
808808
// If the enum case has no bound value, we're done.
809809
if (!eltDecl->hasAssociatedValues()) {
810810
assert(

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4253,24 +4253,24 @@ static ManagedValue flattenOptional(SILGenFunction &SGF, SILLocation loc,
42534253

42544254
SwitchEnumBuilder SEB(SGF.B, loc, optVal);
42554255

4256-
auto *someDecl = SGF.getASTContext().getOptionalSomeDecl();
4257-
SEB.addCase(someDecl, isPresentBB, contBB, [&](ManagedValue input,
4258-
SwitchCaseFullExpr &scope) {
4259-
if (resultTL.isAddressOnly()) {
4260-
SILValue addr =
4261-
addrOnlyResultBuf->getAddressForInPlaceInitialization(SGF, loc);
4262-
input = SGF.B.createUncheckedTakeEnumDataAddr(
4263-
loc, input, someDecl, input.getType().getOptionalObjectType());
4264-
SGF.B.createCopyAddr(loc, input.getValue(), addr, IsNotTake,
4265-
IsInitialization);
4266-
scope.exitAndBranch(loc);
4267-
return;
4268-
}
4269-
scope.exitAndBranch(loc, input.forward(SGF));
4270-
});
4271-
SEB.addCase(
4272-
SGF.getASTContext().getOptionalNoneDecl(), isNotPresentBB, contBB,
4273-
[&](ManagedValue input, SwitchCaseFullExpr &scope) {
4256+
SEB.addOptionalSomeCase(
4257+
isPresentBB, contBB, [&](ManagedValue input, SwitchCaseFullExpr &&scope) {
4258+
if (resultTL.isAddressOnly()) {
4259+
SILValue addr =
4260+
addrOnlyResultBuf->getAddressForInPlaceInitialization(SGF, loc);
4261+
auto *someDecl = SGF.getASTContext().getOptionalSomeDecl();
4262+
input = SGF.B.createUncheckedTakeEnumDataAddr(
4263+
loc, input, someDecl, input.getType().getOptionalObjectType());
4264+
SGF.B.createCopyAddr(loc, input.getValue(), addr, IsNotTake,
4265+
IsInitialization);
4266+
scope.exitAndBranch(loc);
4267+
return;
4268+
}
4269+
scope.exitAndBranch(loc, input.forward(SGF));
4270+
});
4271+
SEB.addOptionalNoneCase(
4272+
isNotPresentBB, contBB,
4273+
[&](ManagedValue input, SwitchCaseFullExpr &&scope) {
42744274
if (resultTL.isAddressOnly()) {
42754275
SILValue addr =
42764276
addrOnlyResultBuf->getAddressForInPlaceInitialization(SGF, loc);
@@ -4896,24 +4896,32 @@ RValue RValueEmitter::visitAssignExpr(AssignExpr *E, SGFContext C) {
48964896
return SGF.emitEmptyTupleRValue(E, C);
48974897
}
48984898

4899-
void SILGenFunction::emitBindOptional(SILLocation loc,
4900-
ManagedValue optionalAddrOrValue,
4899+
void SILGenFunction::emitBindOptional(SILLocation loc, ManagedValue optValue,
49014900
unsigned depth) {
49024901
assert(depth < BindOptionalFailureDests.size());
49034902
auto failureDest = BindOptionalFailureDests[BindOptionalFailureDests.size()
49044903
- depth - 1];
49054904

4906-
// Check whether the optional has a value.
49074905
SILBasicBlock *hasValueBB = createBasicBlock();
4908-
auto hasValue =
4909-
emitDoesOptionalHaveValue(loc, optionalAddrOrValue.getValue());
4910-
4906+
SILBasicBlock *hasNoValueBB = createBasicBlock();
4907+
4908+
// We make a copy to ensure that we pass the value into the
4909+
// switch_enum at plus 1. This is important since emitBindOptional
4910+
// does not consume its argument. Additionally, we need to make sure
4911+
// to create the switch enum builder /after/ emitting the block for
4912+
// cleanups lest we emit an extra destroy in the .none block.
4913+
SwitchEnumBuilder SEB(B, loc, optValue.copy(*this, loc));
4914+
SEB.addOptionalSomeCase(hasValueBB);
49114915
// If not, thread out through a bunch of cleanups.
4912-
SILBasicBlock *hasNoValueBB = Cleanups.emitBlockForCleanups(failureDest, loc);
4913-
B.createCondBranch(loc, hasValue, hasValueBB, hasNoValueBB);
4914-
4915-
// If so, continue.
4916-
B.emitBlock(hasValueBB);
4916+
SEB.addOptionalNoneCase(hasNoValueBB, failureDest,
4917+
[&](ManagedValue mv, SwitchCaseFullExpr &&expr) {
4918+
expr.exitAndBranch(loc);
4919+
});
4920+
std::move(SEB).emit();
4921+
4922+
// Reset the insertion point at the end of hasValueBB so we can
4923+
// continue to emit code there.
4924+
B.setInsertionPoint(hasValueBB);
49174925
}
49184926

49194927
RValue RValueEmitter::visitBindOptionalExpr(BindOptionalExpr *E, SGFContext C) {

‎lib/SILGen/SILGenStmt.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -816,10 +816,9 @@ void StmtEmitter::visitForEachStmt(ForEachStmt *S) {
816816
SILBasicBlock *failExitingBlock = createBasicBlock();
817817
SwitchEnumBuilder switchEnumBuilder(SGF.B, S, nextBufOrValue);
818818

819-
switchEnumBuilder.addCase(
820-
SGF.getASTContext().getOptionalSomeDecl(), createBasicBlock(),
821-
loopDest.getBlock(),
822-
[&](ManagedValue inputValue, SwitchCaseFullExpr &scope) {
819+
switchEnumBuilder.addOptionalSomeCase(
820+
createBasicBlock(), loopDest.getBlock(),
821+
[&](ManagedValue inputValue, SwitchCaseFullExpr &&scope) {
823822
SGF.emitProfilerIncrement(S->getBody());
824823

825824
// Emit the loop body.
@@ -868,8 +867,10 @@ void StmtEmitter::visitForEachStmt(ForEachStmt *S) {
868867

869868
// If we emitted an unreachable in the body, we will not have a valid
870869
// insertion point. Just return early.
871-
if (!SGF.B.hasValidInsertionPoint())
870+
if (!SGF.B.hasValidInsertionPoint()) {
871+
scope.unreachableExit();
872872
return;
873+
}
873874

874875
// Otherwise, associate the loop body's closing brace with this branch.
875876
RegularLocation L(S->getBody());
@@ -881,10 +882,9 @@ void StmtEmitter::visitForEachStmt(ForEachStmt *S) {
881882
// We add loop fail block, just to be defensive about intermediate
882883
// transformations performing cleanups at scope.exit(). We still jump to the
883884
// contBlock.
884-
switchEnumBuilder.addCase(
885-
SGF.getASTContext().getOptionalNoneDecl(), createBasicBlock(),
886-
failExitingBlock,
887-
[&](ManagedValue inputValue, SwitchCaseFullExpr &scope) {
885+
switchEnumBuilder.addOptionalNoneCase(
886+
createBasicBlock(), failExitingBlock,
887+
[&](ManagedValue inputValue, SwitchCaseFullExpr &&scope) {
888888
assert(!inputValue && "None should not be passed an argument!");
889889
scope.exitAndBranch(S);
890890
},

‎lib/SILGen/SwitchEnumBuilder.cpp

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,26 +21,47 @@ using namespace Lowering;
2121
// SwitchCaseFullExpr Implementation
2222
//===----------------------------------------------------------------------===//
2323

24+
SwitchCaseFullExpr::SwitchCaseFullExpr(SILGenFunction &SGF, CleanupLocation loc)
25+
: SGF(SGF), scope(SGF.Cleanups, loc), loc(loc), branchDest() {}
26+
2427
SwitchCaseFullExpr::SwitchCaseFullExpr(SILGenFunction &SGF, CleanupLocation loc,
25-
SILBasicBlock *contBlock)
26-
: SGF(SGF), scope(SGF.Cleanups, loc), loc(loc), contBlock(contBlock) {}
28+
SwitchCaseBranchDest branchDest)
29+
: SGF(SGF), scope(SGF.Cleanups, loc), loc(loc), branchDest(branchDest) {}
2730

2831
void SwitchCaseFullExpr::exitAndBranch(SILLocation loc,
2932
ArrayRef<SILValue> branchArgs) {
30-
assert(contBlock &&
31-
"Should not call this if we do not have a continuation block");
33+
assert(bool(branchDest) && "Must have a branch destination!");
3234
assert(SGF.B.hasValidInsertionPoint());
3335
scope.pop();
34-
SGF.B.createBranch(loc, contBlock.get(), branchArgs);
36+
37+
// Then either do a direct branch or a branch + cleanups.
38+
if (SILBasicBlock *block = branchDest.getBlock()) {
39+
SGF.B.createBranch(loc, block, branchArgs);
40+
return;
41+
}
42+
43+
SGF.Cleanups.emitBranchAndCleanups(branchDest.getJumpDest(), loc, branchArgs);
3544
}
3645

3746
void SwitchCaseFullExpr::exit() {
38-
assert(!contBlock &&
47+
assert(!bool(branchDest) &&
3948
"Should not call this if we do have a continuation block");
4049
assert(SGF.B.hasValidInsertionPoint());
4150
scope.pop();
4251
}
4352

53+
SwitchCaseFullExpr::~SwitchCaseFullExpr() {
54+
assert(!scope.isValid() && "Did not pop scope?!");
55+
}
56+
57+
void SwitchCaseFullExpr::unreachableExit() {
58+
// This is important to ensure that we do not actually emit any cleanups since
59+
// we already know that an unreachable was emitted.
60+
assert(!SGF.B.hasValidInsertionPoint() && "Expected to pop scope without a "
61+
"valid insertion point!");
62+
scope.pop();
63+
}
64+
4465
//===----------------------------------------------------------------------===//
4566
// SwitchEnumBuilder Implementation
4667
//===----------------------------------------------------------------------===//
@@ -93,32 +114,30 @@ void SwitchEnumBuilder::emit() && {
93114
defaultBlockData->dispatchTime ==
94115
DefaultDispatchTime::BeforeNormalCases) {
95116
SILBasicBlock *defaultBlock = defaultBlockData->block;
96-
NullablePtr<SILBasicBlock> contBB = defaultBlockData->contBlock;
117+
SwitchCaseBranchDest branchDest = defaultBlockData->branchDest;
97118
DefaultCaseHandler handler = defaultBlockData->handler;
98119

99120
// Don't allow cleanups to escape the conditional block.
100121
SwitchCaseFullExpr presentScope(builder.getSILGenFunction(),
101-
CleanupLocation::get(loc),
102-
contBB.getPtrOrNull());
122+
CleanupLocation::get(loc), branchDest);
103123
builder.emitBlock(defaultBlock);
104124
ManagedValue input = optional;
105125
if (!isAddressOnly) {
106126
input = builder.createOwnedPHIArgument(optional.getType());
107127
}
108-
handler(input, presentScope);
128+
handler(input, std::move(presentScope));
109129
builder.clearInsertionPoint();
110130
}
111131

112132
for (NormalCaseData &caseData : caseDataArray) {
113133
EnumElementDecl *decl = caseData.decl;
114134
SILBasicBlock *caseBlock = caseData.block;
115-
NullablePtr<SILBasicBlock> contBlock = caseData.contBlock;
135+
SwitchCaseBranchDest branchDest = caseData.branchDest;
116136
NormalCaseHandler handler = caseData.handler;
117137

118138
// Don't allow cleanups to escape the conditional block.
119139
SwitchCaseFullExpr presentScope(builder.getSILGenFunction(),
120-
CleanupLocation::get(loc),
121-
contBlock.getPtrOrNull());
140+
CleanupLocation::get(loc), branchDest);
122141

123142
builder.emitBlock(caseBlock);
124143

@@ -132,7 +151,7 @@ void SwitchEnumBuilder::emit() && {
132151
input = builder.createOwnedPHIArgument(inputType);
133152
}
134153
}
135-
handler(input, presentScope);
154+
handler(input, std::move(presentScope));
136155
builder.clearInsertionPoint();
137156
}
138157

@@ -141,19 +160,18 @@ void SwitchEnumBuilder::emit() && {
141160
if (defaultBlockData &&
142161
defaultBlockData->dispatchTime == DefaultDispatchTime::AfterNormalCases) {
143162
SILBasicBlock *defaultBlock = defaultBlockData->block;
144-
NullablePtr<SILBasicBlock> contBB = defaultBlockData->contBlock;
163+
auto branchDest = defaultBlockData->branchDest;
145164
DefaultCaseHandler handler = defaultBlockData->handler;
146165

147166
// Don't allow cleanups to escape the conditional block.
148167
SwitchCaseFullExpr presentScope(builder.getSILGenFunction(),
149-
CleanupLocation::get(loc),
150-
contBB.getPtrOrNull());
168+
CleanupLocation::get(loc), branchDest);
151169
builder.emitBlock(defaultBlock);
152170
ManagedValue input = optional;
153171
if (!isAddressOnly) {
154172
input = builder.createOwnedPHIArgument(optional.getType());
155173
}
156-
handler(input, presentScope);
174+
handler(input, std::move(presentScope));
157175
builder.clearInsertionPoint();
158176
}
159177
}

0 commit comments

Comments
 (0)