Skip to content

Commit 56f05da

Browse files
committed
Fix OperandOwnership for coroutines.
Anything with a borrow scope in the caller needs to be considered a Borrow of its operand. The end_apply needs to be considered the lifetime-ending use of the borrow.
1 parent cdcd0bf commit 56f05da

File tree

1 file changed

+22
-25
lines changed

1 file changed

+22
-25
lines changed

lib/SIL/IR/OperandOwnership.cpp

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ SHOULD_NEVER_VISIT_INST(GetAsyncContinuation)
122122

123123
// Instructions that require trivial operands.
124124
OPERAND_OWNERSHIP(TrivialUse, AwaitAsyncContinuation)
125-
OPERAND_OWNERSHIP(TrivialUse, AbortApply)
126125
OPERAND_OWNERSHIP(TrivialUse, AddressToPointer)
127126
OPERAND_OWNERSHIP(TrivialUse, AllocRef) // with tail operand
128127
OPERAND_OWNERSHIP(TrivialUse, AllocRefDynamic) // with tail operand
@@ -138,7 +137,6 @@ OPERAND_OWNERSHIP(TrivialUse, DebugValueAddr)
138137
OPERAND_OWNERSHIP(TrivialUse, DeinitExistentialAddr)
139138
OPERAND_OWNERSHIP(TrivialUse, DestroyAddr)
140139
OPERAND_OWNERSHIP(TrivialUse, EndAccess)
141-
OPERAND_OWNERSHIP(TrivialUse, EndApply)
142140
OPERAND_OWNERSHIP(TrivialUse, EndUnpairedAccess)
143141
OPERAND_OWNERSHIP(TrivialUse, GetAsyncContinuationAddr)
144142
OPERAND_OWNERSHIP(TrivialUse, IndexAddr)
@@ -261,6 +259,8 @@ OPERAND_OWNERSHIP(ForwardingBorrow, OpenExistentialValue)
261259
OPERAND_OWNERSHIP(ForwardingBorrow, OpenExistentialBoxValue)
262260

263261
OPERAND_OWNERSHIP(EndBorrow, EndBorrow)
262+
OPERAND_OWNERSHIP(EndBorrow, EndApply)
263+
OPERAND_OWNERSHIP(EndBorrow, AbortApply)
264264

265265
#define NEVER_LOADABLE_CHECKED_REF_STORAGE(Name, ...) \
266266
OPERAND_OWNERSHIP(TrivialUse, Load##Name)
@@ -342,21 +342,7 @@ AGGREGATE_OWNERSHIP(SwitchEnum)
342342
// A begin_borrow is conditionally nested.
343343
OperandOwnership
344344
OperandOwnershipClassifier::visitBeginBorrowInst(BeginBorrowInst *borrow) {
345-
switch (borrow->getOperand().getOwnershipKind()) {
346-
case OwnershipKind::Any:
347-
llvm_unreachable("invalid value ownership");
348-
case OwnershipKind::None:
349-
return OperandOwnership::TrivialUse;
350-
case OwnershipKind::Unowned:
351-
// FIXME: disallow borrowing an Unowned value. Temporarily model it as an
352-
// instantaneous use until SILGenFunction::emitClassMemberDestruction is
353-
// fixed.
354-
return OperandOwnership::UnownedInstantaneousUse;
355-
case OwnershipKind::Guaranteed:
356-
return OperandOwnership::NestedBorrow;
357-
case OwnershipKind::Owned:
358-
return OperandOwnership::Borrow;
359-
}
345+
return OperandOwnership::Borrow;
360346
}
361347

362348
// MARK: Instructions whose use ownership depends on the operand in question.
@@ -399,20 +385,31 @@ OperandOwnershipClassifier::visitStoreBorrowInst(StoreBorrowInst *i) {
399385
return OperandOwnership::TrivialUse;
400386
}
401387

402-
static OperandOwnership getFunctionArgOwnership(SILArgumentConvention argConv) {
388+
// Get the OperandOwnership for instaneous apply, yield, and return uses.
389+
// This does not apply to uses that begin an explicit borrow scope in the
390+
// caller, such as begin_apply.
391+
static OperandOwnership getFunctionArgOwnership(SILArgumentConvention argConv,
392+
bool hasScopeInCaller) {
393+
403394
switch (argConv) {
404395
case SILArgumentConvention::Indirect_In:
405396
case SILArgumentConvention::Direct_Owned:
406397
return OperandOwnership::ForwardingConsume;
407398

408-
// A guaranteed argument is forwarded into the callee. However, from the
399+
// A guaranteed argument is forwarded into the callee. If the call itself has
400+
// no scope (i.e. it is a single apply, try_apply or yield), then from the
409401
// caller's point of view it looks like an instantaneous use. Consequently,
410402
// owned values may be passed to guaranteed arguments without an explicit
411-
// borrow scope in the caller.
403+
// borrow scope in the caller. In contrast, a begin_apply /does/ have an
404+
// explicit borrow scope in the caller so we must treat arguments passed to it
405+
// as being borrowed for the entire borrow scope.
412406
case SILArgumentConvention::Indirect_In_Constant:
413407
case SILArgumentConvention::Indirect_In_Guaranteed:
414408
case SILArgumentConvention::Direct_Guaranteed:
415-
return OperandOwnership::InstantaneousUse;
409+
// For an apply that begins a borrow scope, its arguments are borrowed
410+
// throughout the caller's borrow scope.
411+
return hasScopeInCaller ? OperandOwnership::Borrow
412+
: OperandOwnership::InstantaneousUse;
416413

417414
case SILArgumentConvention::Direct_Unowned:
418415
return OperandOwnership::UnownedInstantaneousUse;
@@ -435,15 +432,15 @@ OperandOwnershipClassifier::visitFullApply(FullApplySite apply) {
435432
? SILArgumentConvention(apply.getSubstCalleeType()->getCalleeConvention())
436433
: apply.getArgumentConvention(op);
437434

438-
auto operandOwnership = getFunctionArgOwnership(argConv);
439-
440435
// Allow passing callee operands with no ownership as guaranteed.
441436
// FIXME: why do we allow this?
442437
if (operandOwnership == OperandOwnership::ForwardingBorrow
443438
&& apply.isCalleeOperand(op)) {
444439
return OperandOwnership::InstantaneousUse;
445440
}
446-
return operandOwnership;
441+
return getFunctionArgOwnership(
442+
argConv,
443+
/*hasScopeInCaller*/ apply.beginsCoroutineEvaluation());
447444
}
448445

449446
OperandOwnership
@@ -478,7 +475,7 @@ OperandOwnership OperandOwnershipClassifier::visitYieldInst(YieldInst *i) {
478475
auto fnType = i->getFunction()->getLoweredFunctionType();
479476
SILArgumentConvention argConv(
480477
fnType->getYields()[getOperandIndex()].getConvention());
481-
return getFunctionArgOwnership(argConv);
478+
return getFunctionArgOwnership(argConv, /*hasScopeInCaller*/ false);
482479
}
483480

484481
OperandOwnership OperandOwnershipClassifier::visitReturnInst(ReturnInst *i) {

0 commit comments

Comments
 (0)