Merge pull request #79177 from atrick/fix-markdep-ownership

atrick · web-flow · commit a54a23a1e685 · 2025-02-06T01:18:14.000-08:00
Fix mark_dependence [nonescaping] ownership
diff --git a/SwiftCompilerSources/Sources/Optimizer/Utilities/OwnershipLiveness.swift b/SwiftCompilerSources/Sources/Optimizer/Utilities/OwnershipLiveness.swift
@@ -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");
     }
   }
@@ -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:
diff --git a/SwiftCompilerSources/Sources/SIL/Operand.swift b/SwiftCompilerSources/Sources/SIL/Operand.swift
@@ -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
   
@@ -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
@@ -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:
@@ -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
diff --git a/include/swift/SIL/OwnershipUseVisitor.h b/include/swift/SIL/OwnershipUseVisitor.h
@@ -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.
@@ -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:
@@ -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:
@@ -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.
diff --git a/include/swift/SIL/SILBridging.h b/include/swift/SIL/SILBridging.h
@@ -373,6 +373,7 @@ struct BridgedOperand {
     DestroyingConsume,
     ForwardingConsume,
     InteriorPointer,
+    AnyInteriorPointer,
     GuaranteedForwarding,
     EndBorrow,
     Reborrow
diff --git a/include/swift/SIL/SILBridgingImpl.h b/include/swift/SIL/SILBridgingImpl.h
@@ -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:
diff --git a/include/swift/SIL/SILValue.h b/include/swift/SIL/SILValue.h
@@ -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)
@@ -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,
@@ -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:
diff --git a/lib/SIL/IR/OperandOwnership.cpp b/lib/SIL/IR/OperandOwnership.cpp
@@ -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
diff --git a/lib/SIL/IR/SILInstruction.cpp b/lib/SIL/IR/SILInstruction.cpp
@@ -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)) {
diff --git a/lib/SIL/IR/SILValue.cpp b/lib/SIL/IR/SILValue.cpp
@@ -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:
diff --git a/lib/SIL/Utils/OwnershipUtils.cpp b/lib/SIL/Utils/OwnershipUtils.cpp
@@ -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;
@@ -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
@@ -387,6 +389,7 @@ bool swift::findExtendedUsesOfSimpleBorrowedValue(
       break;
 
     case OperandOwnership::InteriorPointer:
+    case OperandOwnership::AnyInteriorPointer:
       if (InteriorPointerOperandKind::get(use) ==
           InteriorPointerOperandKind::Invalid)
         return false;
@@ -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: {
diff --git a/lib/SIL/Utils/PrunedLiveness.cpp b/lib/SIL/Utils/PrunedLiveness.cpp
@@ -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) {
@@ -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: {
diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp
@@ -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");
+        }
       }
     }
 
diff --git a/lib/SILOptimizer/Mandatory/ConsumeOperatorCopyableValuesChecker.cpp b/lib/SILOptimizer/Mandatory/ConsumeOperatorCopyableValuesChecker.cpp
@@ -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.
         //
diff --git a/lib/SILOptimizer/Mandatory/DiagnoseLifetimeIssues.cpp b/lib/SILOptimizer/Mandatory/DiagnoseLifetimeIssues.cpp
@@ -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.
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
@@ -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.
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp
@@ -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.
@@ -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:
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp
@@ -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;
diff --git a/lib/SILOptimizer/Utils/CanonicalizeBorrowScope.cpp b/lib/SILOptimizer/Utils/CanonicalizeBorrowScope.cpp
@@ -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.
diff --git a/lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp b/lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp
@@ -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
diff --git a/test/SILOptimizer/ossa_lifetime_completion.sil b/test/SILOptimizer/ossa_lifetime_completion.sil
@@ -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
@@ -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
 }

Original file line number	Diff line number	Diff line change
`@@ -678,7 +678,7 @@ OperandOwnershipClassifier::visitMarkDependenceInst(MarkDependenceInst *mdi) {`
`678`	`678`	`// which we treat like a borrow.`
`679`	`679`	`return OperandOwnership::Borrow;`
`680`	`680`	`}`
`681`		`- return OperandOwnership::InteriorPointer;`
	`681`	`+ return OperandOwnership::AnyInteriorPointer;`
`682`	`682`	`}`
`683`	`683`	`if (mdi->hasUnresolvedEscape()) {`
`684`	`684`	`// This creates a dependent value that may extend beyond the parent's`