Merge pull request #72407 from atrick/pointer-escape-bailout

Fix lifetime dependence in the presence of pointer escapes.

Author: atrick

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift

@@ -85,6 +85,8 @@ private func extendAccessScopes(dependence: LifetimeDependence,
 
   _ = walker.walkDown(root: dependence.dependentValue)
 
+  log("Scope fixup for dependent uses:\n\(accessRange)")
+
   // Lifetime dependenent uses may not be dominated by the access. The dependent value may be used by a phi or stored
   // into a memory location. The access may be conditional relative to such uses. If any use was not dominated, then
   // `accessRange` will include the function entry.
@@ -219,6 +221,7 @@ private struct LifetimeDependenceScopeFixupWalker : LifetimeDependenceDefUseWalk
   }
 
   mutating func escapingDependence(on operand: Operand) -> WalkResult {
+    log(">>> Escaping dependence: \(operand)")
     _ = visitor(operand)
     // Make a best-effort attempt to extend the access scope regardless of escapes. It is possible that some mandatory
     // pass between scope fixup and diagnostics will make it possible for the LifetimeDependenceDefUseWalker to analyze

SwiftCompilerSources/Sources/Optimizer/Utilities/AddressUtils.swift

@@ -12,7 +12,7 @@
 
 import SIL
 
-private let verbose = true
+private let verbose = false
 
 private func log(_ message: @autoclosure () -> String) {
   if verbose {
@@ -97,20 +97,26 @@ extension AddressUseVisitor {
       if markDep.type.isAddress {
         return projectedAddressUse(of: operand, into: markDep)
       }
-      if LifetimeDependence(markDep, context) != nil {
-        // This is unreachable from InteriorUseVisitor because the
-        // base address of a `mark_dependence [nonescaping]` must be a
-        // `begin_access`, and interior liveness does not check uses of
-        // the accessed address.
+      switch markDep.dependenceKind {
+      case .Unresolved:
+        if LifetimeDependence(markDep, context) == nil {
+          break
+        }
+        fallthrough
+      case .NonEscaping:
+        // Note: This is unreachable from InteriorUseVisitor because the base address of a `mark_dependence
+        // [nonescaping]` must be a `begin_access`, and interior liveness does not check uses of the accessed address.
         return dependentAddressUse(of: operand, into: markDep)
+      case .Escaping:
+        break
       }
       // A potentially escaping value depends on this address.
       return escapingAddressUse(of: operand)
 
-    case let pai as PartialApplyInst where pai.isOnStack:
+    case let pai as PartialApplyInst where !pai.mayEscape:
       return dependentAddressUse(of: operand, into: pai)
 
-    case let pai as PartialApplyInst where !pai.isOnStack:
+    case let pai as PartialApplyInst where pai.mayEscape:
       return escapingAddressUse(of: operand)
 
     case is ReturnInst, is ThrowInst, is YieldInst, is AddressToPointerInst:
@@ -502,7 +508,7 @@ extension AddressOwnershipLiveRange {
   ///
   /// For address values, use AccessBase.computeOwnershipRange.
   ///
-  /// FIXME: This should use computeLinearLiveness rather than computeInteriorLiveness as soon as lifetime completion
+  /// FIXME: This should use computeLinearLiveness rather than computeKnownLiveness as soon as lifetime completion
   /// runs immediately after SILGen.
   private static func computeValueLiveRange(of value: Value, _ context: FunctionPassContext)
     -> AddressOwnershipLiveRange? {
@@ -511,7 +517,7 @@ extension AddressOwnershipLiveRange {
       // This is unexpected for a value with derived addresses.
       return nil
     case .owned:
-      return .owned(value, computeInteriorLiveness(for: value, context))
+      return .owned(value, computeKnownLiveness(for: value, context))
     case .guaranteed:
       return .borrow(computeBorrowLiveRange(for: value, context))
     }

SwiftCompilerSources/Sources/Optimizer/Utilities/BorrowUtils.swift

@@ -169,7 +169,7 @@ enum BorrowingInstruction : CustomStringConvertible, Hashable {
       self = .storeBorrow(sbi)
     case let bai as BeginApplyInst:
       self = .beginApply(bai)
-    case let pai as PartialApplyInst where pai.isOnStack:
+    case let pai as PartialApplyInst where !pai.mayEscape:
       self = .partialApply(pai)
     case let mdi as MarkDependenceInst:
       self = .markDependence(mdi)
@@ -405,8 +405,9 @@ func gatherBorrowIntroducers(for value: Value,
 }
 
 /// Compute the live range for the borrow scopes of a guaranteed value. This returns a separate instruction range for
-/// each of the value's borrow introducers. Unioning those ranges would be incorrect. We typically want their
-/// intersection.
+/// each of the value's borrow introducers.
+///
+/// TODO: This should return a single multiply-defined instruction range.
 func computeBorrowLiveRange(for value: Value, _ context: FunctionPassContext)
   -> SingleInlineArray<(BeginBorrowValue, InstructionRange)> {
   assert(value.ownership == .guaranteed)
@@ -418,10 +419,10 @@ func computeBorrowLiveRange(for value: Value, _ context: FunctionPassContext)
   // If introducers is empty, then the dependence is on a trivial value, so
   // there is no ownership range.
   while let beginBorrow = introducers.pop() {
-    /// FIXME: Remove calls to computeInteriorLiveness as soon as lifetime completion runs immediately after
+    /// FIXME: Remove calls to computeKnownLiveness() as soon as lifetime completion runs immediately after
     /// SILGen. Instead, this should compute linear liveness for borrowed value by switching over BeginBorrowValue, just
     /// like LifetimeDependenc.Scope.computeRange().
-    ranges.push((beginBorrow, computeInteriorLiveness(for: beginBorrow.value, context)))
+    ranges.push((beginBorrow, computeKnownLiveness(for: beginBorrow.value, context)))
   }
   return ranges
 }

SwiftCompilerSources/Sources/Optimizer/Utilities/ForwardingUtils.swift

@@ -18,6 +18,14 @@
 
 import SIL
 
+private let verbose = false
+
+private func log(_ message: @autoclosure () -> String) {
+  if verbose {
+    print("### \(message())")
+  }
+}
+
 /// Return true if any use in `value`s forward-extend lifetime has
 /// .pointerEscape operand ownership.
 ///
@@ -355,7 +363,7 @@ struct NonEscapingClosureDefUseWalker {
   }
 
   mutating func walkDown(closure: PartialApplyInst) -> WalkResult {
-    assert(closure.isOnStack)
+    assert(!closure.mayEscape)
     return walkDownUses(of: closure, using: nil)
   }
 
@@ -370,6 +378,7 @@ struct NonEscapingClosureDefUseWalker {
       if operand.instruction.isIncidentalUse {
         return .continueWalk
       }
+      log(">>> Unexpected closure use \(operand)")
       // Escaping or unexpected closure use. Expected escaping uses include ReturnInst with a lifetime-dependent result.
       //
       // TODO: Check in the SIL verifier that all uses are expected.
@@ -385,11 +394,15 @@ extension NonEscapingClosureDefUseWalker: ForwardingDefUseWalker {
 
   mutating func nonForwardingUse(of operand: Operand) -> WalkResult {
     // Nonescaping closures may be moved, copied, or borrowed.
-    if let transition = operand.instruction as? OwnershipTransitionInstruction {
+    switch operand.instruction {
+    case let transition as OwnershipTransitionInstruction:
       return walkDownUses(of: transition.ownershipResult, using: operand)
+    case let convert as ConvertEscapeToNoEscapeInst:
+      return walkDownUses(of: convert, using: operand)
+    default:
+      // Otherwise, assume the use cannot propagate the closure context.
+      return closureContextLeafUse(of: operand)
     }
-    // Otherwise, assume the use cannot propagate the closure context.
-    return closureContextLeafUse(of: operand)
   }
 
   mutating func deadValue(_ value: Value, using operand: Operand?) -> WalkResult {

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

@@ -54,7 +54,7 @@
 
 import SIL
 
-private let verbose = true
+private let verbose = false
 
 private func log(_ message: @autoclosure () -> String) {
   if verbose {
@@ -530,13 +530,12 @@ extension LifetimeDependence {
       visitedValues.insert(value)
     }
 
-    // Visit the base value of a lifetime dependence. If the base is
-    // an address, the dependence scope is the enclosing access. The
-    // walker does not walk past an `mark_dependence [nonescaping]`
-    // that produces an address, because that will never occur inside
-    // of an access scope. An address type mark_dependence
-    // [nonescaping]` can only result from an indirect function result
-    // when opaque values are not enabled.
+    // Visit the base value of a lifetime dependence. If the base is an address, the dependence scope is the enclosing
+    // access. The walker does not walk past an `mark_dependence [nonescaping]` that produces an address, because that
+    // will never occur inside of an access scope. An address type mark_dependence [unresolved]` can only result from an
+    // indirect function result when opaque values are not enabled. Address type `mark_dependence [nonescaping]`
+    // instruction are also produced for captured arguments but ClosureLifetimeFixup, but those aren't considered to
+    // have a LifetimeDependence scope.
     mutating func introducer(_ value: Value, _ owner: Value?) -> WalkResult {
       let base = owner ?? value
       guard let scope = LifetimeDependence.Scope(base: base, context)
@@ -617,32 +616,24 @@ struct VariableIntroducerUseDefWalker : LifetimeDependenceUseDefWalker {
 
 /// Walk up the lifetime dependence chain.
 ///
-/// This finds the introducers of a dependence chain. which represent
-/// the value's "inherited" dependencies. This stops at phis; all
-/// introducers dominate. This stops at addresses in general, but if
-/// the value is loaded from a singly-initialized location, then it
-/// continues walking up the value stored by the initializer. This
+/// This finds the introducers of a dependence chain. which represent the value's "inherited" dependencies. This stops
+/// at phis, so all introducers dominate their dependencies. This stops at addresses in general, but if the value is
+/// loaded from a singly-initialized location, then it continues walking up the value stored by the initializer. This
 /// bypasses the copies to temporary memory locations emitted by SILGen.
 ///
-/// In this example, the dependence root is
-/// copied, borrowed, and forwarded before being used as the base
-/// operand of `mark_dependence`. The dependence "root" is the parent
-/// of the outer-most dependence scope.
+/// In this example, the dependence root is copied, borrowed, and forwarded before being used as the base operand of
+/// `mark_dependence`. The dependence "root" is the parent of the outer-most dependence scope.
 ///
 ///   %root = apply                  // lifetime dependence root
 ///   %copy = copy_value %root
 ///   %parent = begin_borrow %copy   // lifetime dependence parent value
 ///   %base = struct_extract %parent // lifetime dependence base value
 ///   %dependent = mark_dependence [nonescaping] %value on %base
 ///
-/// This extends the ForwardingUseDefWalker, which finds the
-/// forward-extended lifetime introducers. Certain forward-extended
-/// lifetime introducers can inherit a lifetime dependency from their
-/// operand: namely copies, moves, and borrows. These introducers are
-/// considered part of their operand's dependence scope because
-/// non-escapable values can be copied, moved, and
-/// borrowed. Nonetheless, all of their uses must remain within
-/// original dependence scope.
+/// This extends the ForwardingUseDefWalker, which finds the forward-extended lifetime introducers. Certain
+/// forward-extended lifetime introducers can inherit a lifetime dependency from their operand: namely copies, moves,
+/// and borrows. These introducers are considered part of their operand's dependence scope because non-escapable values
+/// can be copied, moved, and borrowed. Nonetheless, all of their uses must remain within original dependence scope.
 ///
 ///   # owned lifetime dependence
 ///   %parent = apply               // begin dependence scope -+
@@ -662,10 +653,9 @@ struct VariableIntroducerUseDefWalker : LifetimeDependenceUseDefWalker {
 ///   ...                                                      |
 ///   destroy_value %parent        // end dependence scope    -+
 ///
-/// All of the dependent uses including `end_borrow %5` and
-/// `destroy_value %4` must be before the end of the dependence scope:
-/// `destroy_value %parent`. In this case, the dependence parent is an
-/// owned value, so the scope is simply the value's OSSA lifetime.
+/// All of the dependent uses including `end_borrow %5` and `destroy_value %4` must be before the end of the dependence
+/// scope: `destroy_value %parent`. In this case, the dependence parent is an owned value, so the scope is simply the
+/// value's OSSA lifetime.
 ///
 /// Minimal requirements:
 ///   var context: Context
@@ -740,6 +730,7 @@ extension LifetimeDependenceUseDefWalker {
     if Phi(value) != nil {
       return introducer(value, owner)
     }
+    // ForwardingUseDefWalker will callback to introducer() when it finds no forwarding instruction.
     return walkUpDefault(forwarded: value, owner)
   }
 
@@ -792,6 +783,8 @@ extension LifetimeDependenceUseDefWalker {
 ///   yieldedDependence(result: Operand) -> WalkResult
 /// Start walking:
 ///   walkDown(root: Value)
+///
+/// Note: this may visit values that are not dominated by `root` because of dependent phi operands.
 protocol LifetimeDependenceDefUseWalker : ForwardingDefUseWalker,
                                           OwnershipUseVisitor,
                                           AddressUseVisitor {

SwiftCompilerSources/Sources/Optimizer/Utilities/LocalVariableUtils.swift

@@ -25,7 +25,7 @@
 
 import SIL
 
-private let verbose = true
+private let verbose = false
 
 private func log(_ message: @autoclosure () -> String) {
   if verbose {
@@ -188,8 +188,8 @@ class LocalVariableAccessInfo: CustomStringConvertible {
   }
 
   var description: String {
-    return "\(access)" +
-      "\n    fully-assigned: \(_isFullyAssigned == nil ? "unknown" : String(describing: _isFullyAssigned!))"
+    return "full-assign: \(_isFullyAssigned == nil ? "unknown" : String(describing: _isFullyAssigned!)) "
+      + "\(access)"
   }
 }
 
@@ -200,7 +200,7 @@ class LocalVariableAccessInfo: CustomStringConvertible {
 /// map.
 ///
 /// TODO: In addition to isFullyAssigned, consider adding a lazily computed access path if any need arises.
-struct LocalVariableAccessMap: Collection, FormattedLikeArray {
+struct LocalVariableAccessMap: Collection, CustomStringConvertible {
   let context: Context
   let allocation: Value
 
@@ -279,6 +279,10 @@ struct LocalVariableAccessMap: Collection, FormattedLikeArray {
   subscript(_ accessIndex: Int) -> LocalVariableAccessInfo { accessList[accessIndex] }
 
   subscript(instruction: Instruction) -> LocalVariableAccessInfo? { accessMap[instruction] }
+
+  public var description: String {
+    "Access map:\n" + map({String(describing: $0)}).joined(separator: "\n")
+  }
 }
 
 /// Gather the accesses of a local allocation: alloc_box, alloc_stack, @in, @inout.
@@ -416,7 +420,7 @@ extension LocalVariableAccessWalker: AddressUseVisitor {
 
   mutating func dependentAddressUse(of operand: Operand, into value: Value) -> WalkResult {
     // Find all uses of partial_apply [on_stack].
-    if let pai = value as? PartialApplyInst, pai.isOnStack {
+    if let pai = value as? PartialApplyInst, !pai.mayEscape {
       var walker = NonEscapingClosureDefUseWalker(context)
       defer { walker.deinitialize() }
       if walker.walkDown(closure: pai) == .abortWalk {
@@ -441,7 +445,8 @@ extension LocalVariableAccessWalker: AddressUseVisitor {
   }
 
   mutating func escapingAddressUse(of operand: Operand) -> WalkResult {
-    return .abortWalk
+    visit(LocalVariableAccess(.escape, operand))
+    return .continueWalk
   }
 
   mutating func unknownAddressUse(of operand: Operand) -> WalkResult {
@@ -732,6 +737,8 @@ extension LocalVariableReachableAccess {
       forwardPropagateEffect(in: block, blockInfo: blockInfo, effect: currentEffect, blockList: &blockList,
                              accessStack: &accessStack)
     }
+    log("\(accessMap)")
+    log("Reachable access:\n\(accessStack.map({ String(describing: $0)}).joined(separator: "\n"))")
     return true
   }