[ShrinkBorrowScope] Don't hoist over any applies.

Previously, hoisting was done over applies that were users of the
borrowed value.  Here, that is no longer done.

That hoisting was done on the theory that multiple distinct lexical
scopes were equivalent to a single enclosing lexical scope.  Dead borrow
elimination, however, means that they are not in fact the same, however:
If a callee takes an object as an argument, and in that callee that
argument is dead, FSO and inliniing (even with approaches to maintain
lexical borrow scopes when the value that is borrowed comes from an
owned argument) can result in a dead borrow scope in the caller which
could then be eliminated which would then enable the destroy_value
to be hoisted over the inlined body, including over barriers.

Author: nate-chandler

lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp

@@ -51,8 +51,6 @@ class ShrinkBorrowScope {
   ///       which is itself a deinit barrier.
   SmallPtrSet<SILBasicBlock *, 8> barredBlocks;
 
-  llvm::SmallDenseMap<ApplySite, size_t> transitiveUsesPerApplySite;
-
   // The list of blocks to look for new points at which to insert end_borrows
   // in.  A block must not be processed if all of its successors have not yet
   // been.  For that reason, it is necessary to allow the same block to be
@@ -136,12 +134,6 @@ class ShrinkBorrowScope {
     }
   }
 
-  size_t usesInApply(ApplySite apply) {
-    if (auto count = transitiveUsesPerApplySite.lookup(apply))
-      return count;
-    return 0;
-  }
-
   bool canHoistOverInstruction(SILInstruction *instruction) {
     return tryHoistOverInstruction(instruction, /*rewrite=*/false);
   }
