[ShrinkBorrowScope] Checked before adding preds.

Previously, after determining that a block was dead, all its
predecessors were added to the worklist.  That is a problem, with the
current algorithm, if any of the predecessors have a terminator over
which the borrow scope cannot be shrunk.  Here, before adding a dead
block's predecessors to the worklist of blocks through which to shrink,
ensure that all those predecessors have terminators over which the scope
can be shrunk.  Finally, when beginning to process a block, if it
doesn't have a starting instruction, first hoist over thet terminator.

Author: nate-chandler

lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp

@@ -298,14 +298,6 @@ void ShrinkBorrowScope::findBarriers() {
     if (!startingInstruction && !hasOnlyDeadSuccessors(block)) {
       continue;
     }
-    if (!startingInstruction &&
-        !tryHoistOverInstruction(block->getTerminator())) {
-      // This block was walked to--it was not one containing one of the initial
-      // end_borrow instructions.  Check whether it forwards the ownership of
-      // the borrowed value (either directly or indirectly).  If it does, we
-      // must not hoist the end_borrow above it.
-      continue;
-    }
     for (auto *successor : block->getSuccessorBlocks()) {
       barredBlocks.erase(successor);
     }
@@ -317,6 +309,14 @@ void ShrinkBorrowScope::findBarriers() {
     // contained an original scope-ending instruction, start scanning from it.
     SILInstruction *instruction =
         startingInstruction ? startingInstruction : block->getTerminator();
+    if (!startingInstruction) {
+      // That there's no starting instruction means that this this block did not
+      // contain an original introducer.  It was added to the worklist later.
+      // 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.
+      assert(tryHoistOverInstruction(block->getTerminator()));
+    }
     SILInstruction *barrier = nullptr;
     while ((instruction = getPreviousInstruction(instruction))) {
       if (instruction == introducer) {
@@ -334,8 +334,16 @@ void ShrinkBorrowScope::findBarriers() {
     } else {
       deadBlocks.insert(block);
       barredBlocks.insert(block);
-      for (auto *predecessor : block->getPredecessorBlocks()) {
-        worklist.push_back(predecessor);
+      // If any of block's predecessor has a terminator over which the scope 
+      // can't be shrunk, the scope is barred from shrinking out of this block.
+      if (llvm::all_of(block->getPredecessorBlocks(), [&](auto *block) {
+            return canHoistOverInstruction(block->getTerminator());
+            })) {
+        // Otherwise, add all predecessors to the worklist and attempt to shrink
+        // the borrow scope through them.
+        for (auto *predecessor : block->getPredecessorBlocks()) {
+          worklist.push_back(predecessor);
+        }
       }
     }
   }

test/SILOptimizer/shrink_borrow_scope.sil

@@ -313,6 +313,38 @@ exit:
     return %instance_c : $C
 }
 
+// CHECK-LABEL: sil [ossa] @dont_hoist_over_some_barred_blocks : {{.*}} {
+// CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
+// CHECK:         [[LIFETIME:%[^,]+]] = begin_borrow [[INSTANCE]] : $C
+// CHECK:         cond_br undef, [[LEFT:bb[0-9]+]], [[RIGHT:bb[0-9]+]]
+// CHECK:       [[LEFT]]:
+// CHECK:         br [[EXIT:bb[0-9]+]](undef : $C)
+// CHECK:       [[RIGHT]]:
+// CHECK:         [[COPY:%[^,]+]] = copy_value [[LIFETIME]] : $C
+// CHECK:         br [[EXIT]]([[COPY]] : $C)
+// CHECK:       [[EXIT]]
+// CHECK:           end_borrow [[LIFETIME]] : $C
+// CHECK-LABEL: } // end sil function 'dont_hoist_over_some_barred_blocks'
+sil [ossa] @dont_hoist_over_some_barred_blocks : $(@owned C) -> () {
+entry(%instance : @owned $C):
+    %borrow = begin_borrow %instance : $C
+    cond_br undef, left, right
+
+left:
+    br exit(undef : $C)
+
+right:
+    %copy = copy_value %borrow : $C
+    br exit(%copy : $C)
+
+exit(%thing : @owned $C):
+    end_borrow %borrow : $C
+    destroy_value %thing : $C
+    destroy_value %instance : $C
+    %retval = tuple ()
+    return %retval : $()
+}
+
 // =============================================================================
 // branching tests                                                            }}
 // =============================================================================
@@ -663,6 +695,27 @@ entry(%instance : @owned $C):
   return %result : $()
 }
 
+// CHECK-LABEL: sil [ossa] @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 {
+bb0(%instance : @owned $C):
+  %lifetime = begin_borrow %instance : $C
+  try_apply undef(%lifetime) : $@convention(thin) (@guaranteed C) -> @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
+}
+
 // =============================================================================
 // instruction tests                                                          }}
 // =============================================================================