Merge pull request #41228 from nate-chandler/lexical_lifetimes/owned_function_args_are_lexical

[SIL] Make owned function arguments lexical.

Author: nate-chandler

docs/SIL.rst

@@ -2364,6 +2364,69 @@ The current list of interior pointer SIL instructions are:
 (*) We still need to finish adding support for project_box, but all other
 interior pointers are guarded already.
 
+Variable Lifetimes
+~~~~~~~~~~~~~~~~~~
+
+In order for programmer intended lifetimes to be maintained under optimization,
+the lifetimes of SIL values which correspond to named source-level values can
+only be modified in limited ways.  Generally, the behavior is that the lifetime
+of a named source-level value cannot _observably_ end before the end of the
+lexical scope in which that value is defined.  Specifically, code motion may
+not move the ends of these lifetimes across a **deinit barrier**.
+
+A few sorts of SIL value have lifetimes that are constrained that way:
+
+1: `begin_borrow [lexical]`
+2: `move_value [lexical]`
+3: @owned function arguments
+4: `alloc_stack [lexical]`
+
+That these three have constrained lifetimes is encoded in ValueBase::isLexical,
+which should be checked before changing the lifetime of a value.
+
+The reason that only @owned function arguments are constrained is that a
+@guaranteed function argument is guaranteed by the function's caller to live for
+the full duration of the function already.  Optimization of the function alone
+can't shorten it.  When such a function is inlined into its caller, though, a
+lexical borrow scope is added for each of its @guaranteed arguments, ensuring
+that the lifetime of the corresponding source-level value is not shortened in a
+way that doesn't respect deinit barriers.
+
+Unlike the other sorts, `alloc_stack [lexical]` isn't a SILValue.  Instead, it
+constrains the lifetime of an addressable variable.  Since the constraint is
+applied to the in-memory representation, no additional lexical SILValue is
+required.
+
+Deinit Barriers
+```````````````
+
+Deinit barriers (see swift::isDeinitBarrier) are instructions which would be
+affected by the side effects of deinitializers.  To maintain the order of
+effects that is visible to the programmer, destroys of lexical values cannot be
+reordered with respect to them.  There are three kinds:
+
+1. synchronization points (locks, memory barriers, syscalls, etc.)
+2. loads of weak or unowned values
+3. accesses of pointers
+
+Examples:
+
+1. Given an instance of a class which owns a file handle and closes the file
+   handle on deinit, writing to the file handle and then deallocating the
+   instance would result in changes being written.  If the destroy of the
+   instance were hoisted above the call to write to the file handle, an error
+   would be raised instead.
+2. Given an instance `c` of a class `C` which weakly references an instance `d`
+   of a second class `D`, if `d` is referenced via a local variable `v`, then
+   loading that weak reference from `c` within the variable scope should return
+   a non-nil reference to `d`.  Hoisting the destroy of `v` above the weak load
+   from `c`, however, would result in the destruction of `d` before that load
+   and a nil weak reference to `D`.
+3. Given an instance of a class which owns a buffer and deallocates it on
+   deinitialization, accessing the pointer and then deallocating the instance
+   is defined behavior.  Hoisting the destroy of the instance above the access
+   to the memory would result in accessing a freed pointer.
+
 Memory Lifetime
 ~~~~~~~~~~~~~~~
 

include/swift/SIL/MemAccessUtils.h

@@ -234,8 +234,8 @@ inline bool accessKindMayConflict(SILAccessKind a, SILAccessKind b) {
 
 /// Return true if \p instruction is a deinitialization barrier.
 ///
-/// Deinitialization barriers constrain variable lifetimes. Lexical end_borrow
-/// and destroy_addr cannot be hoisted above them.
+/// Deinitialization barriers constrain variable lifetimes. Lexical end_borrow,
+/// destroy_value, and destroy_addr cannot be hoisted above them.
 bool isDeinitBarrier(SILInstruction *instruction);
 
 } // end namespace swift

include/swift/SIL/OwnershipUtils.h

@@ -169,6 +169,14 @@ bool findExtendedTransitiveGuaranteedUses(
   SILValue guaranteedValue,
   SmallVectorImpl<Operand *> &usePoints);
 
+/// Find non-transitive uses of a simple (i.e. without looking through
+/// reborrows) value.
+///
+/// The scope-ending use of borrows of the value are included.  If a borrow of
+/// the value is reborrowed, returns false.
+bool findUsesOfSimpleValue(SILValue value,
+                           SmallVectorImpl<Operand *> *usePoints = nullptr);
+
 /// An operand that forwards ownership to one or more results.
 class ForwardingOperand {
   Operand *use = nullptr;

include/swift/SILOptimizer/Utils/OwnershipOptUtils.h

@@ -212,7 +212,8 @@ class OwnershipRAUWHelper {
   /// their value ownership. This ignores any current uses of \p oldValue. To
   /// determine whether \p oldValue can be replaced as-is with it's existing
   /// uses, create an instance of OwnershipRAUWHelper and check its validity.
-  static bool hasValidRAUWOwnership(SILValue oldValue, SILValue newValue);
+  static bool hasValidRAUWOwnership(SILValue oldValue, SILValue newValue,
+                                    ArrayRef<Operand *> oldUses);
 
 private:
   OwnershipFixupContext *ctx;
@@ -283,6 +284,10 @@ class OwnershipRAUWHelper {
   SILValue getReplacementAddress();
 };
 
+/// Whether the provided uses lie within the current liveness of the
+/// specified lexical value.
+bool areUsesWithinLexicalValueLifetime(SILValue, ArrayRef<Operand *>);
+
 /// A utility composed ontop of OwnershipFixupContext that knows how to replace
 /// a single use of a value with another value with a different ownership. We
 /// allow for the values to have different types.

lib/SIL/IR/SILValue.cpp

@@ -99,10 +99,8 @@ ValueBase::getDefiningInstructionResult() {
 }
 
 bool ValueBase::isLexical() const {
-  // TODO: Eventually, rather than SILGen'ing a borrow scope for owned 
-  //       arguments, we will just have this check here.
-  // if (auto *argument = dyn_cast<SILArgument>(this))
-  //   return argument->getOwnershipKind() == OwnershipKind::Owned;
+  if (auto *argument = dyn_cast<SILFunctionArgument>(this))
+    return argument->getOwnershipKind() == OwnershipKind::Owned;
   if (auto *bbi = dyn_cast<BeginBorrowInst>(this))
     return bbi->isLexical();
   if (auto *mvi = dyn_cast<MoveValueInst>(this))

lib/SIL/Utils/OwnershipUtils.cpp

@@ -305,6 +305,25 @@ bool swift::findExtendedUsesOfSimpleBorrowedValue(
   return true;
 }
 
+bool swift::findUsesOfSimpleValue(SILValue value,
+                                  SmallVectorImpl<Operand *> *usePoints) {
+  for (auto *use : value->getUses()) {
+    if (use->getOperandOwnership() == OperandOwnership::Borrow) {
+      if (!BorrowingOperand(use).visitScopeEndingUses([&](Operand *end) {
+            if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
+              return false;
+            }
+            usePoints->push_back(use);
+            return true;
+          })) {
+        return false;
+      }
+    }
+    usePoints->push_back(use);
+  }
+  return true;
+}
+
 // Find all use points of \p guaranteedValue within its borrow scope. All use
 // points will be dominated by \p guaranteedValue.
 //

lib/SILOptimizer/Utils/LexicalDestroyFolding.cpp

@@ -290,7 +290,7 @@ struct BorroweeUsage final {
 /// %lifetime.  In detail, PrunedLiveness::isWithinBoundary relies on
 /// clients to know that instructions are after the start of liveness.  We
 /// determine this via the dominance tree.
-bool findBorroweeUsage(Context &, BorroweeUsage &);
+bool findBorroweeUsage(Context const &, BorroweeUsage &);
 
 /// Sift scope ends of %lifetime for those that CAN be folded.
 class FilterCandidates final {
@@ -308,9 +308,6 @@ class FilterCandidates final {
 
   /// Determines whether each candidate is viable for folding.
   ///
-  /// Requiring findUsers to have been called, this uses the more expensive
-  /// isViableMatch.
-  ///
   /// returns true if any candidates were viable
   ///         false otherwise
   bool run(Candidates &candidates);
@@ -334,8 +331,7 @@ class FilterCandidates final {
 ///
 /// If there are, we can't fold the apply.  Specifically, we can't introduce
 /// a move_value [lexical] %borrowee because that value still needs to be used
-/// in those locations.  While updateSSA would happily rewrite those uses to
-/// be uses of the move, that is not a semantically valid transformation.
+/// in those locations.
 ///
 /// For example, given the following SIL
 ///     %borrowee : @owned
@@ -410,11 +406,7 @@ class Rewriter final {
   ///     possible)
   /// (2) hoist the end_borrow
   /// (3) delete the destroy_value
-  void fold(Match, SmallVectorImpl<int> const &rewritableArgumentIndices);
-
-  // Update SSA to reflect that fact that %move and %borrowee are two
-  // defs for the "same value".
-  void updateSSA();
+  void fold(Match, ArrayRef<int> rewritableArgumentIndices);
 
   // The move_value [lexical] instruction that was added during the run.
   //
@@ -470,25 +462,31 @@ bool run(Context &context) {
 void Rewriter::run() {
   bool foldedAny = false;
   (void)foldedAny;
-  for (unsigned index = 0, count = candidates.vector.size(); index < count;
-       ++index) {
+  auto size = candidates.vector.size();
+  for (unsigned index = 0; index < size; ++index) {
     auto candidate = candidates.vector[index];
-    if (!candidate.viable)
-      continue;
     createMove();
+    if (!candidate.viable) {
+      // Nonviable candidates still end with the pattern
+      //
+      //     end_borrow %lifetime
+      //     ...
+      //     destroy_value %borrowee
+      //
+      // Now that the new move_value [lexical] dominates all candidates, the
+      // every candidate's destroy_value %borrowee is dominated by it, so every
+      // one is dominated by another consuming use which is illegal.  Rewrite
+      // each such destroy_value to be a destroy_value of the move.
+      candidate.match.dvi->setOperand(mvi);
+      continue;
+    }
 
     fold(candidate.match, candidate.argumentIndices);
 #ifndef NDEBUG
     foldedAny = true;
 #endif
   }
   assert(foldedAny && "rewriting without anything to rewrite!?");
-
-  // There are now "two defs for %borrowee":
-  // - %borrowee
-  // - %move
-  // We need to update SSA.
-  updateSSA();
 }
 
 bool Rewriter::createMove() {
@@ -505,8 +503,7 @@ bool Rewriter::createMove() {
   return true;
 }
 
-void Rewriter::fold(Match candidate,
-                    SmallVectorImpl<int> const &rewritableArgumentIndices) {
+void Rewriter::fold(Match candidate, ArrayRef<int> rewritableArgumentIndices) {
   // First, rewrite the apply in terms of the move_value.
   unsigned argumentNumber = 0;
   for (auto index : rewritableArgumentIndices) {
@@ -551,26 +548,6 @@ void Rewriter::fold(Match candidate,
   context.deleter.forceDelete(candidate.dvi);
 }
 
-void Rewriter::updateSSA() {
-  SILSSAUpdater updater;
-  updater.initialize(context.borrowee->getType(),
-                     context.borrowee.getOwnershipKind());
-  updater.addAvailableValue(context.borrowee->getParentBlock(),
-                            context.borrowee);
-  updater.addAvailableValue(mvi->getParentBlock(), mvi);
-
-  SmallVector<Operand *, 16> uses;
-  for (auto use : context.borrowee->getUses()) {
-    if (use->getUser() == mvi)
-      continue;
-    uses.push_back(use);
-  }
-
-  for (auto use : uses) {
-    updater.rewriteUse(*use);
-  }
-}
-
 //===----------------------------------------------------------------------===//
 //                             MARK: Lookups
 //===----------------------------------------------------------------------===//
@@ -638,8 +615,10 @@ bool FilterCandidates::run(Candidates &candidates) {
   return anyViable;
 }
 
-bool findBorroweeUsage(Context &context, BorroweeUsage &usage) {
+bool findBorroweeUsage(Context const &context, BorroweeUsage &usage) {
   auto recordUse = [&](Operand *use) {
+    // Ignore uses that aren't dominated by the introducer.  PrunedLiveness
+    // relies on us doing this check.
     if (!context.dominanceTree.dominates(context.introducer, use->getUser()))
       return;
     usage.uses.push_back(use);