Merge pull request #28235 from gottesmm/pr-b17541a0a4d7b1ca7d40f3c80835d603a5698adc

Author: swift-ci

lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp

@@ -406,24 +406,38 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
   // dead end blocks that use the value in a non-consuming way.
   //
   // TODO: There may be some way of sinking this into the loop below.
-  if (destroys.empty() &&
-      llvm::any_of(borrowScopeIntroducers,
-                   [](BorrowScopeIntroducingValue borrowScope) {
-                     return borrowScope.isLocalScope();
-                   })) {
+  bool haveAnyLocalScopes = llvm::any_of(
+      borrowScopeIntroducers, [](BorrowScopeIntroducingValue borrowScope) {
+        return borrowScope.isLocalScope();
+      });
+
+  if (destroys.empty() && haveAnyLocalScopes) {
     return false;
   }
 
   // If we reached this point, then we know that all of our users can accept a
-  // guaranteed value and our owned value is destroyed only by
-  // destroy_value. Check if all of our destroys are joint post-dominated by the
-  // our end borrow scope set. If they do not, then the copy_value is lifetime
-  // extending the guaranteed value, we can not eliminate it.
+  // guaranteed value and our owned value is destroyed only by a set of
+  // destroy_values. Check if:
+  //
+  // 1. All of our destroys are joint post-dominated by our end borrow scope
+  //    set. If they do not, then the copy_value is lifetime extending the
+  //    guaranteed value, we can not eliminate it.
+  //
+  // 2. If all of our destroy_values are dead end. In such a case, the linear
+  //    lifetime checker will not perform any checks since it assumes that dead
+  //    end destroys can be ignored. Since we are going to end the program
+  //    anyways, we want to be conservative here and optimize only if we do not
+  //    need to insert an end_borrow since all of our borrow introducers are
+  //    non-local scopes.
   {
     SmallVector<BranchPropagatedUser, 8> destroysForLinearLifetimeCheck;
+    bool foundNonDeadEnd = false;
     for (auto *dvi : destroys) {
+      foundNonDeadEnd |= !getDeadEndBlocks().isDeadEnd(dvi->getParent());
       destroysForLinearLifetimeCheck.push_back(&dvi->getAllOperands()[0]);
     }
+    if (!foundNonDeadEnd && haveAnyLocalScopes)
+      return false;
     SmallVector<BranchPropagatedUser, 8> scratchSpace;
     SmallPtrSet<SILBasicBlock *, 4> visitedBlocks;
     if (llvm::any_of(borrowScopeIntroducers,

test/SILOptimizer/semantic-arc-opts.sil

@@ -824,8 +824,8 @@ bb0:
   unreachable
 }
 
-// Make sure we do perform the optimization if our borrowed value is an
-// argument.
+// Make sure that since we have a guaranteed argument and do not need to reason
+// about end_borrows, we handle this.
 //
 // CHECK-LABEL: sil [ossa] @guaranteed_arg_used_by_postdominating_no_return_function : $@convention(thin) (@guaranteed NativeObjectPair) -> MyNever {
 // CHECK-NOT: copy_value
@@ -839,6 +839,23 @@ bb0(%0 : @guaranteed $NativeObjectPair):
   unreachable
 }
 
+
+// Make sure that since our borrow introducer is a begin_borrow, we do not
+// eliminate the copy.
+//
+// CHECK-LABEL: sil [ossa] @borrowed_val_used_by_postdominating_no_return_function : $@convention(thin) (@owned NativeObjectPair) -> MyNever {
+// CHECK: copy_value
+// CHECK: } // end sil function 'borrowed_val_used_by_postdominating_no_return_function'
+sil [ossa] @borrowed_val_used_by_postdominating_no_return_function : $@convention(thin) (@owned NativeObjectPair) -> MyNever {
+bb0(%0 : @owned $NativeObjectPair):
+  %1 = begin_borrow %0 : $NativeObjectPair
+  %2 = struct_extract %1 : $NativeObjectPair, #NativeObjectPair.obj1
+  %3 = copy_value %2 : $Builtin.NativeObject
+  %func = function_ref @unreachable_guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
+  apply %func(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
+  unreachable
+}
+
 // Just make sure that we do not crash on this. We should be able to eliminate
 // everything here.
 //
@@ -855,4 +872,31 @@ bb0(%0 : @guaranteed $NativeObjectPair):
   destroy_value %2 : $Builtin.NativeObject
   %9999 = tuple()
   return %9999 : $()
-}
+}
+
+// Just make sure we do not crash here.
+//
+// CHECK-LABEL: sil [ossa] @do_not_insert_end_borrow_given_deadend : $@convention(thin) (@guaranteed ClassLet) -> () {
+// CHECK: copy_value
+// CHECK: } // end sil function 'do_not_insert_end_borrow_given_deadend'
+sil [ossa] @do_not_insert_end_borrow_given_deadend : $@convention(thin) (@guaranteed ClassLet) -> () {
+bb0(%x : @guaranteed $ClassLet):
+  %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
+  %p = ref_element_addr %x : $ClassLet, #ClassLet.aLet
+  %v = load_borrow %p : $*Klass
+  %c = copy_value %v : $Klass
+  end_borrow %v : $Klass
+  apply %f(%c) : $@convention(thin) (@guaranteed Klass) -> ()
+  cond_br undef, bb1, bb2
+
+bb1:
+  destroy_value %c : $Klass
+  br bb3
+
+bb2:
+  destroy_value %c : $Klass
+  br bb3
+
+bb3:
+  unreachable
+}