Merge pull request swiftlang#80782 from atrick/rdar143992296-addressliverange

Fix AddressOwnershipLiveRange to include the full range.

Author: atrick

SwiftCompilerSources/Sources/Optimizer/Utilities/AddressUtils.swift

@@ -501,6 +501,11 @@ enum AddressOwnershipLiveRange : CustomStringConvertible {
     }
   }
 
+  /// Return the live range of the addressable value that reaches 'begin', not including 'begin', which may itself be an
+  /// access of the address.
+  ///
+  /// The range ends at the destroy or reassignment of the addressable value.
+  ///
   /// Return nil if the live range is unknown.
   static func compute(for address: Value, at begin: Instruction,
                       _ localReachabilityCache: LocalVariableReachabilityCache,
@@ -624,7 +629,7 @@ extension AddressOwnershipLiveRange {
     var reachableUses = Stack<LocalVariableAccess>(context)
     defer { reachableUses.deinitialize() }
 
-    localReachability.gatherKnownReachableUses(from: assignment, in: &reachableUses)
+    localReachability.gatherKnownLifetimeUses(from: assignment, in: &reachableUses)
 
     let assignmentInst = assignment.instruction ?? allocation.parentFunction.entryBlock.instructions.first!
     var range = InstructionRange(begin: assignmentInst, context)

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

@@ -930,8 +930,8 @@ extension LifetimeDependenceDefUseWalker {
       // of its forwarded address has were visited by LocalVariableAccessWalker and recorded as separate 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")
+      // A store does not use the previous in-memory value.
+      return .continueWalk
     case .apply:
       return visitAppliedUse(of: localAccess.operand!, by: localAccess.instruction as! FullApplySite)
     case .escape:
@@ -946,7 +946,6 @@ extension LifetimeDependenceDefUseWalker {
     case .incomingArgument:
       fatalError("Incoming arguments are never reachable")
     }
-    return .continueWalk
   }
 
   private mutating func visitAppliedUse(of operand: Operand, by apply: FullApplySite) -> WalkResult {

SwiftCompilerSources/Sources/Optimizer/Utilities/LocalVariableUtils.swift

@@ -630,13 +630,17 @@ struct LocalVariableReachableAccess {
 
 // Find reaching assignments...
 extension LocalVariableReachableAccess {
-  // Gather all fully assigned accesses that reach `instruction`.
+  // Gather all fully assigned accesses that reach 'instruction'. If 'instruction' is itself a modify access, it is
+  // ignored and the nearest assignments above 'instruction' are still gathered.
   func gatherReachingAssignments(for instruction: Instruction, in accessStack: inout Stack<LocalVariableAccess>)
     -> Bool {
     var blockList = BasicBlockWorklist(context)
     defer { blockList.deinitialize() }
 
-    let initialEffect = backwardScanAccesses(before: instruction, accessStack: &accessStack)
+    var initialEffect: BlockEffect? = nil
+    if let prev = instruction.previous {
+      initialEffect = backwardScanAccesses(before: prev, accessStack: &accessStack)
+    }
     if !backwardPropagateEffect(in: instruction.parentBlock, effect: initialEffect, blockList: &blockList,
                                 accessStack: &accessStack) {
       return false
@@ -647,7 +651,7 @@ extension LocalVariableReachableAccess {
       // lattice: none -> read -> modify -> escape -> assign
       //
       // `blockInfo.effect` is the same as `currentEffect` returned by backwardScanAccesses, except when an early escape
-      // happens after an assign.
+      // happens below an assign, in which case we report the escape here.
       switch currentEffect {
       case .none, .read, .modify, .escape:
         break
@@ -695,7 +699,6 @@ extension LocalVariableReachableAccess {
         continue
       case .assign:
         accessStack.push(accessInfo.access)
-        break
       case .escape:
         break
       }
@@ -709,25 +712,31 @@ extension LocalVariableReachableAccess {
 extension LocalVariableReachableAccess {
   /// This performs a forward CFG walk to find known reachable uses from `assignment`. This ignores aliasing and
   /// escapes.
-  func gatherKnownReachableUses(from assignment: LocalVariableAccess,
+  ///
+  /// The known live range is the range in which the assigned value is valid and may be used by dependent values. It
+  /// includes the destroy or reassignment of the local.
+  func gatherKnownLifetimeUses(from assignment: LocalVariableAccess,
                                 in accessStack: inout Stack<LocalVariableAccess>) {
     if let modifyInst = assignment.instruction {
-      _ = gatherReachableUses(after: modifyInst, in: &accessStack, allowEscape: true)
+      _ = gatherReachableUses(after: modifyInst, in: &accessStack, lifetime: true)
+      return
     }
-    gatherKnownReachableUsesFromEntry(in: &accessStack)
+    gatherKnownLifetimeUsesFromEntry(in: &accessStack)
   }
 
   /// This performs a forward CFG walk to find known reachable uses from the function entry. This ignores aliasing and
   /// escapes.
-  private func gatherKnownReachableUsesFromEntry(in accessStack: inout Stack<LocalVariableAccess>) {
+  private func gatherKnownLifetimeUsesFromEntry(in accessStack: inout Stack<LocalVariableAccess>) {
     assert(accessMap.liveInAccess!.kind == .incomingArgument, "only an argument access is live in to the function")
     let firstInst = accessMap.function.entryBlock.instructions.first!
-    _ = gatherReachableUses(onOrAfter: firstInst, in: &accessStack, allowEscape: true)
+    _ = gatherReachableUses(onOrAfter: firstInst, in: &accessStack, lifetime: true)
   }
 
   /// This performs a forward CFG walk to find all reachable uses of `modifyInst`. `modifyInst` may be a `begin_access
   /// [modify]` or instruction that initializes the local variable.
   ///
+  /// This does not include the destroy or reassignment of the value set by `modifyInst`.
+  ///
   /// Returns true if all possible reachable uses were visited. Returns false if any escapes may reach `modifyInst` are
   /// reachable from `modifyInst`.
   ///
@@ -749,37 +758,40 @@ extension LocalVariableReachableAccess {
     if accessInfo.hasEscaped! {
       return false
     }
-    return gatherReachableUses(after: modifyInst, in: &accessStack, allowEscape: false)
+    return gatherReachableUses(after: modifyInst, in: &accessStack, lifetime: false)
   }
 
   /// This performs a forward CFG walk to find all uses of this local variable reachable after `begin`.
   ///
-  /// If `allowEscape` is true, then this returns false if the walk ended early because of a reachable escape.
+  /// If `lifetime` is true, then this gathers the full known lifetime, includeing destroys and reassignments ignoring
+  /// escapes.
+  ///
+  /// If `lifetime` is false, then this returns `false` if the walk ended early because of a reachable escape.
   private func gatherReachableUses(after begin: Instruction, in accessStack: inout Stack<LocalVariableAccess>,
-                                   allowEscape: Bool) -> Bool {
+                                   lifetime: Bool) -> Bool {
     if let term = begin as? TermInst {
       for succ in term.successors {
-        if !gatherReachableUses(onOrAfter: succ.instructions.first!, in: &accessStack, allowEscape: allowEscape) {
+        if !gatherReachableUses(onOrAfter: succ.instructions.first!, in: &accessStack, lifetime: lifetime) {
           return false
         }
       }
       return true
     } else {
-      return gatherReachableUses(onOrAfter: begin.next!, in: &accessStack, allowEscape: allowEscape)
+      return gatherReachableUses(onOrAfter: begin.next!, in: &accessStack, lifetime: lifetime)
     }
   }
 
   /// This performs a forward CFG walk to find all uses of this local variable reachable after and including `begin`.
   ///
-  /// If `allowEscape` is true, then this returns false if the walk ended early because of a reachable escape.
+  /// If `lifetime` is true, then this returns false if the walk ended early because of a reachable escape.
   private func gatherReachableUses(onOrAfter begin: Instruction, in accessStack: inout Stack<LocalVariableAccess>,
-                                   allowEscape: Bool) -> Bool {
+                                   lifetime: Bool) -> Bool {
     var blockList = BasicBlockWorklist(context)
     defer { blockList.deinitialize() }
 
     let initialBlock = begin.parentBlock
-    let initialEffect = forwardScanAccesses(after: begin, accessStack: &accessStack, allowEscape: allowEscape)
-    if !allowEscape, initialEffect == .escape {
+    let initialEffect = forwardScanAccesses(after: begin, accessStack: &accessStack, lifetime: lifetime)
+    if !lifetime, initialEffect == .escape {
       return false
     }
     forwardPropagateEffect(in: initialBlock, blockInfo: blockMap[initialBlock], effect: initialEffect,
@@ -795,15 +807,15 @@ extension LocalVariableReachableAccess {
       case .none:
         break
       case .escape:
-        if !allowEscape {
+        if !lifetime {
           break
         }
         fallthrough
       case .read, .modify, .assign:
         let firstInst = block.instructions.first!
-        currentEffect = forwardScanAccesses(after: firstInst, accessStack: &accessStack, allowEscape: allowEscape)
+        currentEffect = forwardScanAccesses(after: firstInst, accessStack: &accessStack, lifetime: lifetime)
       }
-      if !allowEscape, currentEffect == .escape {
+      if !lifetime, currentEffect == .escape {
         return false
       }
       forwardPropagateEffect(in: block, blockInfo: blockInfo, effect: currentEffect, blockList: &blockList,
@@ -836,9 +848,9 @@ extension LocalVariableReachableAccess {
   }
 
   // Check all instructions in this block after and including `begin`. Return a BlockEffect indicating the combined
-  // effects seen before stopping the scan. An .assign stops the scan. A .escape stops the scan if allowEscape is false.
+  // effects seen before stopping the scan. An .assign stops the scan. A .escape stops the scan if lifetime is false.
   private func forwardScanAccesses(after first: Instruction, accessStack: inout Stack<LocalVariableAccess>,
-                                   allowEscape: Bool)
+                                   lifetime: Bool)
     -> BlockEffect? {
     var currentEffect: BlockEffect?
     for inst in InstructionList(first: first) {
@@ -848,9 +860,12 @@ extension LocalVariableReachableAccess {
       currentEffect = BlockEffect(for: accessInfo, accessMap.context).meet(currentEffect)
       switch currentEffect! {
       case .assign:
+        if lifetime {
+          accessStack.push(accessInfo.access)
+        }
         return currentEffect
       case .escape:
-        if !allowEscape {
+        if !lifetime {
           log("Local variable: \(accessMap.allocation)\n    escapes at \(inst)")
           return currentEffect
         }

test/SILOptimizer/lifetime_dependence/lifetime_dependence_scope_fixup.swift

@@ -40,27 +40,40 @@ struct View : ~Escapable {
   }
 }
 
-struct MutableView : ~Copyable, ~Escapable {
-  let ptr: UnsafeRawBufferPointer
+struct NCMutableContainer : ~Copyable {
+  let ptr: UnsafeMutableRawBufferPointer
   let c: Int
-  @lifetime(borrow ptr)
-  init(_ ptr: UnsafeRawBufferPointer, _ c: Int) {
+  init(_ ptr: UnsafeMutableRawBufferPointer, _ c: Int) {
     self.ptr = ptr
     self.c = c
   }
+}
+
+struct MutableView : ~Copyable, ~Escapable {
+  let ptr: UnsafeMutableRawBufferPointer
+  @lifetime(borrow ptr)
+  init(_ ptr: UnsafeMutableRawBufferPointer) {
+    self.ptr = ptr
+  }
   @lifetime(copy otherBV)
-  init(_ otherBV: borrowing View) {
+  init(_ otherBV: borrowing MutableView) {
     self.ptr = otherBV.ptr
-    self.c = otherBV.c
   }
-  init(_ k: borrowing NCContainer) {
+  init(_ k: borrowing NCMutableContainer) {
     self.ptr = k.ptr
-    self.c = k.c
+  }
+}
+
+extension MutableView {
+  @lifetime(&self)
+  mutating public func update() -> Self {
+    return unsafe MutableView(ptr)
   }
 }
 
 func use(_ o : borrowing View) {}
 func mutate(_ x: inout NCContainer) { }
+func mutate(_ x: inout NCMutableContainer) { }
 func mutate(_ x: inout View) { }
 func consume(_ o : consuming View) {}
 func use(_ o : borrowing MutableView) {}
@@ -125,9 +138,9 @@ func test3(_ a: Array<Int>) {
   }
 }
 
-func test4(_ a: Array<Int>) {
-  a.withUnsafeBytes {
-    var x = NCContainer($0, a.count)
+func test4(_ a: inout Array<Int>) {
+  a.withUnsafeMutableBytes {
+    var x = NCMutableContainer($0, $0.count)
     mutate(&x)
     let view = MutableView(x)
     use(view)
@@ -180,11 +193,11 @@ func test7(_ a: UnsafeRawBufferPointer) {
   mutate(&x)
 }
 
-func test8(_ a: Array<Int>) {
-  a.withUnsafeBytes {
-    var x = View($0, a.count)
+func test8(_ a: inout Array<Int>) {
+  a.withUnsafeMutableBytes {
+    var x = View(UnsafeRawBufferPointer(start: $0.baseAddress!, count: $0.count), $0.count)
     mutate(&x)
-    let view = MutableView(x)
+    let view = MutableView($0)
     use(view)
     consume(view)
   }
@@ -245,3 +258,29 @@ func testPointeeDependenceOnMutablePointer(p: UnsafePointer<Int64>) {
   _ = ptr.pointee
   _ = ptr
 }
+
+// CHECK-LABEL: sil hidden @$s31lifetime_dependence_scope_fixup16testReassignment1bySw_tF : $@convention(thin) (UnsafeMutableRawBufferPointer) -> () {
+// CHECK: bb0(%0 : $UnsafeMutableRawBufferPointer):
+// CHECK:   [[VAR:%.*]] = alloc_stack [lexical] [var_decl] $MutableView, var, name "span", type $MutableView
+// CHECK:   apply %{{.*}}(%0, %{{.*}}) : $@convention(method) (UnsafeMutableRawBufferPointer, @thin MutableView.Type) -> @lifetime(borrow 0) @owned MutableView
+// CHECK:   [[ACCESS1:%.*]] = begin_access [modify] [static] [[VAR]] : $*MutableView
+// CHECK:   apply %{{.*}}(%{{.*}}) : $@convention(method) (@inout MutableView) -> @lifetime(borrow 0) @owned MutableView
+// CHECK:   [[LD1:%.*]] = load %{{.*}} : $*MutableView
+// CHECK:   apply %{{.*}}([[LD1]]) : $@convention(thin) (@guaranteed MutableView) -> ()
+// CHECK:   end_access [[ACCESS1]] : $*MutableView
+// CHECK:   [[ACCESS2:%.*]] = begin_access [modify] [static] [[VAR]] : $*MutableView
+// CHECK:   apply %{{.*}}(%{{.*}}) : $@convention(method) (@inout MutableView) -> @lifetime(borrow 0) @owned MutableView
+// CHECK:   [[LD2:%.*]] = load %{{.*}} : $*MutableView
+// CHECK:   apply %{{.*}}([[LD2]]) : $@convention(thin) (@guaranteed MutableView) -> ()
+// CHECK:   end_access [[ACCESS2]] : $*MutableView
+// CHECK:   destroy_addr [[VAR]] : $*MutableView
+// CHECK-LABEL: } // end sil function '$s31lifetime_dependence_scope_fixup16testReassignment1bySw_tF'
+func testReassignment(b: UnsafeMutableRawBufferPointer) {
+  var span = MutableView(b)
+
+  var sub = span.update()
+  use(sub)
+
+  sub = span.update()
+  use(sub)
+}

test/SILOptimizer/lifetime_dependence/scope_fixup.sil

@@ -43,6 +43,10 @@ struct TrivialHolder {
   var pointer: UnsafeRawPointer
 }
 
+struct A {
+  init()
+}
+
 struct Holder {
   var object: AnyObject
 }
@@ -59,6 +63,9 @@ sil [ossa] @Wrapper_init : $@convention(method) (@owned NE, @thin Wrapper.Type)
 
 sil [ossa] @NCContainer_ne_read : $@yield_once @convention(method) (@guaranteed NCContainer) -> @lifetime(borrow 0) @yields @guaranteed NE
 
+sil @yieldRawSpan : $@yield_once @convention(method) (@in_guaranteed A) -> @lifetime(borrow address_for_deps 0) @yields @guaranteed RawSpan
+sil @useRawSpan : $@convention(thin) (@guaranteed RawSpan) -> ()
+
 // NCContainer.wrapper._read:
 //   var wrapper: Wrapper {
 //     _read {
@@ -237,3 +244,35 @@ bb0(%0 : @guaranteed $Holder):
   %99 = tuple ()
   return %99 : $()
 }
+
+// FIXME: extend temporary lifetimes for coroutine access.
+//
+// CHECK-LABEL: sil [ossa] @testYieldSpan : $@convention(thin) (@in A) -> () {
+// CHECK:   alloc_stack [lexical] [var_decl] $A
+// CHECK:   (%{{.*}}, %{{.*}}) = begin_apply %{{.*}}(%{{.*}}) : $@yield_once @convention(method) (@in_guaranteed A) -> @lifetime(borrow address_for_deps 0) @yields @guaranteed RawSpan
+// CHECK:   [[MD:%.*]] = mark_dependence [unresolved]
+// CHECK:   [[CP:%.*]] = copy_value [[MD]]
+// CHECK:   apply %{{.*}}([[CP]]) : $@convention(thin) (@guaranteed RawSpan) -> ()
+// CHECK:   destroy_value [[CP]]
+// CHECK:   end_apply
+// CHECK:   destroy_addr
+// CHECK-LABEL: } // end sil function 'testYieldSpan'
+sil [ossa] @testYieldSpan : $@convention(thin) (@in A) -> () {
+bb0(%0 : $*A):
+  %1 = alloc_stack [lexical] [var_decl] $A
+  copy_addr [take] %0 to [init] %1
+
+  %3 = function_ref @yieldRawSpan : $@yield_once @convention(method) (@in_guaranteed A) -> @lifetime(borrow address_for_deps 0) @yields @guaranteed RawSpan
+  (%4, %5) = begin_apply %3(%1) : $@yield_once @convention(method) (@in_guaranteed A) -> @lifetime(borrow address_for_deps 0) @yields @guaranteed RawSpan
+  %6 = mark_dependence [unresolved] %4 on %5
+  %7 = copy_value %6
+  %8 = end_apply %5 as $()
+
+  %9 = function_ref @useRawSpan : $@convention(thin) (@guaranteed RawSpan) -> ()
+  %10 = apply %9(%7) : $@convention(thin) (@guaranteed RawSpan) -> ()
+  destroy_value %7
+  destroy_addr %1
+  dealloc_stack %1
+  %14 = tuple ()
+  return %14
+}