[CopyPropagation] Correctly recorded destroys.

Now that destory_values implicitly end borrow scopes for
`partial_apply [on_stack]`s, they show up as users of values whose
lifetimes are being canonicalized.  Handle that properly by
(1) only adding the `destroy_value`s whose operand is a transitive copy
    of the def to the set of destroys
(2) not considering a `destroy_value` with another operand (i.e. one
    which does not destroy a transitive copy of the def) to be lifetime
    ending

rdar://107198526

Author: nate-chandler

lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp

@@ -101,19 +101,22 @@ static void diagnose(ASTContext &Context, SourceLoc loc, Diag<T...> diag,
   Context.Diags.diagnose(loc, diag, std::forward<U>(args)...);
 }
 
-static DestroyValueInst *dynCastToDestroyOf(SILInstruction *instruction,
-                                            SILValue def) {
+/// Is \p instruction a destroy_value whose operand is \p def, or its
+/// transitive copy.
+static bool isDestroyOfCopyOf(SILInstruction *instruction, SILValue def) {
   auto *destroy = dyn_cast<DestroyValueInst>(instruction);
   if (!destroy)
-    return nullptr;
-  auto originalDestroyedDef = destroy->getOperand();
-  if (originalDestroyedDef == def)
-    return destroy;
-  auto underlyingDestroyedDef =
-      CanonicalizeOSSALifetime::getCanonicalCopiedDef(originalDestroyedDef);
-  if (underlyingDestroyedDef != def)
-    return nullptr;
-  return destroy;
+    return false;
+  auto destroyed = destroy->getOperand();
+  while (true) {
+    if (destroyed == def)
+      return true;
+    auto *copy = dyn_cast<CopyValueInst>(destroyed);
+    if (!copy)
+      break;
+    destroyed = copy->getOperand();
+  }
+  return false;
 }
 
 //===----------------------------------------------------------------------===//
@@ -172,11 +175,18 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
         liveness->updateForUse(user, /*lifetimeEnding*/ true);
         break;
       case OperandOwnership::DestroyingConsume:
-        if (isa<DestroyValueInst>(user)) {
+        if (isDestroyOfCopyOf(user, getCurrentDef())) {
           destroys.insert(user);
         } else {
-          // destroy_value does not force pruned liveness (but store etc. does).
-          liveness->updateForUse(user, /*lifetimeEnding*/ true);
+          // destroy_value of a transitive copy of the currentDef does not
+          // force pruned liveness (but store etc. does).
+
+          // Even though this instruction is a DestroyingConsume of its operand,
+          // if it's a destroy_value whose operand is not a transitive copy of
+          // currentDef, then it's just ending an implicit borrow of currentDef,
+          // not consuming it.
+          auto lifetimeEnding = !isa<DestroyValueInst>(user);
+          liveness->updateForUse(user, lifetimeEnding);
         }
         recordConsumingUse(use);
         break;

test/SILOptimizer/copy_propagation_value.sil

@@ -0,0 +1,70 @@
+// RUN: %target-sil-opt -copy-propagation -enable-sil-verify-all -module-name Swift %s | %FileCheck %s --check-prefixes=CHECK,CHECK-OPT
+// RUN: %target-sil-opt -mandatory-copy-propagation -enable-sil-verify-all -module-name Swift %s | %FileCheck %s --check-prefixes=CHECK,CHECK-ONONE
+
+// Runs CopyPropagation without borrow scope canonicalization.
+
+sil_stage canonical
+
+import Builtin
+
+typealias AnyObject = Builtin.AnyObject
+
+protocol Error {}
+
+class B { }
+
+class C {
+  var a: Builtin.Int64
+}
+
+struct NonTrivialStruct {
+  @_hasStorage var val: C { get set }
+}
+
+class CompileError : Error {}
+
+// This test case used to have an invalid boundary extension.
+// CHECK-LABEL: sil [ossa] @canonicalize_borrow_of_copy_with_interesting_boundary : $@convention(thin) (@owned C) -> (@owned NonTrivialStruct, @error any Error) {
+// CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] :
+// CHECK:         [[COPY:%[^,]+]] = copy_value [[INSTANCE]]
+// CHECK:         destroy_value [[INSTANCE]]
+// CHECK:         cond_br undef, [[SUCCESS:bb[0-9]+]], [[FAILURE:bb[0-9]+]]
+// CHECK:       [[SUCCESS]]:
+// CHECK:         [[BORROW:%[^,]+]] = begin_borrow [[COPY]]
+// CHECK:         [[STRUCT:%[^,]+]] = struct $NonTrivialStruct ([[BORROW]] : $C)
+// CHECK:         [[STRUCT_OUT:%[^,]+]] = copy_value [[STRUCT]]
+// CHECK:         end_borrow [[BORROW]]
+// CHECK:         destroy_value [[COPY]]
+// CHECK:         return [[STRUCT_OUT]]
+// CHECK:       [[FAILURE]]:
+// CHECK:         destroy_value [[COPY]]
+// CHECK:         [[BOX:%[^,]+]] = alloc_existential_box
+// CHECK:         throw [[BOX]]
+// CHECK-LABEL: } // end sil function 'canonicalize_borrow_of_copy_with_interesting_boundary'
+sil [ossa] @canonicalize_borrow_of_copy_with_interesting_boundary : $@convention(thin) (@owned C) -> (@owned NonTrivialStruct, @error any Error) {
+bb0(%0 : @owned $C):
+  %1 = copy_value %0 : $C
+  %2 = copy_value %1 : $C
+  cond_br undef, bb1, bb2
+bb1:
+  destroy_value %0 : $C
+  destroy_value %1 : $C
+  %6 = begin_borrow %2 : $C
+  %7 = struct $NonTrivialStruct (%6 : $C)
+  %8 = copy_value %7 : $NonTrivialStruct
+  end_borrow %6 : $C
+  destroy_value %2 : $C
+  return %8 : $NonTrivialStruct
+bb2:
+  destroy_value %2 : $C
+  %13 = begin_borrow %1 : $C
+  %14 = struct $NonTrivialStruct (%13 : $C)
+  %15 = copy_value %14 : $NonTrivialStruct
+  end_borrow %13 : $C
+  destroy_value %1 : $C
+  destroy_value %15 : $NonTrivialStruct
+  %19 = alloc_existential_box $any Error, $CompileError
+  destroy_value %0 : $C
+  %22 = builtin "willThrow"(%19 : $any Error) : $()
+  throw %19 : $any Error
+}