Skip to content

Commit ad0a331

Browse files
jckarterjckarter
authored
Merge pull request #72111 from jckarter/move-only-optional-chains
SILGen: Forward move-only ownership through optional chains.
2 parents 66651de + 828abb3 commit ad0a331

File tree

8 files changed

+360
-113
lines changed

8 files changed

+360
-113
lines changed

‎lib/SILGen/SILGenBuilder.cpp

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,72 @@ ManagedValue SILGenBuilder::createMarkDependence(
10361036
return cloner.clone(mdi);
10371037
}
10381038

1039+
namespace {
1040+
class EndAccessCleanup final : public Cleanup {
1041+
SILValue beginAccess;
1042+
public:
1043+
EndAccessCleanup(SILValue beginAccess)
1044+
: beginAccess(beginAccess)
1045+
{}
1046+
1047+
void emit(SILGenFunction &SGF, CleanupLocation loc, ForUnwind_t forUnwind)
1048+
override {
1049+
SGF.B.createEndAccess(loc, beginAccess, /*aborted*/ false);
1050+
}
1051+
1052+
void dump(SILGenFunction &SGF) const override {
1053+
llvm::errs() << "EndAccessCleanup\n";
1054+
if (beginAccess) {
1055+
beginAccess->print(llvm::errs());
1056+
}
1057+
}
1058+
};
1059+
}
1060+
1061+
ManagedValue
1062+
SILGenBuilder::createOpaqueBorrowBeginAccess(SILLocation loc,
1063+
ManagedValue address) {
1064+
auto access = createBeginAccess(loc, address.getValue(),
1065+
SILAccessKind::Read,
1066+
SILAccessEnforcement::Static,
1067+
/*no nested conflict*/ true, false);
1068+
SGF.Cleanups.pushCleanup<EndAccessCleanup>(access);
1069+
return ManagedValue::forBorrowedAddressRValue(access);
1070+
}
1071+
1072+
ManagedValue
1073+
SILGenBuilder::createOpaqueConsumeBeginAccess(SILLocation loc,
1074+
ManagedValue address) {
1075+
auto access = createBeginAccess(loc, address.forward(SGF),
1076+
SILAccessKind::Deinit,
1077+
SILAccessEnforcement::Static,
1078+
/*no nested conflict*/ true, false);
1079+
SGF.Cleanups.pushCleanup<EndAccessCleanup>(access);
1080+
return SGF.emitManagedRValueWithCleanup(access);
1081+
}
1082+
1083+
ManagedValue
1084+
SILGenBuilder::createFormalAccessOpaqueBorrowBeginAccess(SILLocation loc,
1085+
ManagedValue address) {
1086+
auto access = createBeginAccess(loc, address.getValue(),
1087+
SILAccessKind::Read,
1088+
SILAccessEnforcement::Static,
1089+
/*no nested conflict*/ true, false);
1090+
SGF.Cleanups.pushCleanup<EndAccessCleanup>(access);
1091+
return ManagedValue::forBorrowedAddressRValue(access);
1092+
}
1093+
1094+
ManagedValue
1095+
SILGenBuilder::createFormalAccessOpaqueConsumeBeginAccess(SILLocation loc,
1096+
ManagedValue address) {
1097+
auto access = createBeginAccess(loc, address.forward(SGF),
1098+
SILAccessKind::Deinit,
1099+
SILAccessEnforcement::Static,
1100+
/*no nested conflict*/ true, false);
1101+
SGF.Cleanups.pushCleanup<EndAccessCleanup>(access);
1102+
return SGF.emitFormalAccessManagedRValueWithCleanup(loc, access);
1103+
}
1104+
10391105
ManagedValue SILGenBuilder::createBeginBorrow(SILLocation loc,
10401106
ManagedValue value,
10411107
bool isLexical,

‎lib/SILGen/SILGenBuilder.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,16 @@ class SILGenBuilder : public SILBuilder {
468468
ManagedValue base,
469469
MarkDependenceKind dependencekind);
470470

471+
ManagedValue createOpaqueBorrowBeginAccess(SILLocation loc,
472+
ManagedValue address);
473+
ManagedValue createFormalAccessOpaqueBorrowBeginAccess(SILLocation loc,
474+
ManagedValue address);
475+
476+
ManagedValue createOpaqueConsumeBeginAccess(SILLocation loc,
477+
ManagedValue address);
478+
ManagedValue createFormalAccessOpaqueConsumeBeginAccess(SILLocation loc,
479+
ManagedValue address);
480+
471481
using SILBuilder::createBeginBorrow;
472482
ManagedValue createBeginBorrow(SILLocation loc, ManagedValue value,
473483
bool isLexical = false,

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5407,28 +5407,6 @@ void SILGenFunction::emitAssignToPatternVars(
54075407
TupleLValueAssigner(*this, loc, destLVs).emit(destType, std::move(src));
54085408
}
54095409

5410-
void SILGenFunction::emitBindOptionalAddress(SILLocation loc,
5411-
ManagedValue optAddress,
5412-
unsigned depth) {
5413-
assert(optAddress.getType().isAddress() && "Expected an address here");
5414-
assert(depth < BindOptionalFailureDests.size());
5415-
auto failureDest =
5416-
BindOptionalFailureDests[BindOptionalFailureDests.size() - depth - 1];
5417-
assert(failureDest.isValid() && "too big to fail");
5418-
5419-
// Since we know that we have an address, we do not need to worry about
5420-
// ownership invariants. Instead just use a select_enum_addr.
5421-
SILBasicBlock *someBB = createBasicBlock();
5422-
SILValue hasValue = emitDoesOptionalHaveValue(loc, optAddress.getValue());
5423-
5424-
auto noneBB = Cleanups.emitBlockForCleanups(failureDest, loc);
5425-
B.createCondBranch(loc, hasValue, someBB, noneBB);
5426-
5427-
// Reset the insertion point at the end of hasValueBB so we can
5428-
// continue to emit code there.
5429-
B.setInsertionPoint(someBB);
5430-
}
5431-
54325410
ManagedValue SILGenFunction::emitBindOptional(SILLocation loc,
54335411
ManagedValue optValue,
54345412
unsigned depth) {
@@ -5440,6 +5418,11 @@ ManagedValue SILGenFunction::emitBindOptional(SILLocation loc,
54405418
SILBasicBlock *hasValueBB = createBasicBlock();
54415419
SILBasicBlock *hasNoValueBB = createBasicBlock();
54425420

5421+
// For move checking purposes, binding always consumes the value whole.
5422+
if (optValue.getType().isMoveOnly() && optValue.getType().isAddress()) {
5423+
optValue = B.createFormalAccessOpaqueConsumeBeginAccess(loc, optValue);
5424+
}
5425+
54435426
SILType optValueTy = optValue.getType();
54445427
SwitchEnumBuilder SEB(B, loc, optValue);
54455428
SEB.addOptionalSomeCase(hasValueBB, nullptr,

‎lib/SILGen/SILGenFunction.h

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,22 @@ enum class SGFAccessKind : uint8_t {
162162
OwnedObjectConsume,
163163
};
164164

165+
static inline bool isBorrowAccess(SGFAccessKind kind) {
166+
switch (kind) {
167+
case SGFAccessKind::IgnoredRead:
168+
case SGFAccessKind::BorrowedAddressRead:
169+
case SGFAccessKind::BorrowedObjectRead:
170+
return true;
171+
case SGFAccessKind::OwnedAddressRead:
172+
case SGFAccessKind::OwnedObjectRead:
173+
case SGFAccessKind::Write:
174+
case SGFAccessKind::ReadWrite:
175+
case SGFAccessKind::OwnedAddressConsume:
176+
case SGFAccessKind::OwnedObjectConsume:
177+
return false;
178+
}
179+
}
180+
165181
static inline bool isReadAccess(SGFAccessKind kind) {
166182
return uint8_t(kind) <= uint8_t(SGFAccessKind::OwnedObjectRead);
167183
}
@@ -2235,14 +2251,6 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
22352251
ManagedValue optionalAddrOrValue,
22362252
unsigned depth);
22372253

2238-
/// Emit the control flow for an optional 'bind' operation, branching to the
2239-
/// active failure destination if the optional value addressed by optionalAddr
2240-
/// is nil, and leaving the insertion point on the success branch.
2241-
///
2242-
/// NOTE: This operation does not consume the managed address.
2243-
void emitBindOptionalAddress(SILLocation loc, ManagedValue optionalAddr,
2244-
unsigned depth);
2245-
22462254
void emitOptionalEvaluation(SILLocation loc, Type optionalType,
22472255
SmallVectorImpl<ManagedValue> &results,
22482256
SGFContext C,

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 88 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -526,29 +526,46 @@ static LValueTypeData getValueTypeData(SILGenFunction &SGF,
526526

527527
/// Given the address of an optional value, unsafely project out the
528528
/// address of the value.
529-
static ManagedValue getAddressOfOptionalValue(SILGenFunction &SGF,
529+
static ManagedValue getPayloadOfOptionalValue(SILGenFunction &SGF,
530530
SILLocation loc,
531-
ManagedValue optAddr,
532-
const LValueTypeData &valueTypeData) {
531+
ManagedValue optBase,
532+
const LValueTypeData &valueTypeData,
533+
SGFAccessKind accessKind) {
533534
// Project out the 'Some' payload.
534535
EnumElementDecl *someDecl = SGF.getASTContext().getOptionalSomeDecl();
535536

536537
// If the base is +1, we want to forward the cleanup.
537-
bool hadCleanup = optAddr.hasCleanup();
538+
SILValue value;
539+
bool isOwned;
540+
if (optBase.isPlusOne(SGF)) {
541+
value = optBase.forward(SGF);
542+
isOwned = true;
543+
} else {
544+
value = optBase.getValue();
545+
isOwned = false;
546+
}
538547

539548
// UncheckedTakeEnumDataAddr is safe to apply to Optional, because it is
540549
// a single-payload enum. There will (currently) never be spare bits
541550
// embedded in the payload.
542-
SILValue valueAddr =
543-
SGF.B.createUncheckedTakeEnumDataAddr(loc, optAddr.forward(SGF), someDecl,
551+
SILValue payload;
552+
if (optBase.getType().isAddress()) {
553+
payload = SGF.B.createUncheckedTakeEnumDataAddr(loc, value, someDecl,
544554
SILType::getPrimitiveAddressType(
545-
valueTypeData.TypeOfRValue));
555+
valueTypeData.TypeOfRValue));
556+
} else {
557+
payload = SGF.B.createUncheckedEnumData(loc, value, someDecl,
558+
SILType::getPrimitiveObjectType(
559+
valueTypeData.TypeOfRValue));
560+
}
546561

547562
// Return the value as +1 if the optional was +1.
548-
if (hadCleanup) {
549-
return SGF.emitManagedBufferWithCleanup(valueAddr);
563+
if (isOwned) {
564+
return SGF.emitManagedBufferWithCleanup(payload);
565+
} else if (payload->getType().isAddress()) {
566+
return ManagedValue::forLValue(payload);
550567
} else {
551-
return ManagedValue::forLValue(valueAddr);
568+
return ManagedValue::forBorrowedRValue(payload);
552569
}
553570
}
554571

@@ -4348,10 +4365,12 @@ LValue SILGenLValue::visitBindOptionalExpr(BindOptionalExpr *e,
43484365
// Binding reads the base even if we then only write to the result.
43494366
auto baseAccessKind = getBaseAccessKindForStorage(accessKind);
43504367

4351-
// We're going to take the address of the base.
4352-
// TODO: deal more efficiently with an object-preferring access.
4353-
baseAccessKind = getAddressAccessKind(baseAccessKind);
4354-
4368+
if (!isBorrowAccess(accessKind)) {
4369+
// We're going to take the address of the base.
4370+
// TODO: deal more efficiently with an object-preferring access.
4371+
baseAccessKind = getAddressAccessKind(baseAccessKind);
4372+
}
4373+
43554374
// Do formal evaluation of the base l-value.
43564375
LValue optLV = visitRec(e->getSubExpr(), baseAccessKind,
43574376
options.forComputedBaseLValue());
@@ -4360,21 +4379,65 @@ LValue SILGenLValue::visitBindOptionalExpr(BindOptionalExpr *e,
43604379
LValueTypeData valueTypeData =
43614380
getOptionalObjectTypeData(SGF, accessKind, optTypeData);
43624381

4363-
// The chaining operator immediately begins a formal access to the
4364-
// base l-value. In concrete terms, this means we can immediately
4365-
// evaluate the base down to an address.
4366-
ManagedValue optAddr = SGF.emitAddressOfLValue(e, std::move(optLV));
4367-
4382+
// The chaining operator immediately evaluates the base.
4383+
// For move-checking purposes, the binding is also treated as an opaque use
4384+
// of the entire value, since we can't leave the value partially initialized
4385+
// across multiple optional binding expressions.
4386+
ManagedValue optBase;
4387+
if (isBorrowAccess(baseAccessKind)) {
4388+
optBase = SGF.emitBorrowedLValue(e, std::move(optLV));
4389+
4390+
if (optBase.getType().isMoveOnly()) {
4391+
if (optBase.getType().isAddress()) {
4392+
optBase = SGF.B.createFormalAccessOpaqueBorrowBeginAccess(e, optBase);
4393+
if (optBase.getType().isLoadable(SGF.F)) {
4394+
optBase = SGF.B.createFormalAccessLoadBorrow(e, optBase);
4395+
}
4396+
} else {
4397+
optBase = SGF.B.createFormalAccessBeginBorrow(e, optBase,
4398+
false,
4399+
/*fixed*/ true);
4400+
}
4401+
}
4402+
} else {
4403+
optBase = SGF.emitAddressOfLValue(e, std::move(optLV));
4404+
4405+
if (isConsumeAccess(accessKind)) {
4406+
if (optBase.getType().isMoveOnly()) {
4407+
optBase = SGF.B.createFormalAccessOpaqueConsumeBeginAccess(e, optBase);
4408+
} else {
4409+
// Take ownership of the base.
4410+
optBase = SGF.emitFormalAccessManagedRValueWithCleanup(e,
4411+
optBase.getValue());
4412+
}
4413+
}
4414+
}
43684415
// Bind the value, branching to the destination address if there's no
43694416
// value there.
4370-
SGF.emitBindOptionalAddress(e, optAddr, e->getDepth());
4417+
assert(e->getDepth() < SGF.BindOptionalFailureDests.size());
4418+
auto failureDepth = SGF.BindOptionalFailureDests.size() - e->getDepth() - 1;
4419+
auto failureDest = SGF.BindOptionalFailureDests[failureDepth];
4420+
assert(failureDest.isValid() && "too big to fail");
4421+
4422+
// Since we know that we have an address, we do not need to worry about
4423+
// ownership invariants. Instead just use a select_enum_addr.
4424+
SILBasicBlock *someBB = SGF.createBasicBlock();
4425+
SILValue hasValue = SGF.emitDoesOptionalHaveValue(e, optBase.getValue());
4426+
4427+
auto noneBB = SGF.Cleanups.emitBlockForCleanups(failureDest, e);
4428+
SGF.B.createCondBranch(e, hasValue, someBB, noneBB);
4429+
4430+
// Reset the insertion point at the end of hasValueBB so we can
4431+
// continue to emit code there.
4432+
SGF.B.setInsertionPoint(someBB);
43714433

43724434
// Project out the payload on the success branch. We can just use a
43734435
// naked ValueComponent here; this is effectively a separate l-value.
4374-
ManagedValue valueAddr =
4375-
getAddressOfOptionalValue(SGF, e, optAddr, valueTypeData);
4436+
ManagedValue optPayload =
4437+
getPayloadOfOptionalValue(SGF, e, optBase, valueTypeData, accessKind);
43764438
LValue valueLV;
4377-
valueLV.add<ValueComponent>(valueAddr, std::nullopt, valueTypeData);
4439+
valueLV.add<ValueComponent>(optPayload, std::nullopt, valueTypeData,
4440+
/*is rvalue*/optBase.getType().isObject());
43784441
return valueLV;
43794442
}
43804443

@@ -5324,13 +5387,11 @@ RValue SILGenFunction::emitRValueForStorageLoad(
53245387
ManagedValue SILGenFunction::emitAddressOfLValue(SILLocation loc,
53255388
LValue &&src,
53265389
TSanKind tsanKind) {
5327-
// Write is only included in this list because of property behaviors.
5328-
// It's obviously unreasonable, though.
53295390
assert(src.getAccessKind() == SGFAccessKind::IgnoredRead ||
53305391
src.getAccessKind() == SGFAccessKind::BorrowedAddressRead ||
53315392
src.getAccessKind() == SGFAccessKind::OwnedAddressRead ||
5332-
src.getAccessKind() == SGFAccessKind::ReadWrite ||
5333-
src.getAccessKind() == SGFAccessKind::Write);
5393+
src.getAccessKind() == SGFAccessKind::OwnedAddressConsume ||
5394+
src.getAccessKind() == SGFAccessKind::ReadWrite);
53345395

53355396
ManagedValue addr;
53365397
PathComponent &&component =

0 commit comments

Comments
 (0)