Merge pull request #78482 from atrick/fix-markdep-load

Fix LifetimeDependenceScopeFixup to avoid rewriting mark_dependence.

Author: atrick

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift

@@ -119,9 +119,8 @@ let lifetimeDependenceScopeFixupPass = FunctionPass(
       continue
     }
     // Redirect the dependence base to ignore irrelevant borrow scopes.
-    guard let newLifetimeDep = markDep.rewriteSkippingBorrow(scope: innerLifetimeDep.scope, context) else {
-      continue
-    }
+    let newLifetimeDep = markDep.rewriteSkippingBorrow(scope: innerLifetimeDep.scope, context)
+
     // Recursively sink enclosing end_access, end_borrow, or end_apply.
     let args = extendScopes(dependence: newLifetimeDep, localReachabilityCache, context)
 
@@ -135,9 +134,9 @@ private extension MarkDependenceInst {
   ///
   /// Note: this could be done as a general simplification, e.g. after inlining. But currently this is only relevant for
   /// diagnostics.
-  func rewriteSkippingBorrow(scope: LifetimeDependence.Scope, _ context: FunctionPassContext) -> LifetimeDependence? {
+  func rewriteSkippingBorrow(scope: LifetimeDependence.Scope, _ context: FunctionPassContext) -> LifetimeDependence {
     guard let newScope = scope.ignoreBorrowScope(context) else {
-      return nil
+      return LifetimeDependence(scope: scope, dependentValue: self)
     }
     let newBase = newScope.parentValue
     if newBase != self.baseOperand.value {

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

@@ -394,6 +394,8 @@ extension LifetimeDependence.Scope {
       self = .borrowed(BeginBorrowValue(bbi)!)
     case is MoveValueInst:
       self = .owned(introducer)
+    case let bai as BeginAccessInst:
+      self = .access(bai)
     default:
       self = .unknown(introducer)
     }
@@ -423,12 +425,12 @@ extension LifetimeDependence.Scope {
   /// Ignore "irrelevent" borrow scopes: load_borrow or begin_borrow without [var_decl]
   func ignoreBorrowScope(_ context: some Context) -> LifetimeDependence.Scope? {
     guard case let .borrowed(beginBorrowVal) = self else {
-      return self
+      return nil
     }
     switch beginBorrowVal {
     case let .beginBorrow(bb):
       if bb.isFromVarDecl {
-        return self
+        return nil
       }
       return LifetimeDependence.Scope(base: bb.borrowedValue, context).ignoreBorrowScope(context)
     case let .loadBorrow(lb):

test/SILGen/addressors.swift

@@ -46,6 +46,7 @@ struct A {
 // CHECK:   [[BASE:%.*]] = load [[T0]] : $*UnsafeMutablePointer<Int32>
 // CHECK:   return [[BASE]] : $UnsafeMutablePointer<Int32>
 
+// SILGEN-LABEL: sil hidden [ossa] @$s10addressors5test0yyF : $@convention(thin) () -> () {
 // CHECK-LABEL: sil hidden @$s10addressors5test0yyF : $@convention(thin) () -> () {
 func test0() {
 // CHECK: [[A:%.*]] = alloc_stack [var_decl] $A
@@ -55,12 +56,16 @@ func test0() {
 // CHECK: store [[AVAL]] to [[A]]
   var a = A()
 
-// CHECK: [[ACCESS:%.*]] = begin_access [read] [static] [[A]] : $*A
+// SILGEN: [[ACCESS:%.*]] = begin_access [read] [unknown] %{{.*}} : $*A
+// SILGEN: [[LD:%.*]] = load [trivial] [[ACCESS]] : $*A
+// SILGEN: [[T3:%.*]] = pointer_to_address %{{.*}} : $Builtin.RawPointer to [strict] $*Int32
+// SILGEN: [[MD:%.*]] = mark_dependence [unresolved] [[T3]] : $*Int32 on [[LD]] : $A
+
 // CHECK: [[T0:%.*]] = function_ref @$s10addressors1AVys5Int32VAEcilu :
 // CHECK: [[T1:%.*]] = apply [[T0]]({{%.*}}, [[AVAL]])
 // CHECK: [[T2:%.*]] = struct_extract [[T1]] : $UnsafePointer<Int32>, #UnsafePointer._rawValue
 // CHECK: [[T3:%.*]] = pointer_to_address [[T2]] : $Builtin.RawPointer to [strict] $*Int32
-// CHECK: [[MD:%.*]] = mark_dependence [nonescaping] [[T3]] : $*Int32 on [[ACCESS]] : $*A
+// CHECK: [[MD:%.*]] = mark_dependence [nonescaping] [[T3]] : $*Int32 on [[AVAL]] : $A
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unsafe] [[MD]] : $*Int32
 // CHECK: [[Z:%.*]] = load [[ACCESS]] : $*Int32
   let z = a[10]

test/SILOptimizer/lifetime_dependence/lifetime_dependence_scope_fixup.swift

@@ -228,3 +228,12 @@ public func test10() {
   }
 }
 
+// CHECK-LABEL: sil hidden @$s31lifetime_dependence_scope_fixup37testPointeeDependenceOnMutablePointer1pySPys5Int64VG_tF : $@convention(thin) (UnsafePointer<Int64>) -> () {
+// CHECK: bb0(%0 : $UnsafePointer<Int64>):
+// CHECK:   [[ALLOC:%.*]] = alloc_stack [var_decl] $UnsafePointer<Int64>, var, name "ptr", type $UnsafePointer<Int64>
+// CHECK:   mark_dependence [nonescaping] %{{.*}} on %0
+// CHECK-LABEL: } // end sil function '$s31lifetime_dependence_scope_fixup37testPointeeDependenceOnMutablePointer1pySPys5Int64VG_tF'
+func testPointeeDependenceOnMutablePointer(p: UnsafePointer<Int64>) {
+  var ptr = p
+  _ = ptr.pointee
+}

test/SILOptimizer/lifetime_dependence/scopefixup.sil

@@ -81,3 +81,42 @@ bb2:
   destroy_value %10
   unwind
 }
+
+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.
+//
+// CHECK-LABEL: sil hidden [ossa] @testTrivialAccess : $@convention(thin) (UnsafePointer<Int64>) -> () {
+// CHECK: bb0(%0 : $UnsafePointer<Int64>):
+// CHECK:   [[ACCESS:%.*]] = begin_access [read] [unknown]
+// CHECK:   [[LD:%.*]] = load [trivial] [[ACCESS]]
+// CHECK:   [[ADR:%.*]] = pointer_to_address %{{.*}} to [strict] $*Int64
+// CHECK:   [[MD:%.*]] = mark_dependence [unresolved] [[ADR]] on [[LD]]
+// CHECK:   [[ACCESS2:%.*]] = begin_access [read] [unsafe] [[MD]]
+// CHECK:   end_access [[ACCESS2]]
+// CHECK:   end_access [[ACCESS]]
+// CHECK-LABEL: } // end sil function 'testTrivialAccess'
+sil hidden [ossa] @testTrivialAccess : $@convention(thin) (UnsafePointer<Int64>) -> () {
+bb0(%0 : $UnsafePointer<Int64>):
+  debug_value %0, let, name "p", argno 1
+  %2 = alloc_box ${ var UnsafePointer<Int64> }, var, name "ptr"
+  %3 = begin_borrow [var_decl] %2
+  %4 = project_box %3, 0
+  store %0 to [trivial] %4
+  %6 = begin_access [read] [unknown] %4
+  %7 = load [trivial] %6
+  end_access %6
+  %9 = function_ref @pointeeAddressor : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (UnsafePointer<τ_0_0>) -> UnsafePointer<τ_0_0>
+  %10 = apply %9<Int64>(%7) : $@convention(method) <τ_0_0 where τ_0_0 : ~Copyable> (UnsafePointer<τ_0_0>) -> UnsafePointer<τ_0_0>
+  %11 = struct_extract %10, #UnsafePointer._rawValue
+  %12 = pointer_to_address %11 to [strict] $*Int64
+  %13 = mark_dependence [unresolved] %12 on %7
+  %14 = begin_access [read] [unsafe] %13
+  %15 = load [trivial] %14
+  end_access %14
+  end_borrow %3
+  destroy_value %2
+  %19 = tuple ()
+  return %19
+}

test/SILOptimizer/unsafeAddress.swift

@@ -97,13 +97,11 @@ func testSMod(s: inout S) {
 
 // Accessing s.data causes an escaping dependence because we don't extend the local access scope of 's'.
 //
-// TODO: could we notice the local access is already nested inside the same kind of access (modify) and consider this to
-// be 'mark_dependence [nonescaping]'?
-//
 // CHECK-LABEL: sil hidden @$s4main16testSInoutBorrow5mut_syAA1SVz_tF : $@convention(thin) (@inout S) -> () {
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0
+// CHECK: [[LD:%.*]] = load [[ACCESS]]
 // CHECK: [[ADR:%.*]] = pointer_to_address %{{.*}} to [strict] $*NC
-// CHECK: [[MD:%.*]] = mark_dependence [[ADR]] on [[ACCESS]]
+// CHECK: [[MD:%.*]] = mark_dependence [nonescaping] [[ADR]] on [[LD]]
 // CHECK: begin_access [read] [unsafe] [[MD]]
 // CHECK: apply
 // CHECK: end_access
@@ -114,8 +112,9 @@ func testSInoutBorrow(mut_s s: inout S) {
 
 // CHECK-LABEL: sil hidden @$s4main19testSInoutMutBorrow5mut_syAA1SVz_tF : $@convention(thin) (@inout S) -> () {
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [static] %0
+// CHECK: [[LD:%.*]] = load [[ACCESS]]
 // CHECK: [[ADR:%.*]] = pointer_to_address %{{.*}} to [strict] $*NC
-// CHECK: [[MD:%.*]] = mark_dependence [nonescaping] [[ADR]] on [[ACCESS]]
+// CHECK: [[MD:%.*]] = mark_dependence [nonescaping] [[ADR]] on [[LD]]
 // CHECK: begin_access [read] [unsafe] [[MD]]
 // CHECK: apply
 // CHECK: end_access