[CanonicalizeBorrowScope] Look through moves.

When encountering inside a borrow scope a non-lexical move_value or a
move_value [lexical] where the borrowed value is itself already lexical,
delete the move_value and regard its uses as uses of the moved-from
value.

Author: nate-chandler

include/swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h

@@ -109,7 +109,7 @@
 
 namespace swift {
 
-extern llvm::Statistic NumCopiesEliminated;
+extern llvm::Statistic NumCopiesAndMovesEliminated;
 extern llvm::Statistic NumCopiesGenerated;
 
 /// Insert a copy on this operand. Trace and update stats.

lib/SILOptimizer/UtilityPasses/UnitTestRunner.cpp

@@ -393,6 +393,23 @@ struct CanonicalizeOSSALifetimeTest : UnitTest {
   }
 };
 
+// Arguments:
+// - SILValue: value to canonicalize
+// Dumps:
+// - function after value canonicalization
+struct CanonicalizeBorrowScopeTest : UnitTest {
+  CanonicalizeBorrowScopeTest(UnitTestRunner *pass) : UnitTest(pass) {}
+  void invoke(Arguments &arguments) override {
+    auto value = arguments.takeValue();
+    auto borrowedValue = BorrowedValue(value);
+    assert(borrowedValue && "specified value isn't a BorrowedValue!?");
+    InstructionDeleter deleter;
+    CanonicalizeBorrowScope canonicalizer(deleter);
+    canonicalizer.canonicalizeBorrowScope(borrowedValue);
+    getFunction()->dump();
+  }
+};
+
 // Arguments:
 // - instruction
 // Dumps:
