Merge pull request #34438 from meg-gupta/reborrowverifier

[ownership] Add a new ReborrowVerifier

Author: meg-gupta

include/swift/SIL/LinearLifetimeChecker.h

@@ -50,6 +50,7 @@ class LinearLifetimeChecker {
   class ErrorBuilder;
 
 private:
+  friend class ReborrowVerifier;
   friend class SILOwnershipVerifier;
   friend class SILValueOwnershipChecker;
 

include/swift/SIL/OwnershipUtils.h

@@ -66,6 +66,9 @@ bool isOwnedForwardingInstruction(SILInstruction *inst);
 /// previous terminator.
 bool isOwnedForwardingValue(SILValue value);
 
+/// Returns true if the instruction is a 'reborrow'.
+bool isReborrowInstruction(const SILInstruction *inst);
+
 class BorrowingOperandKind {
 public:
   enum Kind : uint8_t {
@@ -134,7 +137,7 @@ struct BorrowingOperand {
     return BorrowingOperand(*kind, op);
   }
 
-  void visitEndScopeInstructions(function_ref<void(Operand *)> func) const;
+  void visitLocalEndScopeInstructions(function_ref<void(Operand *)> func) const;
 
   /// Returns true if this borrow scope operand consumes guaranteed
   /// values and produces a new scope afterwards.
@@ -204,7 +207,7 @@ struct BorrowingOperand {
   /// \p errorFunction a callback that if non-null is passed an operand that
   /// triggers a mal-formed SIL error. This is just needed for the ownership
   /// verifier to emit good output.
-  bool getImplicitUses(
+  void getImplicitUses(
       SmallVectorImpl<Operand *> &foundUses,
       std::function<void(Operand *)> *errorFunction = nullptr) const;
 

include/swift/SIL/SILValue.h

@@ -40,7 +40,8 @@ class SILInstruction;
 class SILLocation;
 class DeadEndBlocks;
 class ValueBaseUseIterator;
-class ValueUseIterator;
+class ConsumingUseIterator;
+class NonConsumingUseIterator;
 class SILValue;
 
 /// An enumeration which contains values for all the concrete ValueBase
@@ -263,10 +264,20 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
 
   using use_iterator = ValueBaseUseIterator;
   using use_range = iterator_range<use_iterator>;
+  using consuming_use_iterator = ConsumingUseIterator;
+  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>;
 
   inline use_iterator use_begin() const;
   inline use_iterator use_end() const;
 
+  inline consuming_use_iterator consuming_use_begin() const;
+  inline consuming_use_iterator consuming_use_end() const;
+
+  inline non_consuming_use_iterator non_consuming_use_begin() const;
+  inline non_consuming_use_iterator non_consuming_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).
@@ -285,6 +296,12 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
   /// and it has a single consuming user. Returns .none otherwise.
   inline Operand *getSingleConsumingUse() const;
 
+  /// Returns a range of all consuming uses
+  inline consuming_use_range getConsumingUses() const;
+
+  /// Returns a range of all non consuming uses
+  inline non_consuming_use_range getNonConsumingUses() const;
+
   template <class T>
   inline T *getSingleUserOfType() const;
 
@@ -711,8 +728,10 @@ class Operand {
     TheValue->FirstUse = this;
   }
 
+  friend class ValueBase;
   friend class ValueBaseUseIterator;
-  friend class ValueUseIterator;
+  friend class ConsumingUseIterator;
+  friend class NonConsumingUseIterator;
   template <unsigned N> friend class FixedOperandList;
   friend class TrailingOperandsList;
 };
@@ -729,6 +748,7 @@ using OperandValueArrayRef = ArrayRefView<Operand, SILValue, getSILValueType>;
 /// An iterator over all uses of a ValueBase.
 class ValueBaseUseIterator : public std::iterator<std::forward_iterator_tag,
                                                   Operand*, ptrdiff_t> {
+protected:
   Operand *Cur;
 public:
   ValueBaseUseIterator() = default;
@@ -770,6 +790,74 @@ inline ValueBase::use_iterator ValueBase::use_end() const {
 inline iterator_range<ValueBase::use_iterator> ValueBase::getUses() const {
   return { use_begin(), use_end() };
 }
+
+class ConsumingUseIterator : public ValueBaseUseIterator {
+public:
+  explicit ConsumingUseIterator(Operand *cur) : ValueBaseUseIterator(cur) {}
+  ConsumingUseIterator &operator++() {
+    assert(Cur && "incrementing past end()!");
+    assert(Cur->isConsumingUse());
+    while ((Cur = Cur->NextUse)) {
+      if (Cur->isConsumingUse())
+        break;
+    }
+    return *this;
+  }
+
+  ConsumingUseIterator operator++(int unused) {
+    ConsumingUseIterator copy = *this;
+    ++*this;
+    return copy;
+  }
+};
+
+inline ValueBase::consuming_use_iterator
+ValueBase::consuming_use_begin() const {
+  auto cur = FirstUse;
+  while (cur && !cur->isConsumingUse()) {
+    cur = cur->NextUse;
+  }
+  return ValueBase::consuming_use_iterator(cur);
+}
+
+inline ValueBase::consuming_use_iterator ValueBase::consuming_use_end() const {
+  return ValueBase::consuming_use_iterator(nullptr);
+}
+
+class NonConsumingUseIterator : public ValueBaseUseIterator {
+public:
+  explicit NonConsumingUseIterator(Operand *cur) : ValueBaseUseIterator(cur) {}
+  NonConsumingUseIterator &operator++() {
+    assert(Cur && "incrementing past end()!");
+    assert(!Cur->isConsumingUse());
+    while ((Cur = Cur->NextUse)) {
+      if (!Cur->isConsumingUse())
+        break;
+    }
+    return *this;
+  }
+
+  NonConsumingUseIterator operator++(int unused) {
+    NonConsumingUseIterator copy = *this;
+    ++*this;
+    return copy;
+  }
+};
+
+inline ValueBase::non_consuming_use_iterator
+ValueBase::non_consuming_use_begin() const {
+  auto cur = FirstUse;
+  while (cur && cur->isConsumingUse()) {
+    cur = cur->NextUse;
+  }
+  return ValueBase::non_consuming_use_iterator(cur);
+}
+
+inline ValueBase::non_consuming_use_iterator
+ValueBase::non_consuming_use_end() const {
+  return ValueBase::non_consuming_use_iterator(nullptr);
+}
+
 inline bool ValueBase::hasOneUse() const {
   auto I = use_begin(), E = use_end();
   if (I == E) return false;
@@ -806,6 +894,15 @@ inline Operand *ValueBase::getSingleConsumingUse() const {
   return result;
 }
 
+inline ValueBase::consuming_use_range ValueBase::getConsumingUses() const {
+  return {consuming_use_begin(), consuming_use_end()};
+}
+
+inline ValueBase::non_consuming_use_range
+ValueBase::getNonConsumingUses() const {
+  return {non_consuming_use_begin(), non_consuming_use_end()};
+}
+
 inline bool ValueBase::hasTwoUses() const {
   auto iter = use_begin(), end = use_end();
   for (unsigned i = 0; i < 2; ++i) {

lib/SIL/Utils/OwnershipUtils.cpp

@@ -125,6 +125,15 @@ bool swift::isOwnershipForwardingInst(SILInstruction *i) {
   return isOwnershipForwardingValueKind(SILNodeKind(i->getKind()));
 }
 
+bool swift::isReborrowInstruction(const SILInstruction *i) {
+  switch (i->getKind()) {
+  case SILInstructionKind::BranchInst:
+    return true;
+  default:
+    return false;
+  }
+}
+
 //===----------------------------------------------------------------------===//
 //                           Borrowing Operand
 //===----------------------------------------------------------------------===//
@@ -163,7 +172,7 @@ llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os,
   return os;
 }
 
-void BorrowingOperand::visitEndScopeInstructions(
+void BorrowingOperand::visitLocalEndScopeInstructions(
     function_ref<void(Operand *)> func) const {
   switch (kind) {
   case BorrowingOperandKind::BeginBorrow:
@@ -181,18 +190,8 @@ void BorrowingOperand::visitEndScopeInstructions(
     return;
   }
   case BorrowingOperandKind::Branch:
-    for (auto *succBlock :
-         cast<BranchInst>(op->getUser())->getSuccessorBlocks()) {
-      auto *arg = succBlock->getArgument(op->getOperandNumber());
-      for (auto *use : arg->getUses()) {
-        if (use->isConsumingUse()) {
-          func(use);
-        }
-      }
-    }
     return;
   }
-  llvm_unreachable("Covered switch isn't covered");
 }
 
 void BorrowingOperand::visitBorrowIntroducingUserResults(
@@ -267,46 +266,10 @@ void BorrowingOperand::visitUserResultConsumingUses(
   }
 }
 
-bool BorrowingOperand::getImplicitUses(
+void BorrowingOperand::getImplicitUses(
     SmallVectorImpl<Operand *> &foundUses,
     std::function<void(Operand *)> *errorFunction) const {
-  if (!areAnyUserResultsBorrowIntroducers()) {
-    visitEndScopeInstructions([&](Operand *op) { foundUses.push_back(op); });
-    return false;
-  }
-
-  // Ok, we have an instruction that introduces a new borrow scope and its
-  // result is that borrow scope. In such a case, we need to not just add the
-  // end scope instructions of this scoped operand, but also look through any
-  // guaranteed phis and add their end_borrow to this list as well.
-  SmallVector<BorrowingOperand, 8> worklist;
-  SmallPtrSet<Operand *, 8> visitedValue;
-  worklist.push_back(*this);
-  visitedValue.insert(op);
-  bool foundError = false;
-  while (!worklist.empty()) {
-    auto scopedOperand = worklist.pop_back_val();
-    scopedOperand.visitConsumingUsesOfBorrowIntroducingUserResults(
-        [&](Operand *op) {
-          if (auto subSub = BorrowingOperand::get(op)) {
-            if (!visitedValue.insert(op).second) {
-              if (errorFunction) {
-                (*errorFunction)(op);
-              }
-              foundError = true;
-              return;
-            }
-
-            worklist.push_back(*subSub);
-            visitedValue.insert(subSub->op);
-            return;
-          }
-
-          foundUses.push_back(op);
-        });
-  }
-
-  return foundError;
+  visitLocalEndScopeInstructions([&](Operand *op) { foundUses.push_back(op); });
 }
 
 //===----------------------------------------------------------------------===//

lib/SIL/Verifier/CMakeLists.txt

@@ -2,5 +2,6 @@ target_sources(swiftSIL PRIVATE
   LoadBorrowImmutabilityChecker.cpp
   LinearLifetimeChecker.cpp
   MemoryLifetime.cpp
+  ReborrowVerifier.cpp
   SILOwnershipVerifier.cpp
   SILVerifier.cpp)

lib/SIL/Verifier/LinearLifetimeChecker.cpp

@@ -305,10 +305,14 @@ void State::checkForSameBlockUseAfterFree(Operand *consumingUse,
   // must be instructions in the given block. Make sure that the non consuming
   // user is strictly before the consuming user.
   for (auto *nonConsumingUse : nonConsumingUsesInBlock) {
-    if (std::find_if(consumingUse->getUser()->getIterator(), userBlock->end(),
-                     [&nonConsumingUse](const SILInstruction &i) -> bool {
-                       return nonConsumingUse->getUser() == &i;
-                     }) == userBlock->end()) {
+    if (nonConsumingUse->getUser() != consumingUse->getUser()) {
+      if (std::find_if(consumingUse->getUser()->getIterator(), userBlock->end(),
+                       [&nonConsumingUse](const SILInstruction &i) -> bool {
+                         return nonConsumingUse->getUser() == &i;
+                       }) == userBlock->end()) {
+        continue;
+      }
+    } else if (isReborrowInstruction(consumingUse->getUser())) {
       continue;
     }