[ShrinkBorrowScope] Don't hoist over begin_borrows.

While it is sometimes valid to hoist over begin_borrows of copies of the
borrowee, it is not always valid, as the test case committed here
illustrates.  As a future optimization, we can reenable this hoisting
with the appropriate condition.

Author: nate-chandler

lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp

@@ -124,9 +124,6 @@ struct DeinitBarriers final {
   /// Blocks to "after the end" of which hoisting was able to proceed.
   BasicBlockSet hoistingReachesEndBlocks;
 
-  /// Borrows to be rewritten as borrows of %borrowee.
-  SmallVector<BeginBorrowInst *, 4> borrows;
-
   /// Copies to be rewritten as copies of %borrowee.
   SmallVector<CopyValueInst *, 4> copies;
 
@@ -153,7 +150,7 @@ class DataFlow final {
   Usage const &uses;
   DeinitBarriers &result;
 
-  enum class Classification { Barrier, Borrow, Copy, Other };
+  enum class Classification { Barrier, Copy, Other };
 
   BackwardReachability<DataFlow> reachability;
 
@@ -221,12 +218,7 @@ DataFlow::classifyInstruction(SILInstruction *instruction) {
   if (instruction == &context.introducer) {
     return Classification::Barrier;
   }
-  if (auto *bbi = dyn_cast<BeginBorrowInst>(instruction)) {
-    if (bbi->isLexical() &&
-        isSimpleExtendedIntroducerDef(context, bbi->getOperand())) {
-      return Classification::Borrow;
-    }
-  } else if (auto *cvi = dyn_cast<CopyValueInst>(instruction)) {
+  if (auto *cvi = dyn_cast<CopyValueInst>(instruction)) {
     if (isSimpleExtendedIntroducerDef(context, cvi->getOperand())) {
       return Classification::Copy;
     }
@@ -244,7 +236,6 @@ bool DataFlow::classificationIsBarrier(Classification classification) {
   switch (classification) {
   case Classification::Barrier:
     return true;
-  case Classification::Borrow:
   case Classification::Copy:
   case Classification::Other:
     return false;
@@ -259,9 +250,6 @@ void DataFlow::visitedInstruction(SILInstruction *instruction,
   case Classification::Barrier:
     result.barriers.push_back(instruction);
     return;
-  case Classification::Borrow:
-    result.borrows.push_back(cast<BeginBorrowInst>(instruction));
-    return;
   case Classification::Copy:
     result.copies.push_back(cast<CopyValueInst>(instruction));
     return;
@@ -318,10 +306,6 @@ class Rewriter final {
 bool Rewriter::run() {
   bool madeChange = false;
 
-  for (auto *bbi : barriers.borrows) {
-    bbi->setOperand(context.borrowee);
-    madeChange = true;
-  }
   for (auto *cvi : barriers.copies) {
     cvi->setOperand(context.borrowee);
     context.modifiedCopyValueInsts.push_back(cvi);

test/SILOptimizer/shrink_borrow_scope.sil

@@ -953,6 +953,30 @@ exit(%value : @owned $C):
     return %retval : $()
 }
 
+// Don't hoist over begin_borrows of copies.
+// CHECK-LABEL: sil [ossa] @dont_hoist_over_begin_borrow_copy : {{.*}} {
+// CHECK:         begin_borrow
+// CHECK:         copy_value
+// CHECK:         begin_borrow
+// CHECK:         end_borrow
+// CHECK-LABEL: } // end sil function 'dont_hoist_over_begin_borrow_copy'
+sil [ossa] @dont_hoist_over_begin_borrow_copy : $@convention(thin) (@owned C) -> (@owned C) {
+entry(%owned_in : @owned $C):
+  %borrow_1 = begin_borrow [lexical] %owned_in : $C
+  apply undef(%borrow_1) : $@convention(thin) (@guaranteed C) -> ()
+  %copy_1_1 = copy_value %borrow_1 : $C
+  %borrow_out = begin_borrow [lexical] %copy_1_1 : $C
+  %copy_2 = copy_value %borrow_out : $C
+  end_borrow %borrow_1 : $C
+  destroy_value %owned_in : $C
+  apply undef(%borrow_out) : $@convention(thin) (@guaranteed C) -> ()
+  br exit(%copy_1_1 : $C, %borrow_out : $C, %copy_2 : $C)
+exit(%copy_1_2 : @owned $C, %borrow_in : @guaranteed $C, %copy_2_2 : @owned $C):
+  end_borrow %borrow_in : $C
+  destroy_value %copy_1_2 : $C
+  return %copy_2_2 : $C
+}
+
 // =============================================================================
 // instruction tests                                                          }}
 // =============================================================================