[semantic-arc] Ignore type dependent uses when using a worklist to check for writes.

Otherwise, we hit misc crashes. The worklist should have always been filtering
these. I wonder why we have never hit this issue before.

I added a new API that filters out type dependent uses called
ValueBase::getNonTypeDependentUses() to make it easier to perform def->use
worklist traversal ignoring these uses. This mirrors the APIs that we have
created for filtering type dependent operands when performing use->def worklist
traversal.

I also noticed that we are not eliminating a load [copy] that we could in the
test case. I am going to file a separate bug report for that work.

rdar://79781943

Author: gottesmm

include/swift/SIL/SILValue.h

@@ -43,6 +43,7 @@ class DeadEndBlocks;
 class ValueBaseUseIterator;
 class ConsumingUseIterator;
 class NonConsumingUseIterator;
+class NonTypeDependentUseIterator;
 class SILValue;
 
 /// An enumeration which contains values for all the concrete ValueBase
@@ -387,6 +388,9 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
   using consuming_use_range = iterator_range<consuming_use_iterator>;
   using non_consuming_use_iterator = NonConsumingUseIterator;
   using non_consuming_use_range = iterator_range<non_consuming_use_iterator>;
+  using non_typedependent_use_iterator = NonTypeDependentUseIterator;
+  using non_typedependent_use_range =
+      iterator_range<non_typedependent_use_iterator>;
 
   inline use_iterator use_begin() const;
   inline use_iterator use_end() const;
@@ -397,6 +401,9 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
   inline non_consuming_use_iterator non_consuming_use_begin() const;
   inline non_consuming_use_iterator non_consuming_use_end() const;
 
+  inline non_typedependent_use_iterator non_typedependent_use_begin() const;
+  inline non_typedependent_use_iterator non_typedependent_use_end() const;
+
   /// Returns a range of all uses, which is useful for iterating over all uses.
   /// To ignore debug-info instructions use swift::getNonDebugUses instead
   /// (see comment in DebugUtils.h).
@@ -421,6 +428,10 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
   /// Returns a range of all non consuming uses
   inline non_consuming_use_range getNonConsumingUses() const;
 
+  /// Returns a range of uses that are not classified as a type dependent
+  /// operand of the user.
+  inline non_typedependent_use_range getNonTypeDependentUses() const;
+
   template <class T>
   inline T *getSingleUserOfType() const;
 
@@ -1071,6 +1082,7 @@ class Operand {
   friend class ValueBaseUseIterator;
   friend class ConsumingUseIterator;
   friend class NonConsumingUseIterator;
+  friend class NonTypeDependentUseIterator;
   template <unsigned N> friend class FixedOperandList;
   friend class TrailingOperandsList;
 };
@@ -1197,6 +1209,41 @@ ValueBase::non_consuming_use_end() const {
   return ValueBase::non_consuming_use_iterator(nullptr);
 }
 
