Merge pull request #30069 from gottesmm/pr-cdc5d32f1a5317b8716c016a0e72f9ee40eae4dd

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

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 : $()
+}