Disable handling of nested borrows in DCE

Nested borrow handling can be complex in the presence of reborrows. So
it is not handled currently.

Author: meg-gupta

include/swift/SIL/OwnershipUtils.h

@@ -1009,6 +1009,12 @@ void findTransitiveReborrowBaseValuePairs(
     BorrowingOperand initialScopeOperand, SILValue origBaseValue,
     function_ref<void(SILPhiArgument *, SILValue)> visitReborrowBaseValuePair);
 
+/// Given a begin_borrow visit all end_borrow users of the borrow or its
+/// reborrows.
+void visitTransitiveEndBorrows(
+    BeginBorrowInst *borrowInst,
+    function_ref<void(EndBorrowInst *)> visitEndBorrow);
+
 } // namespace swift
 
 #endif

lib/SIL/Utils/OwnershipUtils.cpp

@@ -1297,3 +1297,25 @@ void swift::findTransitiveReborrowBaseValuePairs(
     });
   }
 }
+
+void swift::visitTransitiveEndBorrows(
+    BeginBorrowInst *borrowInst,
+    function_ref<void(EndBorrowInst *)> visitEndBorrow) {
+  SmallSetVector<SILValue, 4> worklist;
+  worklist.insert(borrowInst);
+
+  while (!worklist.empty()) {
+    auto val = worklist.pop_back_val();
+    for (auto *consumingUse : val->getConsumingUses()) {
+      auto *consumingUser = consumingUse->getUser();
+      if (auto *branch = dyn_cast<BranchInst>(consumingUser)) {
+        auto *succBlock = branch->getSingleSuccessorBlock();
+        auto *phiArg = cast<SILPhiArgument>(
+            succBlock->getArgument(consumingUse->getOperandNumber()));
+        worklist.insert(phiArg);
+      } else {
+        visitEndBorrow(cast<EndBorrowInst>(consumingUser));
+      }
+    }
+  }
+}

lib/SILOptimizer/Transforms/DeadCodeElimination.cpp

@@ -44,11 +44,10 @@ namespace {
 // FIXME: Reconcile the similarities between this and
 //        isInstructionTriviallyDead.
 static bool seemsUseful(SILInstruction *I) {
-  // Even though begin_access/begin_borrow/destroy_value/copy_value have
-  // side-effects, they can be DCE'ed if they do not have useful
-  // dependencies/reverse dependencies
-  if (isa<BeginAccessInst>(I) || isa<BeginBorrowInst>(I) ||
-      isa<CopyValueInst>(I) || isa<DestroyValueInst>(I))
+  // Even though begin_access/destroy_value/copy_value have side-effects, they
+  // can be DCE'ed if they do not have useful dependencies/reverse dependencies
+  if (isa<BeginAccessInst>(I) || isa<CopyValueInst>(I) ||
+      isa<DestroyValueInst>(I))
     return false;
 
   // A load [copy] is okay to be DCE'ed if there are no useful dependencies
@@ -266,6 +265,21 @@ void DCE::markLive() {
         break;
       }
       case SILInstructionKind::BeginBorrowInst: {
+        // Currently we only support borrows of owned values.
+        // Nested borrow handling can be complex in the presence of reborrows.
+        // So it is not handled currently.
+        auto *borrowInst = cast<BeginBorrowInst>(&I);
+        if (borrowInst->getOperand().getOwnershipKind() !=
+            OwnershipKind::Owned) {
+          markInstructionLive(borrowInst);
+          // Visit all end_borrows and mark them live
+          auto visitEndBorrow = [&](EndBorrowInst *endBorrow) {
+            markInstructionLive(endBorrow);
+          };
+          visitTransitiveEndBorrows(borrowInst, visitEndBorrow);
+          continue;
+        }
+        // If not populate reborrowDependencies for this borrow
         findReborrowDependencies(cast<BeginBorrowInst>(&I));
         break;
       }

test/SILOptimizer/dead_code_elimination_nontrivial_ossa.sil

@@ -456,6 +456,57 @@ bb1(%newcopy : @owned $Klass, %newborrow1 : @guaranteed $Klass, %newborrow2 : @g
   end_borrow %newborrow2 : $Klass
   destroy_value %newcopy : $Klass
   destroy_value %copy2 : $Klass
+   %res = tuple ()
+  return %res : $()
+}
+
+// Nested borrows are currently not optimized in DCE
+sil [ossa] @dce_nestedborrowlifetime1 : $@convention(thin) (@guaranteed NonTrivialStruct) -> @owned NonTrivialStruct {
+bb0(%0 : @guaranteed $NonTrivialStruct):
+  %borrowo = begin_borrow %0 : $NonTrivialStruct
+  %borrow = begin_borrow %borrowo : $NonTrivialStruct
+  br bb1(%borrowo : $NonTrivialStruct, %borrow : $NonTrivialStruct)
+
+bb1(%newborrowo : @guaranteed $NonTrivialStruct, %borrow2 : @guaranteed $NonTrivialStruct):
+  %newcopy = copy_value %borrow2 : $NonTrivialStruct
+  end_borrow %borrow2 : $NonTrivialStruct
+  end_borrow %newborrowo : $NonTrivialStruct
+  return %newcopy : $NonTrivialStruct
+}
+
+sil [ossa] @dce_nestedborrowlifetime2 : $@convention(thin) (@guaranteed Klass) -> () {
+bb0(%0 : @guaranteed $Klass):
+  %borrowo = begin_borrow %0 : $Klass
+  %borrow = begin_borrow %borrowo : $Klass
+  %2 = function_ref @$use_klass2 : $@convention(thin) (@guaranteed Klass) -> ()
+  %3 = apply %2(%borrow) : $@convention(thin) (@guaranteed Klass) -> ()
+  %5 = apply %2(%borrowo) : $@convention(thin) (@guaranteed Klass) -> ()
+  br bb1(%borrowo : $Klass, %borrow : $Klass)
+
+bb1(%newborrow : @guaranteed $Klass, %borrow2 : @guaranteed $Klass):
+  end_borrow %borrow2 : $Klass
+  end_borrow %newborrow : $Klass
+  %res = tuple ()
+  return %res : $()
+}
+
+// This test shows it is non trivial to find the insert point of an outer reborrow.
+// Here %newborrowo and %newborrowi are both dead phis.
+// First end_borrow for the incoming value of %newborrowi is added
+// It is non straight forward to find the insert pt for the end_borrow of the incoming value of %newborrowo
+// This may not be important once CanonicalizeOSSALifetime supports rewrite of multi-block borrows. 
+sil [ossa] @dce_nestedborrowlifetime3 : $@convention(thin) (@guaranteed Klass) -> () {
+bb0(%0 : @guaranteed $Klass):
+  %borrowo = begin_borrow %0 : $Klass
+  %borrow = begin_borrow %borrowo : $Klass
+  %2 = function_ref @$use_klass2 : $@convention(thin) (@guaranteed Klass) -> ()
+  %3 = apply %2(%borrow) : $@convention(thin) (@guaranteed Klass) -> ()
+  %5 = apply %2(%borrowo) : $@convention(thin) (@guaranteed Klass) -> ()
+  br bb1(%borrow : $Klass, %borrowo : $Klass)
+
+bb1(%newborrowi : @guaranteed $Klass, %newborrowo : @guaranteed $Klass):
+  end_borrow %newborrowi : $Klass
+  end_borrow %newborrowo : $Klass
   %res = tuple ()
   return %res : $()
 }