Merge pull request #81004 from atrick/nested-modify-read

Fix LifetimeDependenceScopeFixup: read access can depend on 'inout'

Author: atrick

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift

@@ -453,30 +453,30 @@ extension ScopeExtension {
   /// logically combine them into a single access for the purpose of diagnostic lifetime dependence.
   var dependsOnArgs: SingleInlineArray<FunctionArgument> {
     let noCallerScope = SingleInlineArray<FunctionArgument>()
+    // Check that the dependent value is returned by this function.
     if !dependsOnCaller! {
       return noCallerScope
     }
-    // All owners must be arguments to depend on the caller's scope.
+    // Check that all nested scopes that it depends on can be covered by exclusive access in the caller.
+    for extScope in scopes {
+      switch extScope.scope {
+      case .access:
+        break
+      default:
+        return noCallerScope
+      }
+    }
+    // All owners must be arguments with exclusive access to depend on the caller's scope (inout_aliasable arguments do
+    // not have exclusivity).
     var compatibleArgs = SingleInlineArray<FunctionArgument>()
     for owner in owners {
       guard let arg = owner as? FunctionArgument else {
         return noCallerScope
       }
-      compatibleArgs.push(arg)
-    }
-    // The only local scopes that are compatible with the caller's scope are access scopes that either read an
-    // immutable argument or modify a mutable argument.
-    for extScope in scopes {
-      switch extScope.scope {
-      case let .access(beginAccess):
-        for arg in compatibleArgs {
-          if !beginAccess.accessKind.isCompatible(with: arg.convention) {
-            return noCallerScope
-          }
-        }
-      default:
+      guard arg.convention.isIndirectIn || arg.convention.isInout else {
         return noCallerScope
       }
+      compatibleArgs.push(arg)
     }
     return compatibleArgs
   }

SwiftCompilerSources/Sources/SIL/Argument.swift

@@ -607,19 +607,6 @@ public enum ArgumentConvention : CustomStringConvertible {
   }
 }
 
-extension BeginAccessInst.AccessKind {
-  public func isCompatible(with convention: ArgumentConvention) -> Bool {
-    switch self {
-    case .`init`, .deinit:
-      return false
-    case .read:
-      return convention.isIndirectIn
-    case .modify:
-      return convention.isInout
-    }
-  }
-}
-
 // Bridging utilities
 
 extension BridgedArgument {

test/SILOptimizer/lifetime_dependence/scope_fixup.sil

@@ -65,7 +65,8 @@ sil @useA : $@convention(thin) (A) -> ()
 
 sil @getPtr : $@convention(thin) () -> @out UnsafeRawPointer
 
-sil @getOwnedNE : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned NE
+sil @getOwnedNE : $@convention(thin) (@guaranteed Holder) -> @lifetime(borrow 0) @owned NE
+sil @getOwnedNEFromInout : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned NE
 
 sil @useNE : $@convention(thin) (@guaranteed NE) -> ()
 
@@ -470,7 +471,7 @@ bb1:
 
 bb2:
   %access = begin_access [modify] [unknown] %0
-  %f = function_ref @getOwnedNE : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned NE
+  %f = function_ref @getOwnedNEFromInout : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned NE
   %ne = apply %f(%access) : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned NE
   %md = mark_dependence [unresolved] %ne on %access
   end_access %access
@@ -480,3 +481,26 @@ bb2:
 bb3(%ret : @owned $Optional<NE>):
   return %ret
 }
+
+// Allow a [read] access to depend on a caller's [modify] access.
+//
+// CHECK-LABEL: sil hidden [noinline] [ossa] @testNestedModRead : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned NE {
+// CHECK: bb0(%0 : $*Holder):
+// CHECK:   [[ACCESS:%.*]] = begin_access [read] [unknown] %0
+// CHECK:   [[LD:%.*]] = load [copy] [[ACCESS]]
+// CHECK:   [[MD:%.*]] = mark_dependence [unresolved] %{{.*}} on %0
+// CHECK:   destroy_value [[LD]]
+// CHECK:   end_access [[ACCESS]]
+// CHECK:   return [[MD]]
+// CHECK-LABEL: } // end sil function 'testNestedModRead'
+sil hidden [noinline] [ossa] @testNestedModRead : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned NE {
+bb0(%0 : $*Holder):
+  %access = begin_access [read] [unknown] %0
+  %holder = load [copy] %access
+  end_access %access
+  %f = function_ref @getOwnedNE : $@convention(thin) (@guaranteed Holder) -> @lifetime(borrow 0) @owned NE
+  %ne = apply %f(%holder) : $@convention(thin) (@guaranteed Holder) -> @lifetime(borrow 0) @owned NE
+  %md = mark_dependence [unresolved] %ne on %access
+  destroy_value %holder
+  return %md
+}

test/SILOptimizer/lifetime_dependence/semantics.swift

@@ -434,9 +434,8 @@ func testGlobal(local: InnerTrivial) -> Span<Int> {
 
 @lifetime(&a)
 func testInoutBorrow(a: inout [Int]) -> Span<Int> {
-  a.span() // expected-error {{lifetime-dependent value escapes its scope}}
-  // expected-note @-1{{it depends on this scoped access to variable 'a'}}
-} // expected-note {{this use causes the lifetime-dependent value to escape}}
+  a.span() // OK
+}
 
 @lifetime(&a)
 func testInoutMutableBorrow(a: inout [Int]) -> MutableSpan<Int> {
@@ -462,12 +461,10 @@ extension Container {
 
   @lifetime(&self)
   mutating func mutableView() -> MutableView {
-    // Reading 'self.owner' creates a local borrow scope. This new MutableView
-    // depends on a the local borrow scope for 'self.owner', so it cannot be
-    // returned.
-    MutableView(owner: self.owner) // expected-error {{lifetime-dependent value escapes its scope}}
-    // expected-note @-1{{it depends on this scoped access to variable 'self'}}
-  } // expected-note    {{this use causes the lifetime-dependent value to escape}}
+    // Reading 'self.owner' creates a local borrow scope. The caller's exclusive access on 'self' is sufficient for the
+    // result.
+    MutableView(owner: self.owner) // OK
+  }
 
   @lifetime(&self)
   mutating func mutableViewModifiesOwner() -> MutableView {

test/SILOptimizer/lifetime_dependence/verify_diagnostics.swift

@@ -2,6 +2,8 @@
 // RUN:   -o /dev/null \
 // RUN:   -verify \
 // RUN:   -sil-verify-all \
+// RUN:   -module-name test \
+// RUN:   -define-availability "Span 0.1:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, visionOS 9999" \
 // RUN:   -enable-experimental-feature LifetimeDependence
 
 // REQUIRES: swift_in_compiler
@@ -13,13 +15,23 @@
 // Test that conditionally returning an Optional succeeds.
 //
 // See scope_fixup.sil: testReturnPhi.
-@available(SwiftStdlib 6.2, *)
+@available(Span 0.1, *)
 extension Array {
   @lifetime(&self)
-  mutating func dumb() -> MutableSpan<Element>? {
+  mutating func getOptionalMutableSpan() -> MutableSpan<Element>? {
     if count == 0 {
       return nil
     }
     return mutableSpan
   }
 }
+
+// Test that returning an immutable Span from an inout Array.
+//
+// See scope_fixup.sil: testNestedModRead.
+@available(Span 0.1, *)
+@inline(never)
+@lifetime(&array)
+func getImmutableSpan(_ array: inout [Int]) -> Span<Int> {
+ return array.span
+}