@@ -616,6 +633,8 @@ void UnitTestRunner::withTest(StringRef name, Doit doit) {
 
     // Alphabetical mapping from string to unit test subclass.
     ADD_UNIT_TEST_SUBCLASS("canonicalize-ossa-lifetime", CanonicalizeOSSALifetimeTest)
+    ADD_UNIT_TEST_SUBCLASS("canonicalize-borrow-scope",
+                           CanonicalizeBorrowScopeTest)
     ADD_UNIT_TEST_SUBCLASS("dump-function", DumpFunction)
     ADD_UNIT_TEST_SUBCLASS("find-borrow-introducers", FindBorrowIntroducers)
     ADD_UNIT_TEST_SUBCLASS("find-enclosing-defs", FindEnclosingDefsTest)

lib/SILOptimizer/Utils/CanonicalizeBorrowScope.cpp

@@ -44,16 +44,24 @@ static bool hasValueOwnership(SILValue value) {
          value->getOwnershipKind() == OwnershipKind::Owned;
 }
 
-/// Delete a chain of unused copies leading to \p v.
-static void deleteCopyChain(SILValue v, InstructionDeleter &deleter) {
-  while (auto *copy = dyn_cast<CopyValueInst>(v)) {
-    if (!onlyHaveDebugUses(copy))
+static SingleValueInstruction *asCopyOrMove(SILValue v) {
+  if (auto *copy = dyn_cast<CopyValueInst>(v))
+    return copy;
+  if (auto *move = dyn_cast<MoveValueInst>(v))
+    return move;
+  return nullptr;
+}
+
+/// Delete a chain of unused copies and moves leading to \p v.
+static void deleteCopyAndMoveChain(SILValue v, InstructionDeleter &deleter) {
+  while (auto *inst = asCopyOrMove(v)) {
+    if (!onlyHaveDebugUses(inst))
       break;
 
-    v = copy->getOperand();
-    LLVM_DEBUG(llvm::dbgs() << "  Deleting " << *copy);
-    ++NumCopiesEliminated;
-    deleter.forceDelete(copy);
+    v = inst->getOperand(CopyLikeInstruction::Src);
+    LLVM_DEBUG(llvm::dbgs() << "  Deleting " << *inst);
+    ++NumCopiesAndMovesEliminated;
+    deleter.forceDelete(inst);
   }
 }
 
@@ -182,11 +190,12 @@ bool CanonicalizeBorrowScope::computeBorrowLiveness() {
 /// equivalent to the logic in visitBorrowScopeUses that recurses through
 /// copies. The use-def and def-use logic must be consistent.
 SILValue CanonicalizeBorrowScope::findDefInBorrowScope(SILValue value) {
-  while (auto *copy = dyn_cast<CopyValueInst>(value)) {
-    if (isPersistentCopy(copy))
+  while (auto *inst = asCopyOrMove(value)) {
+    auto *copy = dyn_cast<CopyValueInst>(inst);
+    if (copy && isPersistentCopy(copy))
       return copy;
 
-    value = copy->getOperand();
+    value = inst->getOperand(0);
   }
   return value;
 }
@@ -224,21 +233,27 @@ bool CanonicalizeBorrowScope::visitBorrowScopeUses(SILValue innerValue,
     // Gather the uses before updating any of them.
     // 'value' may be deleted in this loop after rewriting its last use.
     // 'use' may become invalid after processing its user.
+
     SmallVector<Operand *, 4> uses(value->getUses());
-    for (Operand *use : uses) {
+    for (auto *use : uses) {
       auto *user = use->getUser();
       // Incidental uses, such as debug_value may be deleted before they can be
       // processed. Their user will now be nullptr. This means that value
       // is dead, so just bail.
       if (!user)
         break;
 
-      // Recurse through copies.
+      // Recurse through copies and moves.
       if (auto *copy = dyn_cast<CopyValueInst>(user)) {
         if (!isPersistentCopy(copy)) {
           defUseWorklist.insert(copy);
           continue;
         }
+      } else if (auto *move = dyn_cast<MoveValueInst>(user)) {
+        if (!move->isLexical() || innerValue->isLexical()) {
+          defUseWorklist.insert(move);
+          continue;
+        }
       }
       // Note: debug_value uses are handled like normal uses here. If they are
       // inner uses, they remain. If they are outer uses, then they will be
@@ -433,7 +448,7 @@ class RewriteInnerBorrowUses {
     // destroys are never needed within a borrow scope.
     if (isa<DestroyValueInst>(user)) {
       scope.getDeleter().forceDelete(user);
-      deleteCopyChain(value, scope.getDeleter());
+      deleteCopyAndMoveChain(value, scope.getDeleter());
       return true;
     }
     SILValue def = scope.findDefInBorrowScope(value);
@@ -447,12 +462,12 @@ class RewriteInnerBorrowUses {
         use->set(def);
         copyLiveUse(use, scope.getCallbacks());
       }
-      deleteCopyChain(value, scope.getDeleter());
+      deleteCopyAndMoveChain(value, scope.getDeleter());
       return true;
     }
     // Non-consuming use.
     use->set(def);
-    deleteCopyChain(value, scope.getDeleter());
+    deleteCopyAndMoveChain(value, scope.getDeleter());
     return true;
   }
 
@@ -474,7 +489,7 @@ class RewriteInnerBorrowUses {
     use->set(scope.findDefInBorrowScope(value));
     ForwardingOperand(use).setForwardingOwnershipKind(
         OwnershipKind::Guaranteed);
-    deleteCopyChain(value, scope.getDeleter());
+    deleteCopyAndMoveChain(value, scope.getDeleter());
     return true;
   }
 };
@@ -593,7 +608,7 @@ class RewriteOuterBorrowUses {
       ForwardingOperand(use).setForwardingOwnershipKind(
           OwnershipKind::Guaranteed);
     }
-    deleteCopyChain(innerValue, scope.getDeleter());
+    deleteCopyAndMoveChain(innerValue, scope.getDeleter());
     return true;
   }
 
@@ -644,7 +659,7 @@ class RewriteOuterBorrowUses {
 
     use->set(outerValue);
 
-    deleteCopyChain(innerValue, scope.getDeleter());
+    deleteCopyAndMoveChain(innerValue, scope.getDeleter());
 
     recordOuterUse(use);
   };

lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp

@@ -78,9 +78,9 @@
 using namespace swift;
 using llvm::SmallSetVector;
 
-llvm::Statistic swift::NumCopiesEliminated = {
-    DEBUG_TYPE, "NumCopiesEliminated",
-    "number of copy_value instructions removed"};
+llvm::Statistic swift::NumCopiesAndMovesEliminated = {
+    DEBUG_TYPE, "NumCopiesAndMovesEliminated",
+    "number of copy_value and move_value instructions removed"};
 
 llvm::Statistic swift::NumCopiesGenerated = {
     DEBUG_TYPE, "NumCopiesGenerated",
@@ -970,7 +970,7 @@ void CanonicalizeOSSALifetime::rewriteCopies() {
       } else {
         if (instsToDelete.insert(srcCopy)) {
           LLVM_DEBUG(llvm::dbgs() << "  Removing " << *srcCopy);
-          ++NumCopiesEliminated;
+          ++NumCopiesAndMovesEliminated;
         }
       }
     }

test/SILOptimizer/canonicalize_borrow_scope_unit.sil

@@ -0,0 +1,59 @@
+// RUN: %target-sil-opt -unit-test-runner %s -o /dev/null 2>&1 | %FileCheck %s
+
+import Builtin
+
+typealias AnyObject = Builtin.AnyObject
+
+struct Unmanaged<Instance> where Instance : AnyObject {
+  unowned(unsafe) var _value: @sil_unmanaged Instance
+}
+
+// CHECK-LABEL: begin {{.*}} on copy_and_move_argument: canonicalize-borrow-scope
+// CHECK-LABEL: sil [ossa] @copy_and_move_argument : {{.*}} {
+// CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] :
+// CHECK:         [[UNMANAGED:%[^,]+]] = ref_to_unmanaged [[INSTANCE]]
+// CHECK:         [[RETVAL:%[^,]+]] = struct $Unmanaged<Instance> ([[UNMANAGED]] : $@sil_unmanaged Instance) 
+// CHECK:         return [[RETVAL]]
+// CHECK-LABEL: } // end sil function 'copy_and_move_argument'
+// CHECK-LABEL: end {{.*}} on copy_and_move_argument: canonicalize-borrow-scope
+sil [ossa] @copy_and_move_argument : $@convention(thin) <Instance where Instance : AnyObject> (@guaranteed Instance) -> Unmanaged<Instance> {
+bb0(%instance : @guaranteed $Instance):
+  test_specification "canonicalize-borrow-scope @argument"
+  %copy_1 = copy_value %instance : $Instance
+  %copy_2 = copy_value %copy_1 : $Instance
+  %move = move_value %copy_2 : $Instance
+  %copy_3 = copy_value %move : $Instance
+  %copy_4 = copy_value %copy_3 : $Instance
+  %unmanaged = ref_to_unmanaged %copy_4 : $Instance to $@sil_unmanaged Instance
+  destroy_value %copy_4 : $Instance
+  destroy_value %copy_3 : $Instance
+  destroy_value %move : $Instance
+  destroy_value %copy_1 : $Instance
+  %retval = struct $Unmanaged<Instance> (%unmanaged : $@sil_unmanaged Instance)
+  return %retval : $Unmanaged<Instance>
+}
+
+// CHECK-LABEL: begin {{.*}} on copy_and_move_lexical_argument: canonicalize-borrow-scope
+// CHECK-LABEL: sil [ossa] @copy_and_move_lexical_argument : {{.*}} {
+// CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] :
+// CHECK:         [[UNMANAGED:%[^,]+]] = ref_to_unmanaged [[INSTANCE]]
+// CHECK:         [[RETVAL:%[^,]+]] = struct $Unmanaged<Instance> ([[UNMANAGED]] : $@sil_unmanaged Instance) 
+// CHECK:         return [[RETVAL]]
+// CHECK-LABEL: } // end sil function 'copy_and_move_lexical_argument'
+// CHECK-LABEL: end {{.*}} on copy_and_move_lexical_argument: canonicalize-borrow-scope
+sil [ossa] @copy_and_move_lexical_argument : $@convention(thin) <Instance where Instance : AnyObject> (@guaranteed Instance) -> Unmanaged<Instance> {
+bb0(%instance : @guaranteed $Instance):
+  test_specification "canonicalize-borrow-scope @argument"
+  %copy_1 = copy_value %instance : $Instance
+  %copy_2 = copy_value %copy_1 : $Instance
+  %move = move_value [lexical] %copy_2 : $Instance
+  %copy_3 = copy_value %move : $Instance
+  %copy_4 = copy_value %copy_3 : $Instance
+  %unmanaged = ref_to_unmanaged %copy_4 : $Instance to $@sil_unmanaged Instance
+  destroy_value %copy_4 : $Instance
+  destroy_value %copy_3 : $Instance
+  destroy_value %move : $Instance
+  destroy_value %copy_1 : $Instance
+  %retval = struct $Unmanaged<Instance> (%unmanaged : $@sil_unmanaged Instance)
+  return %retval : $Unmanaged<Instance>
+}