[semantic-arc] Change StorageGuaranteesLoadVisitor::visitClassAccess to find borrow scope introducers using utilities from OwnershipUtils.h.

I added a new API into OwnershipUtils called
getSingleBorrowIntroducingValue. This API returns a single
BorrowScopeIntroducingValue for a passed in guaranteed value. If we can not find
such a BorrowScopeIntroducingValue or we find multiple such, we return None.

Using that, I refactored StorageGuaranteesLoadVisitor::visitClassAccess(...) to
use this new API which should make it significantly more robust since the
routine uses the definitions of "guaranteed forwarding" that the ownership
verifier uses when it verifies meaning that we can rely on the routine to be
exhaustive and correct. This means that we now promote load [copy] ->
load_borrow even if our borrow scope introducer feeds through a switch_enum or
checked_cast_br result (the main reason I looked into this change).

To create getSingleBorrowIntroducingValue, I refactored
getUnderlyingBorrowIntroucingValues to use a generator to find all of its
underlying values. Then in getSingleBorrowIntroducingValue, I just made it so
that we call the generator 1-2 times (as appropriate) to implement this query.

Author: gottesmm

include/swift/SIL/OwnershipUtils.h

@@ -564,6 +564,14 @@ bool getAllBorrowIntroducingValues(
 Optional<BorrowScopeIntroducingValue>
 getSingleBorrowIntroducingValue(SILValue value);
 
+/// Look up through the def-use chain of \p inputValue, looking for an initial
+/// "borrow" introducing value. If at any point, we find two introducers or we
+/// find a point in the chain we do not understand, we bail and return false. If
+/// we are able to understand all of the def-use graph and only find a single
+/// introducer, then we return a .some(BorrowScopeIntroducingValue).
+Optional<BorrowScopeIntroducingValue>
+getSingleBorrowIntroducingValue(SILValue inputValue);
+
 } // namespace swift
 
 #endif

lib/SILOptimizer/Transforms/SemanticARCOpts.cpp

@@ -1091,45 +1091,50 @@ class StorageGuaranteesLoadVisitor
     if (!field->getField()->isLet()) {
       return answer(true);
     }
-    
+
     // The lifetime of the `let` is guaranteed if it's dominated by the
-    // guarantee on the base. Check for a borrow.
+    // guarantee on the base. See if we can find a single borrow introducer for
+    // this object. If we could not find a single such borrow introducer, assume
+    // that our property is conservatively written to.
     SILValue baseObject = field->getOperand();