@@ -151,36 +143,7 @@ class ShrinkBorrowScope {
       return false;
     }
     if (users.contains(instruction)) {
-      if (auto apply = ApplySite::isa(instruction)) {
-        SmallVector<int, 2> rewritableArgumentIndices;
-        auto count = apply.getNumArguments();
-        for (unsigned index = 0; index < count; ++index) {
-          auto argument = apply.getArgument(index);
-          if (canReplaceValueWithBorrowedValue(argument)) {
-            rewritableArgumentIndices.push_back(index);
-          }
-        }
-        if (rewritableArgumentIndices.size() != usesInApply(apply)) {
-          return false;
-        }
-        if (rewrite) {
-          // We can rewrite all the arguments which are transitive uses of the
-          // borrow.
-          for (auto index : rewritableArgumentIndices) {
-            auto argument = apply.getArgument(index);
-            auto borrowee = introducer->getOperand();
-            if (auto *cvi = dyn_cast<CopyValueInst>(argument)) {
-              cvi->setOperand(borrowee);
-              modifiedCopyValueInsts.push_back(cvi);
-              madeChange = true;
-            } else {
-              apply.setArgument(index, borrowee);
-              madeChange = true;
-            }
-          }
-        }
-        return true;
-      } else if (auto *bbi = dyn_cast<BeginBorrowInst>(instruction)) {
+      if (auto *bbi = dyn_cast<BeginBorrowInst>(instruction)) {
         if (bbi->isLexical() &&
             canReplaceValueWithBorrowedValue(bbi->getOperand())) {
           if (rewrite) {
@@ -237,9 +200,6 @@ bool ShrinkBorrowScope::populateUsers() {
   for (auto *use : uses) {
     auto *user = use->getUser();
     users.insert(user);
-    if (auto apply = ApplySite::isa(user)) {
-      ++transitiveUsesPerApplySite[apply];
-    }
   }
   return true;
 }

test/SILOptimizer/copy_propagation_borrow.sil

@@ -394,9 +394,11 @@ bb3:
 // CHECK:        [[ALLOC:%.*]] = alloc_ref $C
 // CHECK-NEXT:   [[B1:%.*]] = begin_borrow [[ALLOC]]
 // CHECK-NOT:    copy
+// CHECK:        [[B2:%.*]] = begin_borrow [[ALLOC]]
 // CHECK-NOT:    copy
 // CHECK:        end_borrow [[B1]] : $C
-// CHECK:        apply %0([[ALLOC]]) : $@convention(thin) (@guaranteed C) -> ()
+// CHECK:        apply %0([[B2]]) : $@convention(thin) (@guaranteed C) -> ()
+// CHECK:        end_borrow [[B2]] : $C
 // CHECK-NEXT:   return [[ALLOC]] : $C
 // CHECK-LABEL: } // end sil function 'testInterleavedBorrow'
 sil [ossa] @testInterleavedBorrow : $@convention(thin) () -> @owned C {
@@ -426,11 +428,14 @@ bb0:
 // CHECK:        [[B1:%.*]] = begin_borrow [[ALLOC]]
 // CHECK:        apply %{{.*}}([[B1]]) : $@convention(thin) (@guaranteed C) -> ()
 // CHECK-NEXT:   end_borrow [[B1]] : $C
+// CHECK-NEXT:   [[B2:%.*]] = begin_borrow [[ALLOC]]
 // CHECK-NEXT:   cond_br undef, bb1, bb2
 // CHECK:      bb1:
-// CHECK:        apply %{{.*}}([[ALLOC]]) : $@convention(thin) (@guaranteed C) -> ()
+// CHECK:        apply %{{.*}}([[B2]]) : $@convention(thin) (@guaranteed C) -> ()
+// CHECK-NEXT:   end_borrow [[B2]] : $C
 // CHECK-NEXT:   br bb3
 // CHECK:      bb2:
+// CHECK-NEXT:   end_borrow [[B2]] : $C
 // CHECK-NEXT:   br bb3
 // CHECK:      bb3:
 // CHECK-NEXT:   return [[ALLOC]] : $C

test/SILOptimizer/shrink_borrow_scope.sil

@@ -44,8 +44,10 @@ sil [ossa] @synchronization_point : $@convention(thin) () -> ()
 // Hoist over br.
 // CHECK-LABEL: sil [ossa] @hoist_over_branch_1 : {{.*}} {
 // CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
+// CHECK:         [[LIFETIME:%[^,]]] = begin_borrow [[INSTANCE]]
 // CHECK:         [[CALLEE_GUARANTEED:%[^,]+]] = function_ref @callee_guaranteed
-// CHECK:         {{%[^,]+}} = apply [[CALLEE_GUARANTEED]]([[INSTANCE]])
+// CHECK:         {{%[^,]+}} = apply [[CALLEE_GUARANTEED]]([[LIFETIME]])
+// CHECK:         end_borrow [[LIFETIME]]
 // CHECK:         br [[EXIT:bb[0-9]+]]
 // CHECK:       [[EXIT]]:
 // CHECK:         return [[INSTANCE]]
@@ -64,8 +66,10 @@ bl1:
 // Hoist over cond_br.
 // CHECK-LABEL: sil [ossa] @hoist_over_branch_2 : {{.*}} {
 // CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
+// CHECK:         [[LIFETIME:%[^,]]] = begin_borrow [[INSTANCE]]
 // CHECK:         [[CALLEE_GUARANTEED:%[^,]+]] = function_ref @callee_guaranteed
-// CHECK:         {{%[^,]+}} = apply [[CALLEE_GUARANTEED]]([[INSTANCE]])
+// CHECK:         {{%[^,]+}} = apply [[CALLEE_GUARANTEED]]([[LIFETIME]])
+// CHECK:         end_borrow [[LIFETIME]]
 // CHECK:         cond_br undef, [[BL1:bb[0-9]+]], [[BL2:bb[0-9]+]]
 // CHECK:       [[BL1]]:
 // CHECK:         br [[EXIT:bb[0-9]+]]
@@ -93,8 +97,10 @@ exit:
 // Hoist over two brs.
 // CHECK-LABEL: sil [ossa] @hoist_over_branch_3 : {{.*}} {
 // CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
+// CHECK:         [[LIFETIME:%[^,]]] = begin_borrow [[INSTANCE]]
 // CHECK:         [[CALLEE_GUARANTEED:%[^,]+]] = function_ref @callee_guaranteed
-// CHECK:         {{%[^,]+}} = apply [[CALLEE_GUARANTEED]]([[INSTANCE]])
+// CHECK:         {{%[^,]+}} = apply [[CALLEE_GUARANTEED]]([[LIFETIME]])
+// CHECK:         end_borrow [[LIFETIME]]
 // CHECK:         cond_br undef, [[BL1:bb[0-9]+]], [[BL2:bb[0-9]+]]
 // CHECK:       [[BL1]]:
 // CHECK:         br [[EXIT:bb[0-9]+]]
@@ -121,12 +127,15 @@ exit:
 // Don't hoist over 1 / 2 brs.
 // CHECK-LABEL: sil [ossa] @hoist_over_branch_4 : {{.*}} {
 // CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
+// CHECK:         [[LIFETIME:%[^,]]] = begin_borrow [[INSTANCE]]
 // CHECK:         cond_br undef, [[BL1:bb[0-9]+]], [[BL2:bb[0-9]+]]
 // CHECK:       [[BL1]]:
 // CHECK:         [[CALLEE_GUARANTEED:%[^,]+]] = function_ref @callee_guaranteed
-// CHECK:         {{%[^,]+}} = apply [[CALLEE_GUARANTEED]]([[INSTANCE]])
+// CHECK:         {{%[^,]+}} = apply [[CALLEE_GUARANTEED]]([[LIFETIME]])
+// CHECK:         end_borrow [[LIFETIME]]
 // CHECK:         br [[EXIT:bb[0-9]+]]
 // CHECK:       [[BL2]]:
+// CHECK:         end_borrow [[LIFETIME]]
 // CHECK:         br [[EXIT]]
 // CHECK:       [[EXIT]]:
 // CHECK:         return [[INSTANCE]]
@@ -149,8 +158,10 @@ exit:
 // Hoist over switch_enum destinations.
 // CHECK-LABEL: sil [ossa] @hoist_over_branch_5 : {{.*}} {
 // CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C, [[CASE:%[^,]+]] : $OneOfThree):
+// CHECK:         [[LIFETIME:%[^,]]] = begin_borrow [[INSTANCE]]
 // CHECK:         [[CALLEE_GUARANTEED:%[^,]+]] = function_ref @callee_guaranteed
-// CHECK:         {{%[0-9]+}} = apply [[CALLEE_GUARANTEED]]([[INSTANCE]])
+// CHECK:         {{%[0-9]+}} = apply [[CALLEE_GUARANTEED]]([[LIFETIME]])
+// CHECK:         end_borrow [[LIFETIME]]
 // CHECK:         switch_enum [[CASE]] : $OneOfThree, case #OneOfThree.one!enumelt: [[ONE_DEST:bb[0-9]+]], case #OneOfThree.two!enumelt: [[TWO_DEST:bb[0-9]+]], case #OneOfThree.three!enumelt: [[THREE_DEST:bb[0-9]+]]
 // CHECK:       [[ONE_DEST]]:
 // CHECK:         br [[EXIT:bb[0-9]+]]
@@ -590,11 +601,15 @@ entry(%instance : @guaranteed $C):
   return %result : $()
 }
 
-// Hoist over an apply that uses a copy of the borrow.
-// CHECK-LABEL: sil [ossa] @hoist_over_apply_at_copy : {{.*}} {
-// CHECK-NOT:     copy_value
-// CHECK-LABEL: } // end sil function 'hoist_over_apply_at_copy'
-sil [ossa] @hoist_over_apply_at_copy : $@convention(thin) (@owned C) -> () {
+// Don't hoist over an apply that uses a copy of the borrow.
+// CHECK-LABEL: sil [ossa] @dont_hoist_over_apply_at_copy : {{.*}} {
+// CHECK:         begin_borrow
+// CHECK:         copy_value
+// CHECK:         apply
+// CHECK:         end_borrow
+// CHECK:         destroy_value
+// CHECK-LABEL: } // end sil function 'dont_hoist_over_apply_at_copy'
+sil [ossa] @dont_hoist_over_apply_at_copy : $@convention(thin) (@owned C) -> () {
 entry(%instance : @owned $C):
   %synchronization_point = function_ref @synchronization_point : $@convention(thin) () -> ()
   %callee_owned = function_ref @callee_owned : $@convention(thin) (@owned C) -> ()
@@ -612,12 +627,12 @@ entry(%instance : @owned $C):
   return %result : $()
 }
 
-// Hoist over an apply that uses the borrow.
+// Don't hoist over an apply that uses the borrow.
 // CHECK-LABEL: sil [ossa] @hoist_over_apply_at_borrow : {{.*}} {
 // CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
 // CHECK:         [[LIFETIME:%[^,]+]] = begin_borrow [[INSTANCE]]
+// CHECK:         apply {{%[^,]+}}([[LIFETIME]])
 // CHECK:         end_borrow [[LIFETIME]]
-// CHECK:         apply {{%[^,]+}}([[INSTANCE]])
 // CHECK-LABEL: } // end sil function 'hoist_over_apply_at_borrow'
 sil [ossa] @hoist_over_apply_at_borrow : $@convention(thin) (@owned C) -> () {
 entry(%instance : @owned $C):
@@ -695,11 +710,18 @@ entry(%instance : @owned $C):
   return %result : $()
 }
 
-// CHECK-LABEL: sil [ossa] @hoist_over_try_apply_at_borrow : {{.*}} {
+// CHECK-LABEL: sil [ossa] @dont_hoist_over_try_apply_at_borrow : {{.*}} {
 // CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
-// CHECK-NEXT:    try_apply undef([[INSTANCE]]) : $@convention(thin) (@guaranteed C) -> @error Error, normal bb1, error bb2 
-// CHECK-LABEL: } // end sil function 'hoist_over_try_apply_at_borrow'
-sil [ossa] @hoist_over_try_apply_at_borrow : $@convention(thin) (@owned C) -> @error Error {
+// CHECK:         [[LIFETIME:%[^,]+]] = begin_borrow [[INSTANCE]]
+// CHECK:         try_apply undef([[LIFETIME]]) : {{.*}}, normal [[GOOD:bb[0-9]+]], error [[BAD:bb[0-9]+]] 
+// CHECK:       [[GOOD]]
+// CHECK:         end_borrow [[LIFETIME]]
+// CHECK:         destroy_value [[INSTANCE]]
+// CHECK:       [[BAD]]
+// CHECK:         end_borrow [[LIFETIME]]
+// CHECK:         destroy_value [[INSTANCE]]
+// CHECK-LABEL: } // end sil function 'dont_hoist_over_try_apply_at_borrow'
+sil [ossa] @dont_hoist_over_try_apply_at_borrow : $@convention(thin) (@owned C) -> @error Error {
 bb0(%instance : @owned $C):
   %lifetime = begin_borrow %instance : $C
   try_apply undef(%lifetime) : $@convention(thin) (@guaranteed C) -> @error Error, normal good, error bad

test/SILOptimizer/shrink_borrow_scope.swift

@@ -1,5 +1,7 @@
 // RUN: %target-swift-frontend -c -O -enable-copy-propagation=true -enable-lexical-lifetimes=true -sil-verify-all -Xllvm -sil-print-final-ossa-module %s | %FileCheck %s
 
+// REQUIRES: rdar87255563
+
 // =============================================================================
 // = DECLARATIONS                                                             {{
 // =============================================================================