Merge pull request swiftlang#40764 from nate-chandler/copy-propagation/shrink-borrow-scope/dont-hoist-over-any-applies

[ShrinkBorrowScope] Don't hoist over any applies.

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,48 +134,16 @@ 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);
   }
 
   bool tryHoistOverInstruction(SILInstruction *instruction, bool rewrite = true) {
+    if (instruction == introducer) {
+      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) {
@@ -234,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;
 }
@@ -298,20 +261,12 @@ void ShrinkBorrowScope::findBarriers() {
       // At that time, it was checked that this block (along with all that
       // successor's other predecessors) had a terminator over which the borrow
       // scope could be shrunk.  Shrink it now.
-#ifndef NDEBUG
-      bool hoisted = 
-#endif
-      tryHoistOverInstruction(block->getTerminator());
-#ifndef NDEBUG
+      bool hoisted = tryHoistOverInstruction(block->getTerminator());
       assert(hoisted);
-#endif
+      (void)hoisted;
     }
     SILInstruction *barrier = nullptr;
     while ((instruction = instruction->getPreviousInstruction())) {
-      if (instruction == introducer) {
-        barrier = instruction;
-        break;
-      }
       if (!tryHoistOverInstruction(instruction)) {
         barrier = instruction;
         break;

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,13 +627,13 @@ 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'
+// 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):
   %lifetime = begin_borrow %instance : $C
@@ -645,7 +660,7 @@ entry(%instance : @owned $C):
 // CHECK:         [[COPY:%[^,]+]] = copy_value [[LIFETIME]]
 // CHECK:         end_borrow [[LIFETIME]]
 // CHECK:         begin_borrow [[COPY]]
-// CHECK-LABEL: // end sil function 'hoist_over_borrow_of_copy'
+// CHECK-LABEL: } // end sil function 'hoist_over_borrow_of_copy'
 sil [ossa] @hoist_over_borrow_of_copy : $@convention(thin) (@owned C) -> () {
 entry(%instance : @owned $C):
   %lifetime_1 = begin_borrow %instance : $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
@@ -716,6 +738,74 @@ bad(%15 : @owned $Error):
   throw %15 : $Error
 }
 
+// CHECK-LABEL: sil [ossa] @dont_hoist_over_try_apply_barrier : {{.*}} {
+// CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
+// CHECK:         [[LIFETIME:%[^,]+]] = begin_borrow [[INSTANCE]]
+// CHECK:         try_apply undef() : {{.*}}, normal [[GOOD:bb[0-9]+]], error [[BAD:bb[0-9]+]]
+// CHECK:       [[GOOD]]([[REGISTER_3:%[^,]+]] : $()):
+// CHECK:         end_borrow [[LIFETIME]]
+// CHECK:         destroy_value [[INSTANCE]]
+// CHECK:       [[BAD]]([[ERROR:%[^,]+]] : @owned $Error):
+// CHECK:         end_borrow [[LIFETIME]]
+// CHECK:         destroy_value [[INSTANCE]]
+// CHECK:         throw [[ERROR]]
+// CHECK-LABEL: } // end sil function 'dont_hoist_over_try_apply_barrier'
+sil [ossa] @dont_hoist_over_try_apply_barrier : $@convention(thin) (@owned C) -> @error Error {
+bb0(%instance : @owned $C):
+  %lifetime = begin_borrow %instance : $C
+  %_ = apply undef(%lifetime) : $@convention(thin) (@guaranteed C) -> ()
+  try_apply undef() : $@convention(thin) () -> @error Error, normal good, error bad
+
+good(%8 : $()):
+  end_borrow %lifetime : $C
+  destroy_value %instance : $C
+  %13 = tuple ()
+  return %13 : $()
+
+bad(%15 : @owned $Error):
+  end_borrow %lifetime : $C
+  destroy_value %instance : $C
+  throw %15 : $Error
+}
+
+// Hoist up to two parallel barrier applies from a block with two predecessors.
+//
+// CHECK-LABEL: sil [ossa] @hoist_up_to_two_barrier_applies : $@convention(thin) (@owned C) -> () {
+// CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
+// CHECK:         [[LIFETIME:%[^,]+]] = begin_borrow [lexical] [[INSTANCE]]
+// CHECK:         cond_br undef, [[LEFT:bb[0-9]+]], [[RIGHT:bb[0-9]+]]
+// CHECK:       [[LEFT]]:
+// CHECK:         [[REGISTER_3:%[^,]+]] = apply undef()
+// CHECK:         end_borrow [[LIFETIME]]
+// CHECK:         br [[EXIT:bb[0-9]+]]
+// CHECK:       [[RIGHT]]:
+// CHECK:         [[REGISTER_6:%[^,]+]] = apply undef()
+// CHECK:         end_borrow [[LIFETIME]]
+// CHECK:         br [[EXIT]]
+// CHECK:       [[EXIT]]:
+// CHECK:         destroy_value [[INSTANCE]]
+// CHECK-LABEL: } // end sil function 'hoist_up_to_two_barrier_applies'
+sil [ossa] @hoist_up_to_two_barrier_applies : $@convention(thin) (@owned C) -> () {
+entry(%instance : @owned $C):
+  %lifetime = begin_borrow [lexical] %instance : $C
+  %_ = apply undef(%lifetime) : $@convention(thin) (@guaranteed C) -> ()
+  cond_br undef, left, right
+
+left:
+  %result_left = apply undef() : $@convention(thin) () -> ()
+  br exit
+
+right:
+  %result_right = apply undef() : $@convention(thin) () -> ()
+  br exit
+
+exit:
+  end_borrow %lifetime : $C
+  destroy_value %instance : $C
+  %retval = tuple ()
+  return %retval : $()
+}
+
 // =============================================================================
 // instruction tests                                                          }}
 // =============================================================================

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                                                             {{
 // =============================================================================