-    auto beginBorrow = dyn_cast<BeginBorrowInst>(baseObject);
-    if (beginBorrow)
-      baseObject = beginBorrow->getOperand();
-    baseObject = stripCasts(baseObject);
-
-    // A guaranteed argument trivially keeps the base alive for the duration of
-    // the projection.
-    if (auto *arg = dyn_cast<SILFunctionArgument>(baseObject)) {
-      if (arg->getArgumentConvention().isGuaranteedConvention()) {
+    auto value = getSingleBorrowIntroducingValue(baseObject);
+    if (!value) {
+      return answer(true);
+    }
+
+    // Ok, we have a single borrow introducing value. First do a quick check if
+    // we have a non-local scope that is a function argument. In such a case, we
+    // know statically that our let can not be written to in the current
+    // function. To be conservative, assume that all other non-local scopes
+    // write to memory.
+    if (!value->isLocalScope()) {
+      if (value->kind == BorrowScopeIntroducingValueKind::SILFunctionArgument) {
         return answer(false);
       }
+
+      // TODO: Once we model Coroutine results as non-local scopes, we should be
+      // able to return false here for them as well.
+      return answer(true);
     }
-    
-    // See if there's a borrow of the base object our load is based on.
-    SILValue borrowInst;
-    if (isa<LoadBorrowInst>(baseObject)) {
-      borrowInst = baseObject;
-    } else {
-      borrowInst = beginBorrow;
-    }
-    // TODO: We could also look at a guaranteed phi argument and see whether
-    // the loaded copy is dominated by it.
-    if (!borrowInst)
+
+    // TODO: This is disabled temporarily for guaranteed phi args just for
+    // staging purposes. Thus be conservative and assume true in these cases.
+    if (value->kind == BorrowScopeIntroducingValueKind::Phi) {
       return answer(true);
+    }
 
-    // Use the linear lifetime checker to check whether the copied
-    // value is dominated by the lifetime of the borrow it's based on.
-    SmallVector<SILInstruction *, 4> baseEndBorrows;
-    llvm::copy(borrowInst->getUsersOfType<EndBorrowInst>(),
-               std::back_inserter(baseEndBorrows));
+    // Ok, we now know that we have a local scope whose lifetime we need to
+    // analyze. With that in mind, gather up the lifetime ending uses of our
+    // borrow scope introducing value and then use the linear lifetime checker
+    // to check whether the copied value is dominated by the lifetime of the
+    // borrow it's based on.
+    SmallVector<SILInstruction *, 4> endScopeInsts;
+    value->getLocalScopeEndingInstructions(endScopeInsts);
 
     SmallPtrSet<SILBasicBlock *, 4> visitedBlocks;
     LinearLifetimeChecker checker(visitedBlocks, ARCOpt.getDeadEndBlocks());
     // Returns true on success. So we invert.
-    bool foundError = !checker.validateLifetime(baseObject, baseEndBorrows,
+    bool foundError = !checker.validateLifetime(baseObject, endScopeInsts,
                                                 liveRange.getDestroys());
     return answer(foundError);
   }

test/SILOptimizer/semantic-arc-opts.sil

@@ -29,6 +29,7 @@ sil @get_nativeobject_pair : $@convention(thin) () -> @owned NativeObjectPair
 class Klass {}
 sil @guaranteed_klass_user : $@convention(thin) (@guaranteed Klass) -> ()
 sil @guaranteed_fakeoptional_klass_user : $@convention(thin) (@guaranteed FakeOptional<Klass>) -> ()
+sil @guaranteed_fakeoptional_classlet_user : $@convention(thin) (@guaranteed FakeOptional<ClassLet>) -> ()
 
 struct MyInt {
   var value: Builtin.Int32
@@ -1499,4 +1500,160 @@ bb2:
 bb3:
   %9999 = tuple()
   return %9999 : $()
-}
+}
+
+// CHECK-LABEL: sil [ossa] @convert_load_copy_to_load_borrow_despite_switch_enum_functionarg : $@convention(thin) (@guaranteed FakeOptional<ClassLet>) -> () {
+// CHECK-NOT: load [copy]
+// CHECK: load_borrow
+// CHECK-NOT: load [copy]
+// CHECK: } // end sil function 'convert_load_copy_to_load_borrow_despite_switch_enum_functionarg'
+sil [ossa] @convert_load_copy_to_load_borrow_despite_switch_enum_functionarg : $@convention(thin) (@guaranteed FakeOptional<ClassLet>) -> () {
+bb0(%0 : @guaranteed $FakeOptional<ClassLet>):
+  %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
+  switch_enum %0 : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2
+
+bb1(%1 : @guaranteed $ClassLet):
+  %2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
+  %3 = load [copy] %2 : $*Klass
+  apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
+  destroy_value %3 : $Klass
+  br bb3
+
+bb2:
+  br bb3
+
+bb3:
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// CHECK-LABEL: sil [ossa] @convert_load_copy_to_load_borrow_despite_switch_enum_beginborrow : $@convention(thin) (@owned FakeOptional<ClassLet>) -> () {
+// CHECK-NOT: load [copy]
+// CHECK: load_borrow
+// CHECK-NOT: load [copy]
+// CHECK: } // end sil function 'convert_load_copy_to_load_borrow_despite_switch_enum_beginborrow'
+sil [ossa] @convert_load_copy_to_load_borrow_despite_switch_enum_beginborrow : $@convention(thin) (@owned FakeOptional<ClassLet>) -> () {
+bb0(%0 : @owned $FakeOptional<ClassLet>):
+  %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
+  %0a = begin_borrow %0 : $FakeOptional<ClassLet>
+  switch_enum %0a : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2
+
+bb1(%1 : @guaranteed $ClassLet):
+  %2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
+  %3 = load [copy] %2 : $*Klass
+  apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
+  destroy_value %3 : $Klass
+  br bb3
+
+bb2:
+  br bb3
+
+bb3:
+  end_borrow %0a : $FakeOptional<ClassLet>
+  destroy_value %0 : $FakeOptional<ClassLet>
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// CHECK-LABEL: sil [ossa] @convert_load_copy_to_load_borrow_despite_switch_enum_loadborrow : $@convention(thin) (@in_guaranteed FakeOptional<ClassLet>) -> () {
+// CHECK-NOT: load [copy]
+// CHECK: load_borrow
+// CHECK-NOT: load [copy]
+// CHECK: load_borrow
+// CHECK-NOT: load [copy]
+// CHECK: } // end sil function 'convert_load_copy_to_load_borrow_despite_switch_enum_loadborrow'
+sil [ossa] @convert_load_copy_to_load_borrow_despite_switch_enum_loadborrow : $@convention(thin) (@in_guaranteed FakeOptional<ClassLet>) -> () {
+bb0(%0 : $*FakeOptional<ClassLet>):
+  %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
+  %0a = load_borrow %0 : $*FakeOptional<ClassLet>
+  switch_enum %0a : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2
+
+bb1(%1 : @guaranteed $ClassLet):
+  %2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
+  %3 = load [copy] %2 : $*Klass
+  apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
+  destroy_value %3 : $Klass
+  br bb3
+
+bb2:
+  br bb3
+
+bb3:
+  end_borrow %0a : $FakeOptional<ClassLet>
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// TODO: We can support this in a little bit once the rest of SemanticARCOpts is
+// guaranteed to be safe with guaranteed phis.
+//
+// CHECK-LABEL: sil [ossa] @convert_load_copy_to_load_borrow_despite_switch_enum_guaranteedphi_1 : $@convention(thin) (@owned FakeOptional<ClassLet>) -> () {
+// CHECK: load [copy]
+// CHECK: } // end sil function 'convert_load_copy_to_load_borrow_despite_switch_enum_guaranteedphi_1'
+sil [ossa] @convert_load_copy_to_load_borrow_despite_switch_enum_guaranteedphi_1 : $@convention(thin) (@owned FakeOptional<ClassLet>) -> () {
+bb0(%0 : @owned $FakeOptional<ClassLet>):
+  %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
+  cond_br undef, bb0a, bb0b
+
+bb0a:
+  %0a = begin_borrow %0 : $FakeOptional<ClassLet>
+  br bb0c(%0a : $FakeOptional<ClassLet>)
+
+bb0b:
+  %0b = begin_borrow %0 : $FakeOptional<ClassLet>
+  br bb0c(%0b : $FakeOptional<ClassLet>)
+
+bb0c(%0c : @guaranteed $FakeOptional<ClassLet>):
+  switch_enum %0c : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2
+
+bb1(%1 : @guaranteed $ClassLet):
+  %2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
+  %3 = load [copy] %2 : $*Klass
+  apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
+  destroy_value %3 : $Klass
+  br bb3
+
+bb2:
+  br bb3
+
+bb3:
+  end_borrow %0c : $FakeOptional<ClassLet>
+  destroy_value %0 : $FakeOptional<ClassLet>
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// Make sure that if begin_borrow has a consuming end scope use, we can still
+// eliminate load [copy].
+//
+// CHECK-LABEL: sil [ossa] @convert_load_copy_to_load_borrow_despite_switch_enum_guaranteedphi_2 : $@convention(thin) (@owned FakeOptional<ClassLet>) -> () {
+// CHECK-NOT: load [copy]
+// CHECK: load_borrow
+// CHECK-NOT: load [copy]
+// CHECK: } // end sil function 'convert_load_copy_to_load_borrow_despite_switch_enum_guaranteedphi_2'
+sil [ossa] @convert_load_copy_to_load_borrow_despite_switch_enum_guaranteedphi_2 : $@convention(thin) (@owned FakeOptional<ClassLet>) -> () {
+bb0(%0 : @owned $FakeOptional<ClassLet>):
+  %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
+  cond_br undef, bb1, bb2
+
+bb1:
+  %0a = begin_borrow %0 : $FakeOptional<ClassLet>
+  br bb3(%0a : $FakeOptional<ClassLet>)
+
+bb2:
+  %0b = begin_borrow %0 : $FakeOptional<ClassLet>
+  %0b2 = unchecked_enum_data %0b : $FakeOptional<ClassLet>, #FakeOptional.some!enumelt.1
+  %2 = ref_element_addr %0b2 : $ClassLet, #ClassLet.aLet
+  %3 = load [copy] %2 : $*Klass
+  apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
+  destroy_value %3 : $Klass
+  br bb3(%0b : $FakeOptional<ClassLet>)
+
+bb3(%0c : @guaranteed $FakeOptional<ClassLet>):
+  %f2 = function_ref @guaranteed_fakeoptional_classlet_user : $@convention(thin) (@guaranteed FakeOptional<ClassLet>) -> ()
+  apply %f2(%0c) : $@convention(thin) (@guaranteed FakeOptional<ClassLet>) -> ()
+  end_borrow %0c : $FakeOptional<ClassLet>
+  destroy_value %0 : $FakeOptional<ClassLet>
+  %9999 = tuple()
+  return %9999 : $()
+}