Fix computeAddressableRange, test, and comment.

Address review feedback from the previous commit.

Author: atrick

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

@@ -528,6 +528,13 @@ extension LifetimeDependence.Scope {
 
   // For trivial values, compute the range in which the value may have its address taken.
   // Returns nil unless the address has both an addressable parameter use and a dealloc_stack use.
+  //
+  // This extended range is convervative. In theory, it can lead to a "false positive" diagnostic if this scope was
+  // computed for a apply site that takes this address as *non-addressable*, as opposed to the apply site discovered
+  // here that takes the alloc_stack as an *addressable* argument. This won't normally happen because addressability
+  // depends on the value's type (via @_addressableForDepenencies), so all other lifetime-dependent applies should be
+  // addressable. Furthermore, SILGen only creates temporary stack locations for addressable arguments for a single
+  // argument.
   private static func computeAddressableRange(initializer: AccessBase.Initializer, _ context: Context)
     -> InstructionRange? {
     switch initializer {
@@ -538,23 +545,23 @@ extension LifetimeDependence.Scope {
         return nil
       }
       var isAddressable = false
-      var deallocInst: DeallocStackInst?
+      var deallocInsts = SingleInlineArray<DeallocStackInst>()
       for use in initialAddr.uses {
         let inst = use.instruction
         switch inst {
-        case let apply as ApplyInst:
+        case let apply as ApplySite:
           isAddressable = isAddressable || apply.isAddressable(operand: use)
         case let dealloc as DeallocStackInst:
-          deallocInst = dealloc
+          deallocInsts.append(dealloc)
         default:
           break
         }
       }
-      guard let deallocInst = deallocInst, isAddressable else {
+      guard isAddressable, !deallocInsts.isEmpty else {
         return nil
       }
       var range = InstructionRange(begin: initializingStore, context)
-      range.insert(deallocInst)
+      range.insert(contentsOf: deallocInsts)
       return range
     }
   }

test/SILOptimizer/lifetime_dependence/scope_fixup.sil

@@ -87,6 +87,11 @@ public struct AThing : ~Copyable {
   init(somethings: UnsafeMutableRawBufferPointer)
 }
 
+@_addressableForDependencies
+public struct InlineInt {
+  var i: Int
+}
+
 sil @getContainer : $@convention(thin) (@inout AThing) -> @lifetime(borrow address_for_deps 0) @out Container
 sil @getMutableView : $@convention(thin) (@in_guaranteed Container) -> @lifetime(copy 0) @out Optional<MutableView>
 sil @modAThing : $@convention(thin) (@inout AThing) -> ()
@@ -138,6 +143,10 @@ sil @getAddressableObj : $@convention(method) (@guaranteed Holder) -> @owned Add
 
 sil @getAddressableObj_NE : $@convention(thin) (@in_guaranteed AddressableForDepsObj) -> @owned NE
 
+sil @globalAddress : $@convention(thin) () -> Builtin.RawPointer
+sil @addressInt : $@convention(thin) (@in_guaranteed InlineInt) -> @lifetime(borrow address_for_deps 0) @owned Span<Int>
+sil @useSpan : $@convention(thin) (@guaranteed Span<Int>) -> ()
+
 // NCContainer.wrapper._read:
 //   var wrapper: Wrapper {
 //     _read {
@@ -914,3 +923,50 @@ bb2:
   dealloc_box [dead_end] %1
   unreachable
 }
+
+// Test trivial alloc_stack extension when used by an addressable dependency.
+//
+// CHECK-LABEL: sil hidden [noinline] [ossa] @testTempTrivialStackCopy : $@convention(thin) () -> () {
+// CHECK:   [[STK:%[0-9]+]] = alloc_stack $InlineInt
+// CHECK:   [[MD:%[0-9]+]] = mark_dependence [unresolved] %{{.*}} on [[STK]]
+// CHECK:   [[VAR:%[0-9]+]] = move_value [var_decl] [[MD]]
+// CHECK: bb3:                                              // Preds: bb2 bb1
+// CHECK:   [[BORROW:%[0-9]+]] = begin_borrow [[VAR]]
+// CHECK:   apply %{{.*}}([[BORROW]]) : $@convention(thin) (@guaranteed Span<Int>) -> ()
+// CHECK:   destroy_value [[VAR]]
+// CHECK:   dealloc_stack [[STK]]
+// CHECK-LABEL: } // end sil function 'testTempTrivialStackCopy'
+sil hidden [noinline] [ossa] @testTempTrivialStackCopy : $@convention(thin) () -> () {
+bb0:
+  %0 = function_ref @globalAddress : $@convention(thin) () -> Builtin.RawPointer
+  %1 = apply %0() : $@convention(thin) () -> Builtin.RawPointer
+  %2 = pointer_to_address %1 to [strict] $*InlineInt
+  %3 = begin_access [read] [dynamic] %2
+  %4 = load [trivial] %3
+  end_access %3
+  %6 = alloc_stack $InlineInt
+  store %4 to [trivial] %6
+
+  %8 = function_ref @addressInt : $@convention(thin) (@in_guaranteed InlineInt) -> @lifetime(borrow address_for_deps 0) @owned Span<Int>
+  %9 = apply %8(%6) : $@convention(thin) (@in_guaranteed InlineInt) -> @lifetime(borrow address_for_deps 0) @owned Span<Int>
+  %10 = mark_dependence [unresolved] %9 on %6
+  %11 = move_value [var_decl] %10
+  cond_br undef, bb1, bb2
+
+bb1:
+  dealloc_stack %6
+  br bb3
+bb2:
+  dealloc_stack %6
+  br bb3
+
+bb3:
+  %13 = begin_borrow %11
+
+  %14 = function_ref @useSpan : $@convention(thin) (@guaranteed Span<Int>) -> ()
+  %15 = apply %14(%13) : $@convention(thin) (@guaranteed Span<Int>) -> ()
+  end_borrow %13
+  destroy_value %11
+  %18 = tuple ()
+  return %18
+}

test/SILOptimizer/lifetime_dependence/verify_diagnostics.sil

@@ -67,6 +67,8 @@ public struct InlineInt {
 }
 sil @globalAddress : $@convention(thin) () -> Builtin.RawPointer
 sil @addressInt : $@convention(thin) (@in_guaranteed InlineInt) -> @lifetime(borrow address_for_deps 0) @owned Span<Int>
+sil @addressOfInt : $@convention(thin) (@in_guaranteed Int) -> @lifetime(borrow address 0) @owned Span<Int>
+sil @noAddressInt : $@convention(thin) (@in_guaranteed Int) -> @lifetime(borrow 0) @owned Span<Int>
 sil @useSpan : $@convention(thin) (@guaranteed Span<Int>) -> ()
 
 // Test returning a owned dependence on a trivial value
@@ -259,10 +261,12 @@ bb0(%0 : $*NE, %1 : $*Holder):
 // extends to the dealloc_stack.
 //
 // SILGen should never generate temporary stack copies for addressable dependencies, but we need to diagnose them anyway
-// so that a SILGen bug does not become a miscompile.
+// so that a SILGen bug does not become a miscompile. Furthermore, LifetimeDependenceScopeFixup now automatically
+// extends such local allocations, so this diagnostic only fires on source-level tests when scope extension is
+// impossible.
 //
 // rdar://159680262 ([nonescapable] diagnose dependence on a temporary copy of a global array)
-sil hidden [noinline] [ossa] @testTrivialStackCopy : $@convention(thin) () -> () {
+sil hidden [noinline] [ossa] @testTempTrivialStackCopy : $@convention(thin) () -> () {
 bb0:
   %0 = function_ref @globalAddress : $@convention(thin) () -> Builtin.RawPointer
   %1 = apply %0() : $@convention(thin) () -> Builtin.RawPointer
@@ -287,3 +291,64 @@ bb0:
   %18 = tuple ()
   return %18
 }
+
+// Test an address dependency on a trivial value temporarily copied to the stack. The value's addressable range only
+// extends to the dealloc_stack. This represents the SIL from @testTempTrivialStackCopy after
+// LifetimeDependenceScopeFixup has extended the local allocation.
+sil hidden [noinline] [ossa] @testExtendedTrivialStackCopy : $@convention(thin) () -> () {
+bb0:
+  %0 = function_ref @globalAddress : $@convention(thin) () -> Builtin.RawPointer
+  %1 = apply %0() : $@convention(thin) () -> Builtin.RawPointer
+  %2 = pointer_to_address %1 to [strict] $*InlineInt
+  %3 = begin_access [read] [dynamic] %2
+  %4 = load [trivial] %3
+  end_access %3
+  %6 = alloc_stack $InlineInt
+  store %4 to [trivial] %6
+
+  %8 = function_ref @addressInt : $@convention(thin) (@in_guaranteed InlineInt) -> @lifetime(borrow address_for_deps 0) @owned Span<Int>
+  %9 = apply %8(%6) : $@convention(thin) (@in_guaranteed InlineInt) -> @lifetime(borrow address_for_deps 0) @owned Span<Int>
+  %10 = mark_dependence [unresolved] %9 on %6
+  %11 = move_value [var_decl] %10
+  %13 = begin_borrow %11
+
+  %14 = function_ref @useSpan : $@convention(thin) (@guaranteed Span<Int>) -> ()
+  %15 = apply %14(%13) : $@convention(thin) (@guaranteed Span<Int>) -> ()
+  end_borrow %13
+  destroy_value %11
+  dealloc_stack %6
+  %18 = tuple ()
+  return %18
+}
+
+// Test two address dependencies on the same temporay. The first dependency is addressable; it's uses are covered by the
+// stack allocation. The second dependency is non-addressable; it's uses are not covered by the stack allocation. So,
+// this is correct SIL but is falsely diagnosed as a lifetime error. This conservative error is fine because SILGen
+// never reuses a temporary allocaiton for both an addressable and non-addressable dependency.
+sil hidden [noinline] [ossa] @testFalseTempTrivialStackCopy : $@convention(thin) (Int) -> () {
+bb0(%0: $Int):
+  %1 = alloc_stack $Int
+  store %0 to [trivial] %1
+
+  %3 = function_ref @addressOfInt : $@convention(thin) (@in_guaranteed Int) -> @lifetime(borrow address 0) @owned Span<Int>
+  %4 = apply %3(%1) : $@convention(thin) (@in_guaranteed Int) -> @lifetime(borrow address 0) @owned Span<Int>
+  %5 = mark_dependence [unresolved] %4 on %1
+  %6 = move_value [var_decl] %5
+  destroy_value %6
+
+  %8 = function_ref @noAddressInt : $@convention(thin) (@in_guaranteed Int) -> @lifetime(borrow 0) @owned Span<Int>
+  %9 = apply %8(%1) : $@convention(thin) (@in_guaranteed Int) -> @lifetime(borrow 0) @owned Span<Int>
+  %10 = mark_dependence [unresolved] %9 on %1
+  %11 = move_value [var_decl] %10 // expected-error{{lifetime-dependent value escapes its scope}}
+  // expected-note@-15{{it depends on the lifetime of an argument of 'testFalseTempTrivialStackCopy'}}
+
+  dealloc_stack %1
+
+  %13 = begin_borrow %11
+  %14 = function_ref @useSpan : $@convention(thin) (@guaranteed Span<Int>) -> ()
+  %15 = apply %14(%13) : $@convention(thin) (@guaranteed Span<Int>) -> ()
+  end_borrow %13
+  destroy_value %11 // expected-note{{this use of the lifetime-dependent value is out of scope}}
+  %18 = tuple ()
+  return %18
+}

test/SILOptimizer/lifetime_dependence/verify_diagnostics.swift

@@ -322,18 +322,63 @@ func inoutToImmortal(_ s: inout RawSpan) {
 @available(Span 0.1, *)
 var values: InlineArray<_, Int> = [0, 1, 2]
 
+@available(Span 0.1, *)
+func getValues() -> InlineArray<3, Int> { values }
+
+// Test a trivial global variable used by addressableForDependencies (InlineArray.span). This either requires a direct address
+// to the global or it requires extension of the local stack allocation by LifetimeDependenceScopeFixup.
+//
 // rdar://159680262 ([nonescapable] diagnose dependence on a temporary copy of a global array)
 @available(Span 0.1, *)
 func readGlobalSpan() -> Int {
   let span = values.span
   return span[3]
 }
 
-// TODO: rdar://151320168 ([nonescapable] support immortal span over global array)
+// Test a trivial global variable used by addressableForDependencies (InlineArray.span).
+//
+// TODO: This will no longer be an error when SILGen passes a direct address to the global instead of a temporary stack
+// allocation: rdar://151320168 ([nonescapable] support immortal span over global array)
 @available(Span 0.1, *)
 @_lifetime(immortal)
 func returnGlobalSpan() -> Span<Int> {
   let span = values.span // expected-error{{lifetime-dependent variable 'span' escapes its scope}}
   // expected-note@-1{{it depends on this scoped access to variable 'values'}}
   return span // expected-note{{this use causes the lifetime-dependent value to escape}}
 }
+
+// Test a trivial temporary stack allocation used by addressableForDependencies
+// (InlineArray.span). LifetimeDependenceScopeFixup needs to extend the local stack allocation.
+@available(Span 0.1, *)
+func readTempSpan() -> Int {
+  let span = getValues().span
+  return span[3]
+}
+
+// Test a trivial temporary stack allocation used by addressableForDependencies (InlineArray.span). This illegally
+// returns an address into a temporary.
+@available(Span 0.1, *)
+@_lifetime(immortal)
+func returnTempSpan() -> Span<Int> {
+  let span = getValues().span // expected-error{{lifetime-dependent variable 'span' escapes its scope}}
+  // expected-note@-1{{it depends on the lifetime of this parent value}}
+  return span // expected-note{{this use causes the lifetime-dependent value to escape}}
+}
+
+// Test a trivial temporary stack allocation used by an addressable parameter
+// (Borrow.init). LifetimeDependenceScopeFixup needs to extend the local stack allocation.
+@available(Span 0.1, *)
+func readTempBorrow() -> Int {
+  let borrow = Borrow(3)
+  return borrow[]
+}
+
+// Test a trivial temporary stack allocation used by an addressable parameter (Borrow.init). This illegally returns an
+// address into a temporary.
+@available(Span 0.1, *)
+@_lifetime(immortal)
+func returnTempBorrow() -> Borrow<Int> {
+  let span = Borrow(3) // expected-error{{lifetime-dependent variable 'span' escapes its scope}}
+  // expected-note@-1{{it depends on the lifetime of this parent value}}
+  return span // expected-note{{this use causes the lifetime-dependent value to escape}}
+}