Merge pull request swiftlang#79841 from meg-gupta/fixsemanticarc

Fix two optimizer issues

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/DestroyHoisting.swift

@@ -88,9 +88,13 @@ private func optimize(value: Value, _ context: FunctionPassContext) {
     return
   }
 
-  var hoistableDestroys = selectHoistableDestroys(of: value, context)
+  var (foundDestroys, hoistableDestroys) = selectHoistableDestroys(of: value, context)
   defer { hoistableDestroys.deinitialize() }
 
+  guard foundDestroys else {
+    return
+  }
+
   guard var minimalLiverange = InstructionRange(withLiverangeOf: value, ignoring: hoistableDestroys, context) else {
     return
   }
@@ -99,12 +103,13 @@ private func optimize(value: Value, _ context: FunctionPassContext) {
   hoistDestroys(of: value, toEndOf: minimalLiverange, restrictingTo: &hoistableDestroys, context)
 }
 
-private func selectHoistableDestroys(of value: Value, _ context: FunctionPassContext) -> InstructionSet {
+private func selectHoistableDestroys(of value: Value, _ context: FunctionPassContext) -> (Bool, InstructionSet) {
   // Also includes liveranges of copied values and values stored to memory.
   var forwardExtendedLiverange = InstructionRange(withForwardExtendedLiverangeOf: value, context)
   defer { forwardExtendedLiverange.deinitialize() }
 
   let deadEndBlocks = context.deadEndBlocks
+  var foundDestroys = false
   var hoistableDestroys = InstructionSet(context)
 
   for use in value.uses {
@@ -114,10 +119,11 @@ private func selectHoistableDestroys(of value: Value, _ context: FunctionPassCon
        // TODO: once we have complete OSSA lifetimes we don't need to handle dead-end blocks.
        !deadEndBlocks.isDeadEnd(destroy.parentBlock)
     {
+      foundDestroys = true
       hoistableDestroys.insert(destroy)
     }
   }
-  return hoistableDestroys
+  return (foundDestroys, hoistableDestroys)
 }
 
 private func hoistDestroys(of value: Value,

lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

@@ -388,6 +388,11 @@ static bool isUseBetweenInstAndBlockEnd(
 static bool tryJoinIfDestroyConsumingUseInSameBlock(
     SemanticARCOptVisitor &ctx, CopyValueInst *cvi, DestroyValueInst *dvi,
     SILValue operand, Operand *singleCVIConsumingUse) {
+  // Pointer escapes propagate values ways that may not be discoverable.
+  // If \p cvi or \p operand has escaped, then do not optimize.
+  if (findPointerEscape(cvi) || findPointerEscape(operand)) {
+    return false;
+  }
   // First see if our destroy_value is in between singleCVIConsumingUse and the
   // end of block. If this is not true, then we know the destroy_value must be
   // /before/ our singleCVIConsumingUse meaning that by joining the lifetimes,
@@ -517,12 +522,6 @@ static bool tryJoinIfDestroyConsumingUseInSameBlock(
           }))
         return false;
   }
-  // Check whether the uses considered immediately above are all effectively
-  // instantaneous uses. Pointer escapes propagate values ways that may not be
-  // discoverable.
-  if (findPointerEscape(operand)) {
-    return false;
-  }
 
   // Ok, we now know that we can eliminate this value.
   LLVM_DEBUG(llvm::dbgs()

test/SILOptimizer/definite-init-convert-to-escape.swift

@@ -92,10 +92,12 @@ public func returnOptionalEscape() -> (() ->())?
 // NOPEEPHOLE:  switch_enum [[V1]] : $Optional<{{.*}}>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[NONE_BB:bb[0-9]+]]
 //
 // NOPEEPHOLE: [[SOME_BB]]([[V2:%.*]]: $@callee_guaranteed () -> ()):
+// NOPEEPHOLE-NEXT:  strong_retain [[V2]]
 // NOPEEPHOLE-NEXT:  [[CVT:%.*]] = convert_escape_to_noescape [[V2]]
 // NOPEEPHOLE-NEXT:  [[SOME:%.*]] = enum $Optional<{{.*}}>, #Optional.some!enumelt, [[V2]]
 // NOPEEPHOLE-NEXT:  [[MDI:%.*]] = mark_dependence [[CVT]] : $@noescape @callee_guaranteed () -> () on [[SOME]] : $Optional<@callee_guaranteed () -> ()>
 // NOPEEPHOLE-NEXT:  [[NOESCAPE_SOME:%.*]] = enum $Optional<{{.*}}>, #Optional.some!enumelt, [[MDI]]
+// NOPEEPHOLE-NEXT:  strong_release [[V2]]
 // NOPEEPHOLE-NEXT:  br bb2([[NOESCAPE_SOME]] : $Optional<{{.*}}>, [[SOME]] : $Optional<{{.*}}>, [[SOME]] : $Optional<{{.*}}>)
 //
 // NOPEEPHOLE: bb2([[NOESCAPE_SOME:%.*]] : $Optional<{{.*}}>, [[SOME1:%.*]] : $Optional<{{.*}}>, [[SOME:%.*]] : $Optional<{{.*}}>):

test/SILOptimizer/rdar146142041.sil

@@ -0,0 +1,159 @@
+// RUN: %target-sil-opt -enable-sil-verify-all %s -semantic-arc-opts -destroy-hoisting | %FileCheck %s
+
+import Swift
+
+class Klass {
+  init()
+}
+
+struct NonTrivial {
+  @_hasStorage let k: Klass { get }
+  init(k: Klass)
+}
+
+sil @foo : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
+
+// test1 is the reduction from rdar://146142041. test2 and test3 are mutations of test1 and may not be created from swift code in practice
+// CHECK-LABEL: sil [ossa] @test1 :
+// CHECK: [[PA:%.*]] = partial_apply
+// CHECK: [[COPY:%.*]] = copy_value [[PA]] 
+// CHECK-LABEL: } // end sil function 'test1'
+sil [ossa] @test1 : $@convention(thin) () -> () {
+bb0:
+  cond_br undef, bb2, bb1
+
+bb1:
+  %1 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.none!enumelt
+  br bb7(%1)
+
+bb2:
+  br bb3
+
+bb3:
+  %4 = alloc_box ${ var Optional<NonTrivial> }
+  %5 = begin_borrow [lexical] %4
+  %6 = project_box %5, 0
+  inject_enum_addr %6, #Optional.none!enumelt
+  cond_br undef, bb4, bb5
+
+bb4:
+  unreachable
+
+bb5:
+  br bb6
+
+bb6:
+  %11 = function_ref @foo : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
+  %12 = partial_apply [callee_guaranteed] %11(%6) : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
+  %13 = copy_value %12
+  %14 = convert_escape_to_noescape %13 to $@noescape @callee_guaranteed () -> ()
+  %15 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.some!enumelt, %13
+  end_borrow %5
+  destroy_value %12
+  destroy_value %4
+  destroy_value %14
+  br bb7(%15)
+
+bb7(%21 : @owned $Optional<@callee_guaranteed () -> ()>):
+  destroy_value %21
+  br bb8
+
+bb8:
+  %24 = tuple ()
+  return %24
+}
+
+// CHECK-LABEL: sil [ossa] @test2 :
+// CHECK: [[PA:%.*]] = partial_apply
+// CHECK: [[COPY:%.*]] = copy_value [[PA]] 
+// CHECK-LABEL: } // end sil function 'test2'
+sil [ossa] @test2 : $@convention(thin) () -> () {
+bb0:
+  cond_br undef, bb2, bb1
+
+bb1:
+  %1 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.none!enumelt
+  br bb7(%1)
+
+bb2:
+  br bb3
+
+bb3:
+  %4 = alloc_box ${ var Optional<NonTrivial> }
+  %5 = begin_borrow [lexical] %4
+  %6 = project_box %5, 0
+  inject_enum_addr %6, #Optional.none!enumelt
+  cond_br undef, bb4, bb5
+
+bb4:
+  unreachable
+
+bb5:
+  br bb6
+
+bb6:
+  %11 = function_ref @foo : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
+  %12 = partial_apply [callee_guaranteed] %11(%6) : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
+  %13 = copy_value %12
+  destroy_value %12
+  %14 = convert_escape_to_noescape %13 to $@noescape @callee_guaranteed () -> ()
+  %15 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.some!enumelt, %13
+  end_borrow %5
+  destroy_value %4
+  destroy_value %14
+  br bb7(%15)
+
+bb7(%21 : @owned $Optional<@callee_guaranteed () -> ()>):
+  destroy_value %21
+  br bb8
+
+bb8:
+  %24 = tuple ()
+  return %24
+}
+
+// Ensure no ownership verification error 
+sil [ossa] @test3 : $@convention(thin) () -> () {
+bb0:
+  cond_br undef, bb2, bb1
+
+bb1:
+  %1 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.none!enumelt
+  br bb7(%1)
+
+bb2:
+  br bb3
+
+bb3:
+  %4 = alloc_box ${ var Optional<NonTrivial> }
+  %5 = begin_borrow [lexical] %4
+  %6 = project_box %5, 0
+  inject_enum_addr %6, #Optional.none!enumelt
+  cond_br undef, bb4, bb5
+
+bb4:
+  unreachable
+
+bb5:
+  br bb6
+
+bb6:
+  %11 = function_ref @foo : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
+  %12 = partial_apply [callee_guaranteed] %11(%6) : $@convention(thin) (@inout_aliasable Optional<NonTrivial>) -> ()
+  %13 = copy_value %12
+  %14 = convert_escape_to_noescape %13 to $@noescape @callee_guaranteed () -> ()
+  %15 = enum $Optional<@callee_guaranteed () -> ()>, #Optional.some!enumelt, %13
+  end_borrow %5
+  destroy_value %4
+  destroy_value %12
+  destroy_value %14
+  br bb7(%15)
+
+bb7(%21 : @owned $Optional<@callee_guaranteed () -> ()>):
+  destroy_value %21
+  br bb8
+
+bb8:
+  %24 = tuple ()
+  return %24
+}