Merge pull request #80979 from atrick/rdar149397018-optional-return

LifetimeDependenceScopeFixup: handle returning dependent Optional

Author: atrick

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift

@@ -762,13 +762,20 @@ extension ExtendableScope {
     defer { unusedEnds.deinitialize() }
     for end in range.ends {
       let location = end.location.autoGenerated
-      if end is ReturnInst {
+      switch end {
+      case is BranchInst:
+        assert(end.parentBlock.singleSuccessor!.terminator is ReturnInst,
+               "a phi only ends a use range if it is a returned value")
+        fallthrough
+      case is ReturnInst:
         // End this inner scope just before the return. The mark_dependence base operand will be redirected to a
         // function argument.
         let builder = Builder(before: end, location: location, context)
         // Insert newEnd so that this scope will be nested in any outer scopes.
         range.insert(createEndInstruction(builder, context))
         continue
+      default:
+        break
       }
       if unusedEnds.contains(end) {
         unusedEnds.erase(end)

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

@@ -587,7 +587,7 @@ extension LifetimeDependenceDefUseWalker {
   // Override ForwardingDefUseWalker.
   mutating func walkDown(operand: Operand) -> WalkResult {
     // Initially delegate all uses to OwnershipUseVisitor.
-    // walkDownDefault will be called for uses that forward ownership.
+    // forwardingUse() will be called for uses that forward ownership, which then delegates to walkDownDefault().
     return classify(operand: operand)
   }
 
@@ -665,6 +665,11 @@ extension LifetimeDependenceDefUseWalker {
 
   mutating func forwardingUse(of operand: Operand, isInnerLifetime: Bool)
     -> WalkResult {
+    // Lifetime dependence is only interested in dominated uses. Treat a returned phi like a dominated use by stopping
+    // at the phi operand rather than following the forwarded value to the ReturnInst.
+    if let phi = Phi(using: operand), phi.isReturnValue {
+      return returnedDependence(result: operand)
+    }
     // Delegate ownership forwarding operations to the ForwardingDefUseWalker.
     return walkDownDefault(forwarding: operand)
   }

SwiftCompilerSources/Sources/SIL/Argument.swift

@@ -192,6 +192,22 @@ public struct Phi {
   }
 }
 
+extension Phi {
+  /// Return true of this phi is directly returned with no side effects between the phi and the return.
+  public var isReturnValue: Bool {
+    if let singleUse = value.uses.singleUse, let ret = singleUse.instruction as? ReturnInst,
+       ret.parentBlock == successor {
+      for inst in successor.instructions {
+        if inst.mayHaveSideEffects {
+          return false
+        }
+      }
+      return true
+    }
+    return false
+  }
+}
+
 extension Operand {
   public var forwardingBorrowedFromUser: BorrowedFromInst? {
     if let bfi = instruction as? BorrowedFromInst, index == 0 {

test/SILOptimizer/lifetime_dependence/scope_fixup.sil

@@ -64,7 +64,8 @@ sil @getNEPointerToA : $@convention(thin) (@guaranteed NE) -> UnsafePointer<A>
 sil @useA : $@convention(thin) (A) -> ()
 
 sil @getPtr : $@convention(thin) () -> @out UnsafeRawPointer
-sil @getSpan : $@convention(thin) (@in_guaranteed AnyObject) -> @lifetime(borrow 0) @out NE
+
+sil @getOwnedNE : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned NE
 
 sil @useNE : $@convention(thin) (@guaranteed NE) -> ()
 
@@ -439,3 +440,43 @@ bb0(%0 : @owned $B):
   %24 = tuple ()
   return %24
 }
+
+// =============================================================================
+// Return value extension
+// =============================================================================
+
+// Sink the end access to the "return" phi and rewrite mark_dependence on the argument.
+//
+// CHECK-LABEL: sil hidden [ossa] @testReturnPhi : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned Optional<NE> {
+// CHECK: bb0(%0 : $*Holder):
+// CHECK: bb1:
+// CHECK:   enum $Optional<NE>, #Optional.none!enumelt
+// CHECK: bb2:
+// CHECK:   [[ACCESS:%.*]] = begin_access [modify] [unknown] %0
+// CHECK:   [[MD:%.*]] = mark_dependence [unresolved] {{%.*}} on %0
+// CHECK:   [[OPT:%.*]] = enum $Optional<NE>, #Optional.some!enumelt, %7
+// CHECK:   end_access [[ACCESS]]
+// CHECK:   br bb3([[OPT]])
+// CHECK: bb3([[RET:%.*]] : @owned $Optional<NE>):
+// CHECK:   return [[RET]]
+// CHECK-LABEL: } // end sil function 'testReturnPhi'
+sil hidden [ossa] @testReturnPhi : $@convention(thin) (@inout Holder) -> @lifetime(borrow 0) @owned Optional<NE> {
+bb0(%0 : $*Holder):
+  cond_br undef, bb1, bb2
+
+bb1:
+  %none = enum $Optional<NE>, #Optional.none!enumelt
+  br bb3(%none)
+
+bb2:
+  %access = begin_access [modify] [unknown] %0
+  %f = function_ref @getOwnedNE : $@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
+  %some = enum $Optional<NE>, #Optional.some!enumelt, %md
+  br bb3(%some)
+
+bb3(%ret : @owned $Optional<NE>):
+  return %ret
+}

test/SILOptimizer/lifetime_dependence/verify_diagnostics.swift

@@ -0,0 +1,25 @@
+// RUN: %target-swift-frontend %s -emit-sil \
+// RUN:   -o /dev/null \
+// RUN:   -verify \
+// RUN:   -sil-verify-all \
+// RUN:   -enable-experimental-feature LifetimeDependence
+
+// REQUIRES: swift_in_compiler
+// REQUIRES: swift_feature_LifetimeDependence
+
+// Test diagnostic output for interesting corner cases. Similar to semantics.swift, but this tests corner cases in the
+// implementation as opposed to basic language rules.
+
+// Test that conditionally returning an Optional succeeds.
+//
+// See scope_fixup.sil: testReturnPhi.
+@available(SwiftStdlib 6.2, *)
+extension Array {
+  @lifetime(&self)
+  mutating func dumb() -> MutableSpan<Element>? {
+    if count == 0 {
+      return nil
+    }
+    return mutableSpan
+  }
+}