Skip to content

Commit a54a23a

Browse files
atrickatrick
authored
Merge pull request #79177 from atrick/fix-markdep-ownership
Fix mark_dependence [nonescaping] ownership
2 parents 57234f8 + 01c3343 commit a54a23a

20 files changed

+85
-21
lines changed

‎SwiftCompilerSources/Sources/Optimizer/Utilities/OwnershipLiveness.swift

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -415,9 +415,11 @@ extension OwnershipUseVisitor {
415415
case .borrow:
416416
return visitBorrowingUse(of: operand)
417417

418-
// TODO: Eventually, visit owned InteriorPointers as implicit borrows.
419-
case .interiorPointer, .trivialUse, .endBorrow, .reborrow,
420-
.guaranteedForwarding:
418+
case .anyInteriorPointer:
419+
return visitInteriorPointerUse(of: operand)
420+
421+
// TODO: .interiorPointer should instead be handled like .anyInteriorPointer.
422+
case .interiorPointer, .trivialUse, .endBorrow, .reborrow, .guaranteedForwarding:
421423
fatalError("ownership incompatible with an owned value");
422424
}
423425
}
@@ -449,7 +451,7 @@ extension OwnershipUseVisitor {
449451
case .borrow:
450452
return visitBorrowingUse(of: operand)
451453

452-
case .interiorPointer:
454+
case .interiorPointer, .anyInteriorPointer:
453455
return visitInteriorPointerUse(of: operand)
454456

455457
case .trivialUse, .forwardingConsume, .destroyingConsume:

‎SwiftCompilerSources/Sources/SIL/Operand.swift

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,11 @@ public enum OperandOwnership {
268268
/// Interior Pointer. Propagates a trivial value (e.g. address, pointer, or no-escape closure) that depends on the guaranteed value within the base's borrow scope. The verifier checks that all uses of the trivial
269269
/// value are in scope. (ref_element_addr, open_existential_box)
270270
case interiorPointer
271-
271+
272+
/// Any Interior Pointer. An interior pointer that allows any operand ownership. This will be removed as soon as SIL
273+
/// migrates away from extraneous borrow scopes.
274+
case anyInteriorPointer
275+
272276
/// Forwarded Borrow. Propagates the guaranteed value within the base's borrow scope. (tuple_extract, struct_extract, cast, switch)
273277
case guaranteedForwarding
274278

@@ -282,7 +286,7 @@ public enum OperandOwnership {
282286
switch self {
283287
case .nonUse, .trivialUse, .instantaneousUse, .unownedInstantaneousUse,
284288
.forwardingUnowned, .pointerEscape, .bitwiseEscape, .borrow,
285-
.interiorPointer, .guaranteedForwarding:
289+
.interiorPointer, .anyInteriorPointer, .guaranteedForwarding:
286290
return false
287291
case .destroyingConsume, .forwardingConsume, .endBorrow, .reborrow:
288292
return true
@@ -313,6 +317,8 @@ public enum OperandOwnership {
313317
return BridgedOperand.OperandOwnership.ForwardingConsume
314318
case .interiorPointer:
315319
return BridgedOperand.OperandOwnership.InteriorPointer
320+
case .anyInteriorPointer:
321+
return BridgedOperand.OperandOwnership.AnyInteriorPointer
316322
case .guaranteedForwarding:
317323
return BridgedOperand.OperandOwnership.GuaranteedForwarding
318324
case .endBorrow:
@@ -337,6 +343,7 @@ extension Operand {
337343
case .DestroyingConsume: return .destroyingConsume
338344
case .ForwardingConsume: return .forwardingConsume
339345
case .InteriorPointer: return .interiorPointer
346+
case .AnyInteriorPointer: return .anyInteriorPointer
340347
case .GuaranteedForwarding: return .guaranteedForwarding
341348
case .EndBorrow: return .endBorrow
342349
case .Reborrow: return .reborrow

‎include/swift/SIL/OwnershipUseVisitor.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,9 @@ bool OwnershipUseVisitor<Impl>::visitOwnedUse(Operand *use) {
397397
}
398398
return handleUsePoint(use, UseLifetimeConstraint::LifetimeEnding);
399399

400+
case OperandOwnership::AnyInteriorPointer:
401+
return visitInteriorPointerUses(use);
402+
400403
case OperandOwnership::PointerEscape:
401404
// TODO: Change ProjectBox ownership to InteriorPointer and allow them to
402405
// take owned values.
@@ -416,7 +419,7 @@ bool OwnershipUseVisitor<Impl>::visitOwnedUse(Operand *use) {
416419
case OperandOwnership::Borrow:
417420
return visitInnerBorrow(use);
418421

419-
// TODO: Eventually, handle owned InteriorPointers as implicit borrows.
422+
// TODO: InteriorPointer should be handled like AnyInteriorPointer.
420423
case OperandOwnership::InteriorPointer:
421424
case OperandOwnership::TrivialUse:
422425
case OperandOwnership::EndBorrow:
@@ -479,6 +482,7 @@ bool OwnershipUseVisitor<Impl>::visitGuaranteedUse(Operand *use) {
479482
return visitInnerBorrow(use);
480483

481484
case OperandOwnership::InteriorPointer:
485+
case OperandOwnership::AnyInteriorPointer:
482486
return visitInteriorPointerUses(use);
483487

484488
case OperandOwnership::TrivialUse:
@@ -490,8 +494,9 @@ bool OwnershipUseVisitor<Impl>::visitGuaranteedUse(Operand *use) {
490494

491495
template <typename Impl>
492496
bool OwnershipUseVisitor<Impl>::visitInteriorPointerUses(Operand *use) {
493-
assert(use->getOperandOwnership() == OperandOwnership::InteriorPointer ||
494-
isa<ProjectBoxInst>(use->getUser()));
497+
assert(use->getOperandOwnership() == OperandOwnership::InteriorPointer
498+
|| use->getOperandOwnership() == OperandOwnership::AnyInteriorPointer
499+
|| isa<ProjectBoxInst>(use->getUser()));
495500

496501
if (auto scopedAddress = ScopedAddressValue::forUse(use)) {
497502
// e.g. client may need to insert end_borrow if scopedAddress is a store_borrow.

‎include/swift/SIL/SILBridging.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,7 @@ struct BridgedOperand {
373373
DestroyingConsume,
374374
ForwardingConsume,
375375
InteriorPointer,
376+
AnyInteriorPointer,
376377
GuaranteedForwarding,
377378
EndBorrow,
378379
Reborrow

‎include/swift/SIL/SILBridgingImpl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,8 @@ BridgedOperand::OperandOwnership BridgedOperand::getOperandOwnership() const {
645645
return OperandOwnership::ForwardingConsume;
646646
case swift::OperandOwnership::InteriorPointer:
647647
return OperandOwnership::InteriorPointer;
648+
case swift::OperandOwnership::AnyInteriorPointer:
649+
return OperandOwnership::AnyInteriorPointer;
648650
case swift::OperandOwnership::GuaranteedForwarding:
649651
return OperandOwnership::GuaranteedForwarding;
650652
case swift::OperandOwnership::EndBorrow:

‎include/swift/SIL/SILValue.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,13 @@ struct OperandOwnership {
848848
/// value are in scope.
849849
/// (ref_element_addr, open_existential_box)
850850
InteriorPointer,
851+
852+
// TODO: Remove AnyInteriorPointer after fixing
853+
// OperandOwnership::getOwnershipConstraint() to allow InteriorPointer
854+
// operands to take any operand ownership. This will prevent useless borrow
855+
// scopes from being generated, so it will require some SIL migration. But
856+
// all OSSA utilities need to correctly handle interior uses anyway.
857+
AnyInteriorPointer,
851858
/// Forwarded Borrow. Propagates the guaranteed value within the base's
852859
/// borrow scope.
853860
/// (tuple_extract, struct_extract, cast, switch)
@@ -942,6 +949,9 @@ inline OwnershipConstraint OperandOwnership::getOwnershipConstraint() {
942949
case OperandOwnership::DestroyingConsume:
943950
case OperandOwnership::ForwardingConsume:
944951
return {OwnershipKind::Owned, UseLifetimeConstraint::LifetimeEnding};
952+
case OperandOwnership::AnyInteriorPointer:
953+
return {OwnershipKind::Any, UseLifetimeConstraint::NonLifetimeEnding};
954+
// TODO: InteriorPointer should be handled like AnyInteriorPointer.
945955
case OperandOwnership::InteriorPointer:
946956
case OperandOwnership::GuaranteedForwarding:
947957
return {OwnershipKind::Guaranteed,
@@ -971,6 +981,7 @@ inline bool canAcceptUnownedValue(OperandOwnership operandOwnership) {
971981
case OperandOwnership::DestroyingConsume:
972982
case OperandOwnership::ForwardingConsume:
973983
case OperandOwnership::InteriorPointer:
984+
case OperandOwnership::AnyInteriorPointer:
974985
case OperandOwnership::GuaranteedForwarding:
975986
case OperandOwnership::EndBorrow:
976987
case OperandOwnership::Reborrow:

‎lib/SIL/IR/OperandOwnership.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ OperandOwnershipClassifier::visitMarkDependenceInst(MarkDependenceInst *mdi) {
678678
// which we treat like a borrow.
679679
return OperandOwnership::Borrow;
680680
}
681-
return OperandOwnership::InteriorPointer;
681+
return OperandOwnership::AnyInteriorPointer;
682682
}
683683
if (mdi->hasUnresolvedEscape()) {
684684
// This creates a dependent value that may extend beyond the parent's

‎lib/SIL/IR/SILInstruction.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2009,6 +2009,7 @@ bool MarkDependenceInst::visitNonEscapingLifetimeEnds(
20092009
llvm::function_ref<bool (Operand *)> visitUnknownUse) const {
20102010
assert(getFunction()->hasOwnership() && isNonEscaping()
20112011
&& "only meaningful for nonescaping dependencies");
2012+
assert(getType().isObject() && "lifetime ends only exist for values");
20122013
bool noUsers = true;
20132014
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, visitScopeEnd,
20142015
visitUnknownUse)) {

‎lib/SIL/IR/SILValue.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,8 @@ StringRef OperandOwnership::asString() const {
564564
return "forwarding-consume";
565565
case OperandOwnership::InteriorPointer:
566566
return "interior-pointer";
567+
case OperandOwnership::AnyInteriorPointer:
568+
return "any-interior-pointer";
567569
case OperandOwnership::GuaranteedForwarding:
568570
return "guaranteed-forwarding";
569571
case OperandOwnership::EndBorrow:

‎lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ bool swift::findPointerEscape(SILValue original) {
8181
});
8282
break;
8383
}
84-
case OperandOwnership::InteriorPointer: {
84+
case OperandOwnership::InteriorPointer:
85+
case OperandOwnership::AnyInteriorPointer: {
8586
if (InteriorPointerOperand(use).findTransitiveUses() !=
8687
AddressUseKind::NonEscaping) {
8788
return true;
@@ -247,6 +248,7 @@ bool swift::findInnerTransitiveGuaranteedUses(
247248
break;
248249

249250
case OperandOwnership::InteriorPointer:
251+
case OperandOwnership::AnyInteriorPointer:
250252
#if 0 // FIXME!!! Enable in a following commit that fixes RAUW
251253
// If our base guaranteed value does not have any consuming uses
252254
// (consider function arguments), we need to be sure to include interior
@@ -387,6 +389,7 @@ bool swift::findExtendedUsesOfSimpleBorrowedValue(
387389
break;
388390

389391
case OperandOwnership::InteriorPointer:
392+
case OperandOwnership::AnyInteriorPointer:
390393
if (InteriorPointerOperandKind::get(use) ==
391394
InteriorPointerOperandKind::Invalid)
392395
return false;
@@ -712,6 +715,7 @@ bool BorrowingOperand::visitScopeEndingUses(
712715
case BorrowingOperandKind::MarkDependenceNonEscaping: {
713716
auto *user = cast<MarkDependenceInst>(op->getUser());
714717
assert(user->isNonEscaping() && "escaping dependencies don't borrow");
718+
assert(user->getType().isObject() && "borrows only exist for values");
715719
return user->visitNonEscapingLifetimeEnds(visitScopeEnd, visitUnknownUse);
716720
}
717721
case BorrowingOperandKind::BeginAsyncLet: {

‎lib/SIL/Utils/PrunedLiveness.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,8 @@ PrunedLiveRange<LivenessWithDefs>::updateForBorrowingOperand(Operand *operand) {
299299
template <typename LivenessWithDefs>
300300
AddressUseKind PrunedLiveRange<LivenessWithDefs>::checkAndUpdateInteriorPointer(
301301
Operand *operand) {
302-
assert(operand->getOperandOwnership() == OperandOwnership::InteriorPointer);
302+
assert(operand->getOperandOwnership() == OperandOwnership::InteriorPointer
303+
|| operand->getOperandOwnership() == OperandOwnership::AnyInteriorPointer);
303304

304305
if (auto scopedAddress = ScopedAddressValue::forUse(operand)) {
305306
scopedAddress.visitScopeEndingUses([this](Operand *end) {
@@ -364,6 +365,7 @@ LiveRangeSummary PrunedLiveRange<LivenessWithDefs>::recursivelyUpdateForDef(
364365
summary.meet(AddressUseKind::PointerEscape);
365366
break;
366367
case OperandOwnership::InteriorPointer:
368+
case OperandOwnership::AnyInteriorPointer:
367369
summary.meet(checkAndUpdateInteriorPointer(use));
368370
break;
369371
case OperandOwnership::GuaranteedForwarding: {

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,12 +1458,16 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
14581458
"kind since guaranteed and owned values can always be passed "
14591459
"in unowned positions");
14601460

1461-
require(operand.getOperandOwnership() !=
1462-
OperandOwnership::InteriorPointer ||
1463-
InteriorPointerOperandKind::get(&operand) !=
1464-
InteriorPointerOperandKind::Invalid,
1465-
"All operands with InteriorPointer operand ownership should be "
1466-
"added to the InteriorPointerOperand utility");
1461+
switch (operand.getOperandOwnership()) {
1462+
default:
1463+
break;
1464+
case OperandOwnership::InteriorPointer:
1465+
case OperandOwnership::AnyInteriorPointer:
1466+
require(InteriorPointerOperandKind::get(&operand) !=
1467+
InteriorPointerOperandKind::Invalid,
1468+
"All operands with InteriorPointer operand ownership should be "
1469+
"added to the InteriorPointerOperand utility");
1470+
}
14671471
}
14681472
}
14691473

‎lib/SILOptimizer/Mandatory/ConsumeOperatorCopyableValuesChecker.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,8 @@ bool CheckerLivenessInfo::compute() {
175175
return true;
176176
});
177177
break;
178-
case OperandOwnership::InteriorPointer: {
178+
case OperandOwnership::InteriorPointer:
179+
case OperandOwnership::AnyInteriorPointer: {
179180
// An interior pointer user extends liveness until the end of the
180181
// interior pointer section.
181182
//

‎lib/SILOptimizer/Mandatory/DiagnoseLifetimeIssues.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ visitUses(SILValue def, bool updateLivenessAndWeakStores, int callDepth) {
261261
continue;
262262

263263
case OperandOwnership::InteriorPointer:
264+
case OperandOwnership::AnyInteriorPointer:
264265
// Treat most interior pointers as escapes until they can be audited.
265266
// But if the interior pointer cannot be used to copy the parent
266267
// reference, then it does not need to be considered an escape.

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1687,6 +1687,7 @@ struct CopiedLoadBorrowEliminationVisitor
16871687
case OperandOwnership::InstantaneousUse:
16881688
case OperandOwnership::UnownedInstantaneousUse:
16891689
case OperandOwnership::InteriorPointer:
1690+
case OperandOwnership::AnyInteriorPointer:
16901691
case OperandOwnership::BitwiseEscape: {
16911692
// Look through copy_value of a move only value. We treat copy_value of
16921693
// copyable values as normal uses.

‎lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ bool Implementation::gatherUses(SILValue value) {
296296
case OperandOwnership::InstantaneousUse:
297297
case OperandOwnership::UnownedInstantaneousUse:
298298
case OperandOwnership::InteriorPointer:
299+
case OperandOwnership::AnyInteriorPointer:
299300
case OperandOwnership::BitwiseEscape: {
300301
// Look through copy_value of a move only value. We treat copy_value of
301302
// copyable values as normal uses.
@@ -1587,6 +1588,7 @@ static bool gatherBorrows(SILValue rootValue,
15871588
}
15881589
continue;
15891590
case OperandOwnership::InteriorPointer:
1591+
case OperandOwnership::AnyInteriorPointer:
15901592
// We don't care about these.
15911593
continue;
15921594
case OperandOwnership::GuaranteedForwarding:

‎lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ bool MoveOnlyObjectCheckerPImpl::checkForSameInstMultipleUseErrors(
243243
instToOperandsMap.insert(nextUse->getUser(), nextUse);
244244
continue;
245245
case OperandOwnership::InteriorPointer:
246+
case OperandOwnership::AnyInteriorPointer:
246247
// We do not care about interior pointer uses since there aren't any
247248
// interior pointer using instructions that are also consuming uses.
248249
continue;

‎lib/SILOptimizer/Utils/CanonicalizeBorrowScope.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,7 @@ bool CanonicalizeBorrowScope::visitBorrowScopeUses(SILValue innerValue,
264264
llvm_unreachable("this operand cannot handle ownership");
265265

266266
case OperandOwnership::InteriorPointer:
267+
case OperandOwnership::AnyInteriorPointer:
267268
case OperandOwnership::EndBorrow:
268269
case OperandOwnership::Reborrow:
269270
// Ignore uses that must be within the borrow scope.

‎lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
238238
}
239239
break;
240240
case OperandOwnership::InteriorPointer:
241+
case OperandOwnership::AnyInteriorPointer:
241242
case OperandOwnership::GuaranteedForwarding:
242243
case OperandOwnership::EndBorrow:
243244
// Guaranteed values are exposed by inner adjacent reborrows. If user is

‎test/SILOptimizer/ossa_lifetime_completion.sil

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -965,7 +965,7 @@ struct Wrapper {
965965

966966
// CHECK-LABEL: begin running test {{.*}} on testInteriorMarkDepNonEscAddressValue: ossa_lifetime_completion
967967
// CHECK-LABEL: sil [ossa] @testInteriorMarkDepNonEscAddressValue : {{.*}} {
968-
// CHECK: mark_dependence
968+
// CHECK: mark_dependence [nonescaping]
969969
// CHECK: end_borrow
970970
// CHECK-LABEL: } // end sil function 'testInteriorMarkDepNonEscAddressValue'
971971
// CHECK-LABEL: end running test {{.*}} on testInteriorMarkDepNonEscAddressValue: ossa_lifetime_completion
@@ -975,6 +975,21 @@ bb0(%0 : @owned $Wrapper, %1 : $*Klass):
975975
%2 = begin_borrow %0 : $Wrapper
976976
%3 = struct_extract %2 : $Wrapper, #Wrapper.c
977977
%4 = mark_dependence [nonescaping] %1 : $*Klass on %3 : $Klass
978-
end_borrow %2 : $Wrapper
978+
unreachable
979+
}
980+
981+
// CHECK-LABEL: begin running test {{.*}} on testOwnedMarkDepNonEscAddressValue: ossa_lifetime_completion
982+
// CHECK-LABEL: sil [ossa] @testOwnedMarkDepNonEscAddressValue : {{.*}} {
983+
// CHECK: end_borrow
984+
// CHECK: mark_dependence [nonescaping]
985+
// CHECK: destroy_value [dead_end] %0
986+
// CHECK-LABEL: } // end sil function 'testOwnedMarkDepNonEscAddressValue'
987+
// CHECK-LABEL: end running test {{.*}} on testOwnedMarkDepNonEscAddressValue: ossa_lifetime_completion
988+
sil [ossa] @testOwnedMarkDepNonEscAddressValue : $@convention(thin) (@owned Wrapper, @inout Klass) -> () {
989+
bb0(%0 : @owned $Wrapper, %1 : $*Klass):
990+
specify_test "ossa_lifetime_completion %0 liveness"
991+
%2 = begin_borrow %0 : $Wrapper
992+
%3 = struct_extract %2 : $Wrapper, #Wrapper.c
993+
%4 = mark_dependence [nonescaping] %1 : $*Klass on %0 : $Wrapper
979994
unreachable
980995
}

0 commit comments

Comments
 (0)