Skip to content

Fix mark_dependence [nonescaping] ownership #79177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,11 @@ extension OwnershipUseVisitor {
case .borrow:
return visitBorrowingUse(of: operand)

// TODO: Eventually, visit owned InteriorPointers as implicit borrows.
case .interiorPointer, .trivialUse, .endBorrow, .reborrow,
.guaranteedForwarding:
case .anyInteriorPointer:
return visitInteriorPointerUse(of: operand)

// TODO: .interiorPointer should instead be handled like .anyInteriorPointer.
case .interiorPointer, .trivialUse, .endBorrow, .reborrow, .guaranteedForwarding:
fatalError("ownership incompatible with an owned value");
}
}
Expand Down Expand Up @@ -449,7 +451,7 @@ extension OwnershipUseVisitor {
case .borrow:
return visitBorrowingUse(of: operand)

case .interiorPointer:
case .interiorPointer, .anyInteriorPointer:
return visitInteriorPointerUse(of: operand)

case .trivialUse, .forwardingConsume, .destroyingConsume:
Expand Down
11 changes: 9 additions & 2 deletions SwiftCompilerSources/Sources/SIL/Operand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,11 @@ public enum OperandOwnership {
/// 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
/// value are in scope. (ref_element_addr, open_existential_box)
case interiorPointer


/// Any Interior Pointer. An interior pointer that allows any operand ownership. This will be removed as soon as SIL
/// migrates away from extraneous borrow scopes.
case anyInteriorPointer

/// Forwarded Borrow. Propagates the guaranteed value within the base's borrow scope. (tuple_extract, struct_extract, cast, switch)
case guaranteedForwarding

Expand All @@ -282,7 +286,7 @@ public enum OperandOwnership {
switch self {
case .nonUse, .trivialUse, .instantaneousUse, .unownedInstantaneousUse,
.forwardingUnowned, .pointerEscape, .bitwiseEscape, .borrow,
.interiorPointer, .guaranteedForwarding:
.interiorPointer, .anyInteriorPointer, .guaranteedForwarding:
return false
case .destroyingConsume, .forwardingConsume, .endBorrow, .reborrow:
return true
Expand Down Expand Up @@ -313,6 +317,8 @@ public enum OperandOwnership {
return BridgedOperand.OperandOwnership.ForwardingConsume
case .interiorPointer:
return BridgedOperand.OperandOwnership.InteriorPointer
case .anyInteriorPointer:
return BridgedOperand.OperandOwnership.AnyInteriorPointer
case .guaranteedForwarding:
return BridgedOperand.OperandOwnership.GuaranteedForwarding
case .endBorrow:
Expand All @@ -337,6 +343,7 @@ extension Operand {
case .DestroyingConsume: return .destroyingConsume
case .ForwardingConsume: return .forwardingConsume
case .InteriorPointer: return .interiorPointer
case .AnyInteriorPointer: return .anyInteriorPointer
case .GuaranteedForwarding: return .guaranteedForwarding
case .EndBorrow: return .endBorrow
case .Reborrow: return .reborrow
Expand Down
11 changes: 8 additions & 3 deletions include/swift/SIL/OwnershipUseVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,9 @@ bool OwnershipUseVisitor<Impl>::visitOwnedUse(Operand *use) {
}
return handleUsePoint(use, UseLifetimeConstraint::LifetimeEnding);

case OperandOwnership::AnyInteriorPointer:
return visitInteriorPointerUses(use);

case OperandOwnership::PointerEscape:
// TODO: Change ProjectBox ownership to InteriorPointer and allow them to
// take owned values.
Expand All @@ -416,7 +419,7 @@ bool OwnershipUseVisitor<Impl>::visitOwnedUse(Operand *use) {
case OperandOwnership::Borrow:
return visitInnerBorrow(use);

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

case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
return visitInteriorPointerUses(use);

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

template <typename Impl>
bool OwnershipUseVisitor<Impl>::visitInteriorPointerUses(Operand *use) {
assert(use->getOperandOwnership() == OperandOwnership::InteriorPointer ||
isa<ProjectBoxInst>(use->getUser()));
assert(use->getOperandOwnership() == OperandOwnership::InteriorPointer
|| use->getOperandOwnership() == OperandOwnership::AnyInteriorPointer
|| isa<ProjectBoxInst>(use->getUser()));

if (auto scopedAddress = ScopedAddressValue::forUse(use)) {
// e.g. client may need to insert end_borrow if scopedAddress is a store_borrow.
Expand Down
1 change: 1 addition & 0 deletions include/swift/SIL/SILBridging.h
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ struct BridgedOperand {
DestroyingConsume,
ForwardingConsume,
InteriorPointer,
AnyInteriorPointer,
GuaranteedForwarding,
EndBorrow,
Reborrow
Expand Down
2 changes: 2 additions & 0 deletions include/swift/SIL/SILBridgingImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,8 @@ BridgedOperand::OperandOwnership BridgedOperand::getOperandOwnership() const {
return OperandOwnership::ForwardingConsume;
case swift::OperandOwnership::InteriorPointer:
return OperandOwnership::InteriorPointer;
case swift::OperandOwnership::AnyInteriorPointer:
return OperandOwnership::AnyInteriorPointer;
case swift::OperandOwnership::GuaranteedForwarding:
return OperandOwnership::GuaranteedForwarding;
case swift::OperandOwnership::EndBorrow:
Expand Down
11 changes: 11 additions & 0 deletions include/swift/SIL/SILValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,13 @@ struct OperandOwnership {
/// value are in scope.
/// (ref_element_addr, open_existential_box)
InteriorPointer,

// TODO: Remove AnyInteriorPointer after fixing
// OperandOwnership::getOwnershipConstraint() to allow InteriorPointer
// operands to take any operand ownership. This will prevent useless borrow
// scopes from being generated, so it will require some SIL migration. But
// all OSSA utilities need to correctly handle interior uses anyway.
AnyInteriorPointer,
/// Forwarded Borrow. Propagates the guaranteed value within the base's
/// borrow scope.
/// (tuple_extract, struct_extract, cast, switch)
Expand Down Expand Up @@ -942,6 +949,9 @@ inline OwnershipConstraint OperandOwnership::getOwnershipConstraint() {
case OperandOwnership::DestroyingConsume:
case OperandOwnership::ForwardingConsume:
return {OwnershipKind::Owned, UseLifetimeConstraint::LifetimeEnding};
case OperandOwnership::AnyInteriorPointer:
return {OwnershipKind::Any, UseLifetimeConstraint::NonLifetimeEnding};
// TODO: InteriorPointer should be handled like AnyInteriorPointer.
case OperandOwnership::InteriorPointer:
case OperandOwnership::GuaranteedForwarding:
return {OwnershipKind::Guaranteed,
Expand Down Expand Up @@ -971,6 +981,7 @@ inline bool canAcceptUnownedValue(OperandOwnership operandOwnership) {
case OperandOwnership::DestroyingConsume:
case OperandOwnership::ForwardingConsume:
case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
case OperandOwnership::GuaranteedForwarding:
case OperandOwnership::EndBorrow:
case OperandOwnership::Reborrow:
Expand Down
2 changes: 1 addition & 1 deletion lib/SIL/IR/OperandOwnership.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ OperandOwnershipClassifier::visitMarkDependenceInst(MarkDependenceInst *mdi) {
// which we treat like a borrow.
return OperandOwnership::Borrow;
}
return OperandOwnership::InteriorPointer;
return OperandOwnership::AnyInteriorPointer;
}
if (mdi->hasUnresolvedEscape()) {
// This creates a dependent value that may extend beyond the parent's
Expand Down
1 change: 1 addition & 0 deletions lib/SIL/IR/SILInstruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2009,6 +2009,7 @@ bool MarkDependenceInst::visitNonEscapingLifetimeEnds(
llvm::function_ref<bool (Operand *)> visitUnknownUse) const {
assert(getFunction()->hasOwnership() && isNonEscaping()
&& "only meaningful for nonescaping dependencies");
assert(getType().isObject() && "lifetime ends only exist for values");
bool noUsers = true;
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, visitScopeEnd,
visitUnknownUse)) {
Expand Down
2 changes: 2 additions & 0 deletions lib/SIL/IR/SILValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,8 @@ StringRef OperandOwnership::asString() const {
return "forwarding-consume";
case OperandOwnership::InteriorPointer:
return "interior-pointer";
case OperandOwnership::AnyInteriorPointer:
return "any-interior-pointer";
case OperandOwnership::GuaranteedForwarding:
return "guaranteed-forwarding";
case OperandOwnership::EndBorrow:
Expand Down
6 changes: 5 additions & 1 deletion lib/SIL/Utils/OwnershipUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ bool swift::findPointerEscape(SILValue original) {
});
break;
}
case OperandOwnership::InteriorPointer: {
case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer: {
if (InteriorPointerOperand(use).findTransitiveUses() !=
AddressUseKind::NonEscaping) {
return true;
Expand Down Expand Up @@ -247,6 +248,7 @@ bool swift::findInnerTransitiveGuaranteedUses(
break;

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

case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
if (InteriorPointerOperandKind::get(use) ==
InteriorPointerOperandKind::Invalid)
return false;
Expand Down Expand Up @@ -712,6 +715,7 @@ bool BorrowingOperand::visitScopeEndingUses(
case BorrowingOperandKind::MarkDependenceNonEscaping: {
auto *user = cast<MarkDependenceInst>(op->getUser());
assert(user->isNonEscaping() && "escaping dependencies don't borrow");
assert(user->getType().isObject() && "borrows only exist for values");
return user->visitNonEscapingLifetimeEnds(visitScopeEnd, visitUnknownUse);
}
case BorrowingOperandKind::BeginAsyncLet: {
Expand Down
4 changes: 3 additions & 1 deletion lib/SIL/Utils/PrunedLiveness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,8 @@ PrunedLiveRange<LivenessWithDefs>::updateForBorrowingOperand(Operand *operand) {
template <typename LivenessWithDefs>
AddressUseKind PrunedLiveRange<LivenessWithDefs>::checkAndUpdateInteriorPointer(
Operand *operand) {
assert(operand->getOperandOwnership() == OperandOwnership::InteriorPointer);
assert(operand->getOperandOwnership() == OperandOwnership::InteriorPointer
|| operand->getOperandOwnership() == OperandOwnership::AnyInteriorPointer);

if (auto scopedAddress = ScopedAddressValue::forUse(operand)) {
scopedAddress.visitScopeEndingUses([this](Operand *end) {
Expand Down Expand Up @@ -364,6 +365,7 @@ LiveRangeSummary PrunedLiveRange<LivenessWithDefs>::recursivelyUpdateForDef(
summary.meet(AddressUseKind::PointerEscape);
break;
case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
summary.meet(checkAndUpdateInteriorPointer(use));
break;
case OperandOwnership::GuaranteedForwarding: {
Expand Down
16 changes: 10 additions & 6 deletions lib/SIL/Verifier/SILVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1458,12 +1458,16 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
"kind since guaranteed and owned values can always be passed "
"in unowned positions");

require(operand.getOperandOwnership() !=
OperandOwnership::InteriorPointer ||
InteriorPointerOperandKind::get(&operand) !=
InteriorPointerOperandKind::Invalid,
"All operands with InteriorPointer operand ownership should be "
"added to the InteriorPointerOperand utility");
switch (operand.getOperandOwnership()) {
default:
break;
case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
require(InteriorPointerOperandKind::get(&operand) !=
InteriorPointerOperandKind::Invalid,
"All operands with InteriorPointer operand ownership should be "
"added to the InteriorPointerOperand utility");
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ bool CheckerLivenessInfo::compute() {
return true;
});
break;
case OperandOwnership::InteriorPointer: {
case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer: {
// An interior pointer user extends liveness until the end of the
// interior pointer section.
//
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/Mandatory/DiagnoseLifetimeIssues.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ visitUses(SILValue def, bool updateLivenessAndWeakStores, int callDepth) {
continue;

case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
// Treat most interior pointers as escapes until they can be audited.
// But if the interior pointer cannot be used to copy the parent
// reference, then it does not need to be considered an escape.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,7 @@ struct CopiedLoadBorrowEliminationVisitor
case OperandOwnership::InstantaneousUse:
case OperandOwnership::UnownedInstantaneousUse:
case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
case OperandOwnership::BitwiseEscape: {
// Look through copy_value of a move only value. We treat copy_value of
// copyable values as normal uses.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ bool Implementation::gatherUses(SILValue value) {
case OperandOwnership::InstantaneousUse:
case OperandOwnership::UnownedInstantaneousUse:
case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
case OperandOwnership::BitwiseEscape: {
// Look through copy_value of a move only value. We treat copy_value of
// copyable values as normal uses.
Expand Down Expand Up @@ -1587,6 +1588,7 @@ static bool gatherBorrows(SILValue rootValue,
}
continue;
case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
// We don't care about these.
continue;
case OperandOwnership::GuaranteedForwarding:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ bool MoveOnlyObjectCheckerPImpl::checkForSameInstMultipleUseErrors(
instToOperandsMap.insert(nextUse->getUser(), nextUse);
continue;
case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
// We do not care about interior pointer uses since there aren't any
// interior pointer using instructions that are also consuming uses.
continue;
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/Utils/CanonicalizeBorrowScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ bool CanonicalizeBorrowScope::visitBorrowScopeUses(SILValue innerValue,
llvm_unreachable("this operand cannot handle ownership");

case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
case OperandOwnership::EndBorrow:
case OperandOwnership::Reborrow:
// Ignore uses that must be within the borrow scope.
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
}
break;
case OperandOwnership::InteriorPointer:
case OperandOwnership::AnyInteriorPointer:
case OperandOwnership::GuaranteedForwarding:
case OperandOwnership::EndBorrow:
// Guaranteed values are exposed by inner adjacent reborrows. If user is
Expand Down
19 changes: 17 additions & 2 deletions test/SILOptimizer/ossa_lifetime_completion.sil
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ struct Wrapper {

// CHECK-LABEL: begin running test {{.*}} on testInteriorMarkDepNonEscAddressValue: ossa_lifetime_completion
// CHECK-LABEL: sil [ossa] @testInteriorMarkDepNonEscAddressValue : {{.*}} {
// CHECK: mark_dependence
// CHECK: mark_dependence [nonescaping]
// CHECK: end_borrow
// CHECK-LABEL: } // end sil function 'testInteriorMarkDepNonEscAddressValue'
// CHECK-LABEL: end running test {{.*}} on testInteriorMarkDepNonEscAddressValue: ossa_lifetime_completion
Expand All @@ -975,6 +975,21 @@ bb0(%0 : @owned $Wrapper, %1 : $*Klass):
%2 = begin_borrow %0 : $Wrapper
%3 = struct_extract %2 : $Wrapper, #Wrapper.c
%4 = mark_dependence [nonescaping] %1 : $*Klass on %3 : $Klass
end_borrow %2 : $Wrapper
unreachable
}

// CHECK-LABEL: begin running test {{.*}} on testOwnedMarkDepNonEscAddressValue: ossa_lifetime_completion
// CHECK-LABEL: sil [ossa] @testOwnedMarkDepNonEscAddressValue : {{.*}} {
// CHECK: end_borrow
// CHECK: mark_dependence [nonescaping]
// CHECK: destroy_value [dead_end] %0
// CHECK-LABEL: } // end sil function 'testOwnedMarkDepNonEscAddressValue'
// CHECK-LABEL: end running test {{.*}} on testOwnedMarkDepNonEscAddressValue: ossa_lifetime_completion
sil [ossa] @testOwnedMarkDepNonEscAddressValue : $@convention(thin) (@owned Wrapper, @inout Klass) -> () {
bb0(%0 : @owned $Wrapper, %1 : $*Klass):
specify_test "ossa_lifetime_completion %0 liveness"
%2 = begin_borrow %0 : $Wrapper
%3 = struct_extract %2 : $Wrapper, #Wrapper.c
%4 = mark_dependence [nonescaping] %1 : $*Klass on %0 : $Wrapper
unreachable
}