Merge pull request swiftlang#78716 from atrick/fix-lifedep-out

Fix LifetimeDependenceDiagnostics for @out dependencies

Author: atrick

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

@@ -880,6 +880,17 @@ extension LifetimeDependenceDefUseWalker {
 extension LifetimeDependenceDefUseWalker {
   mutating func walkDown(root: Value) -> WalkResult {
     if root.type.isAddress {
+      if let md = root as? MarkDependenceInst, !root.isEscapable {
+        // LifetimeDependence.dependentValue is typically a mark_dependence. If its 'value' address is a non-Escapable
+        // local variable, then consider all other reachable uses of that local variable to be dependent uses. Remember
+        // the operand to the mark_dependence as if it was a store. Diagnostics will consider this the point of variable
+        // initialization.
+        if visitStoredUses(of: md.valueOperand, into: md.value) == .abortWalk {
+          return .abortWalk
+        }
+      }
+      // The root address may also be an escapable mark_dependence that guards its address uses (unsafeAddress), or an
+      // allocation or incoming argument. In all these cases, it is sufficient to walk down the address uses.
       return walkDownAddressUses(of: root)
     }
     return walkDownUses(of: root, using: nil)
@@ -1127,11 +1138,13 @@ extension LifetimeDependenceDefUseWalker {
     }
   }
 
-  // Visit stores to a local variable (alloc_box), temporary storage
-  // (alloc_stack). This handles stores of the entire value and stores
-  // to a tuple element. Stores to a field within another nominal
-  // value are considered lifetime dependence leaf uses; the type
-  // system enforces non-escapability on the aggregate value.
+  // Visit stores to a local variable (alloc_box), temporary storage (alloc_stack). This handles stores of the entire
+  // value and stores to a tuple element. Stores to a field within another nominal value are considered lifetime
+  // dependence leaf uses; the type system enforces non-escapability on the aggregate value.
+  //
+  // If 'operand' is an address, then the "store" corresponds to initialization via an @out argument. The initial
+  // call to visitStoredUses will have 'operand == address' where the "stored value" is the temporary stack
+  // allocation for the @out parameter.
   private mutating func visitStoredUses(of operand: Operand,
                                         into address: Value) -> WalkResult {
     assert(address.type.isAddress)
@@ -1207,6 +1220,11 @@ extension LifetimeDependenceDefUseWalker {
       default:
         return .abortWalk
       }
+    case .dependence:
+       // An address-forwarding mark_dependence is simply a marker that indicates the start of an in-memory
+       // dependent value. Typically, it has no uses. If it does have uses, then they are visited earlier by
+       // LocalVariableAccessWalker to record any other local accesses.
+       return .continueWalk
     case .store:
       let si = localAccess.operand!.instruction as! StoringInstruction
       assert(si.sourceOperand == initialValue, "the only reachable store should be the current assignment")

SwiftCompilerSources/Sources/Optimizer/Utilities/LocalVariableUtils.swift

@@ -43,6 +43,7 @@ struct LocalVariableAccess: CustomStringConvertible {
     case inoutYield       // indirect yield from this accessor
     case beginAccess // Reading or reassigning a 'var'
     case load        // Reading a 'let'. Returning 'var' from an initializer.
+    case dependence  // A mark_dependence after an apply with an indirect result. No effect.
     case store       // 'var' initialization and destruction
     case apply       // indirect arguments
     case escape      // alloc_box captures
@@ -76,7 +77,7 @@ struct LocalVariableAccess: CustomStringConvertible {
       case .`init`, .modify:
         return true
       }
-    case .load:
+    case .load, .dependence:
       return false
     case .incomingArgument, .outgoingArgument, .store, .inoutYield:
       return true
@@ -115,6 +116,8 @@ struct LocalVariableAccess: CustomStringConvertible {
       str += "beginAccess"
     case .load:
       str += "load"
+    case .dependence:
+      str += "dependence"
     case .store:
       str += "store"
     case .apply:
@@ -149,7 +152,7 @@ class LocalVariableAccessInfo: CustomStringConvertible {
       case .`init`, .modify:
         break // lazily compute full assignment
       }
-    case .load:
+    case .load, .dependence:
       self._isFullyAssigned = false
     case .store:
       if let store = localAccess.instruction as? StoringInstruction {
@@ -197,20 +200,18 @@ class LocalVariableAccessInfo: CustomStringConvertible {
   }
 
   var description: String {
-    return "full-assign: \(_isFullyAssigned == nil ? "unknown" : String(describing: _isFullyAssigned!)) "
+    return "full-assign: \(_isFullyAssigned == nil ? "unknown" : String(describing: _isFullyAssigned!)), "
       + "\(access)"
   }
 
   // Does this address correspond to the local variable's base address? Any writes to this address will be a full
   // assignment. This should match any instructions that the LocalVariableAccessMap initializer below recognizes as an
   // allocation.
   static private func isBase(address: Value) -> Bool {
-    switch address {
-    case is AllocBoxInst, is AllocStackInst, is BeginAccessInst:
-      return true
-    default:
-      return false
-    }
+    // TODO: create an API alternative to 'accessPath' that bails out on the first path component and succeeds on the
+    // first begin_access.
+    let path = address.accessPath
+    return path.base.isLocal && path.projectionPath.isEmpty
   }
 }
 
@@ -377,6 +378,10 @@ extension LocalVariableAccessWalker : ForwardingDefUseWalker {
 extension LocalVariableAccessWalker: AddressUseVisitor {
   private mutating func walkDownAddressUses(address: Value) -> WalkResult {
     for operand in address.uses.ignoreTypeDependence {
+      if let md = operand.instruction as? MarkDependenceInst, operand == md.valueOperand {
+        // Record the forwarding mark_dependence as a fake access before continuing to walk down.
+        visit(LocalVariableAccess(.dependence, operand))
+      }
       if classifyAddress(operand: operand) == .abortWalk {
         return .abortWalk
       }

test/ModuleInterface/Inputs/lifetime_dependence.swift

@@ -1,3 +1,21 @@
+@_unsafeNonescapableResult
+@_alwaysEmitIntoClient
+@_transparent
+@lifetime(borrow source)
+internal func _overrideLifetime<T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable>(
+  _ dependent: consuming T, borrowing source: borrowing U) -> T {
+  dependent
+}
+
+@_unsafeNonescapableResult
+@_alwaysEmitIntoClient
+@_transparent
+@lifetime(source)
+internal func _overrideLifetime<T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable>(
+  _ dependent: consuming T, copying source: borrowing U) -> T {
+  dependent
+}
+
 public struct AnotherView : ~Escapable {
   @usableFromInline let _ptr: UnsafeRawBufferPointer
   @usableFromInline let _count: Int
@@ -21,19 +39,23 @@ public struct BufferView : ~Escapable {
   @inlinable
   @lifetime(borrow a)
   internal init(_ ptr: UnsafeRawBufferPointer, _ a: borrowing Array<Int>) {
-    self.init(ptr, a.count)
+    let bv = BufferView(ptr, a.count)
+    self = _overrideLifetime(bv, borrowing: a)
   }
   @inlinable
   @lifetime(a)
   internal init(_ ptr: UnsafeRawBufferPointer, _ a: consuming AnotherView) {
-    self.init(ptr, a._count)
+    let bv = BufferView(ptr, a._count)
+    self = _overrideLifetime(bv, copying: a)
   }
 }
 
 @inlinable
 @lifetime(x)
 public func derive(_ x: consuming BufferView) -> BufferView {
-  return BufferView(x._ptr, x._count)
+  let pointer = x._ptr
+  let bv = BufferView(pointer, x._count)
+  return _overrideLifetime(bv, copying: x)
 }
 
 @inlinable
@@ -42,7 +64,9 @@ public func use(_ x: consuming BufferView) {}
 @inlinable
 @lifetime(view)
 public func consumeAndCreate(_ view: consuming BufferView) -> BufferView {
-  return BufferView(view._ptr, view._count)
+  let pointer = view._ptr
+  let bv = BufferView(pointer, view._count)
+  return _overrideLifetime(bv, copying: view)
 }
 
 @inlinable
@@ -54,14 +78,6 @@ public func deriveThisOrThat(_ this: consuming BufferView, _ that: consuming Buf
   return BufferView(that._ptr, that._count)
 }
 
-@_unsafeNonescapableResult
-@_transparent
-@lifetime(borrow source)
-internal func _overrideLifetime<T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable>(
-  _ dependent: consuming T, borrowing source: borrowing U) -> T {
-  dependent
-}
-
 public struct Container {
   var buffer: UnsafeRawBufferPointer
   var object: AnyObject

test/SILOptimizer/lifetime_dependence/scopefixup.sil

@@ -85,7 +85,7 @@ bb2:
 sil @pointeeAddressor : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (UnsafePointer<τ_0_0>) -> UnsafePointer<τ_0_0>
 
 // Test dependence on a loaded trivial value. The dependence scope should not be on the access scope, which is narrower
-// than the scoped of the loaded value.
+// than the scope of the loaded value.
 //
 // CHECK-LABEL: sil hidden [ossa] @testTrivialAccess : $@convention(thin) (UnsafePointer<Int64>) -> () {
 // CHECK: bb0(%0 : $UnsafePointer<Int64>):