+class NonTypeDependentUseIterator : public ValueBaseUseIterator {
+public:
+  explicit NonTypeDependentUseIterator(Operand *cur)
+      : ValueBaseUseIterator(cur) {}
+  NonTypeDependentUseIterator &operator++() {
+    assert(Cur && "incrementing past end()!");
+    assert(!Cur->isTypeDependent());
+    while ((Cur = Cur->NextUse)) {
+      if (!Cur->isTypeDependent())
+        break;
+    }
+    return *this;
+  }
+
+  NonTypeDependentUseIterator operator++(int unused) {
+    NonTypeDependentUseIterator copy = *this;
+    ++*this;
+    return copy;
+  }
+};
+
+inline ValueBase::non_typedependent_use_iterator
+ValueBase::non_typedependent_use_begin() const {
+  auto cur = FirstUse;
+  while (cur && cur->isTypeDependent()) {
+    cur = cur->NextUse;
+  }
+  return ValueBase::non_typedependent_use_iterator(cur);
+}
+
+inline ValueBase::non_typedependent_use_iterator
+ValueBase::non_typedependent_use_end() const {
+  return ValueBase::non_typedependent_use_iterator(nullptr);
+}
+
 inline bool ValueBase::hasOneUse() const {
   auto I = use_begin(), E = use_end();
   if (I == E) return false;
@@ -1242,6 +1289,11 @@ ValueBase::getNonConsumingUses() const {
   return {non_consuming_use_begin(), non_consuming_use_end()};
 }
 
+inline ValueBase::non_typedependent_use_range
+ValueBase::getNonTypeDependentUses() const {
+  return {non_typedependent_use_begin(), non_typedependent_use_end()};
+}
+
 inline bool ValueBase::hasTwoUses() const {
   auto iter = use_begin(), end = use_end();
   for (unsigned i = 0; i < 2; ++i) {

lib/SILOptimizer/SemanticARC/Context.cpp

@@ -38,16 +38,20 @@ void Context::verify() const {
 bool Context::constructCacheValue(
     SILValue initialValue,
     SmallVectorImpl<Operand *> &wellBehavedWriteAccumulator) {
-  SmallVector<Operand *, 8> worklist(initialValue->getUses());
+  SmallVector<Operand *, 8> worklist(initialValue->getNonTypeDependentUses());
 
   while (!worklist.empty()) {
     auto *op = worklist.pop_back_val();
+    assert(!op->isTypeDependent() &&
+           "Uses that are type dependent should have been filtered before "
+           "being inserted into the worklist");
     SILInstruction *user = op->getUser();
 
     if (Projection::isAddressProjection(user) ||
         isa<ProjectBlockStorageInst>(user)) {
       for (SILValue r : user->getResults()) {
-        llvm::copy(r->getUses(), std::back_inserter(worklist));
+        llvm::copy(r->getNonTypeDependentUses(),
+                   std::back_inserter(worklist));
       }
       continue;
     }
@@ -59,7 +63,8 @@ bool Context::constructCacheValue(
       }
 
       //  Otherwise, look through it and continue.
-      llvm::copy(oeai->getUses(), std::back_inserter(worklist));
+      llvm::copy(oeai->getNonTypeDependentUses(),
+                 std::back_inserter(worklist));
       continue;
     }
 
@@ -93,7 +98,8 @@ bool Context::constructCacheValue(
       }
 
       // And then add the users to the worklist and continue.
-      llvm::copy(bai->getUses(), std::back_inserter(worklist));
+      llvm::copy(bai->getNonTypeDependentUses(),
+                 std::back_inserter(worklist));
       continue;
     }
 

test/SILOptimizer/semantic-arc-opts-loadcopy-to-loadborrow.sil

@@ -13,6 +13,7 @@ import Builtin
 //////////////////
 
 typealias AnyObject = Builtin.AnyObject
+protocol Error {}
 
 enum MyNever {}
 enum FakeOptional<T> {
@@ -1450,3 +1451,51 @@ bb1:
 bb2:
   unreachable
 }
+
+// Make sure that we ignore type dependent operands when walking through the use
+// list to determine if we are writing to a piece of memory.
+protocol HasClassProperty {
+  var c: Klass { get set }
+}
+
+struct HasClassPropertyWrapper {
+  var h: HasClassProperty
+  var otherProperty: Builtin.NativeObject
+}
+
+sil @use_hasclassproperty : $@convention(method) <τ_0_0 where τ_0_0 : HasClassProperty> (@inout τ_0_0) -> (@owned Builtin.NativeObject, @error Error)
+
+// Make sure we don't crash on this code. We used to crash by not ignoring the
+// type dependent operand usage of %2.
+//
+// TODO: We should be able to convert %5 to a load_borrow since we know that %1
+// and %4 do not alias. When that is done, update the test case appropriately.
+//
+// CHECK-LABEL: sil [ossa] @use_class_property_wrapper : $@convention(thin) (@inout HasClassPropertyWrapper) -> () {
+// CHECK: load [copy]
+// CHECK: } //  end sil function 'use_class_property_wrapper'
+sil [ossa] @use_class_property_wrapper : $@convention(thin) (@inout HasClassPropertyWrapper) -> () {
+bb0(%0 : $*HasClassPropertyWrapper):
+  %1 = struct_element_addr %0 : $*HasClassPropertyWrapper, #HasClassPropertyWrapper.h
+  %2 = open_existential_addr mutable_access %1 : $*HasClassProperty to $*@opened("85AB1D00-DF62-11EB-A413-ACDE48001122") HasClassProperty
+  %3 = function_ref @use_hasclassproperty : $@convention(method) <τ_0_0 where τ_0_0 : HasClassProperty> (@inout τ_0_0) -> (@owned Builtin.NativeObject, @error Error)
+  %4 = struct_element_addr %0 : $*HasClassPropertyWrapper, #HasClassPropertyWrapper.otherProperty
+  %5 = load [copy] %4 : $*Builtin.NativeObject
+  %f2 = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
+  apply %f2(%5) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
+  try_apply %3<@opened("85AB1D00-DF62-11EB-A413-ACDE48001122") HasClassProperty>(%2) : $@convention(method) <τ_0_0 where τ_0_0 : HasClassProperty> (@inout τ_0_0) -> (@owned Builtin.NativeObject, @error Error), normal bb1, error bb2
+
+bb1(%str : @owned $Builtin.NativeObject):
+  destroy_value %str : $Builtin.NativeObject
+  destroy_value %5 : $Builtin.NativeObject
+  br bb3
+
+bb2(%error : @owned $Error):
+  destroy_value %error : $Error
+  destroy_value %5 : $Builtin.NativeObject
+  br bb3
+
+bb3:
+  %9999 = tuple()
+  return %9999 : $()
+}