[loop-arc] Instead of using a lambda, use a callback data structure t…

…o process ARC results.

This is necessary since previously we would:

1. Perform data flow over an entire function.
2. Process all of the gathered data, inserting instructions to be deleted into a
list to avoid dangling pointer issues.
3. Delete instructions after processing the data to avoid the dangling pointer
issues.
4. If we saw any nested retain/releases and deleted any instructions, run ARC
again on the entire function.

For loop arc, we want to:

1. Visit the loop nest in post order.
2. For each loop:
   a. Perform data flow over the loop.
   b. Process all of the gathered data, inserting instructions to be deleted
      into a list to avoid dangling pointer issues.
   c. Delete instructions after loop processing has finished and potentially
      summarize the loop.
   d. If we saw any nested retain/releases and deleted any instructions, run ARC
      again on the loop.

This is more efficient in terms of the number of times that we perform dataflow
and allows us to summarize as we go.

The main disadvantage is that Block ARC steps (3,4) could occur in
GlobalARCOpts.cpp. Loop ARC on the other hand needs (2.c.) and (2.d.) to occur
while processing.

This means I need a real callback context and an extra callback call to say when
it is ok for a user of the analysis to remove instructions.

rdar://22238658

* The dangling pointer issue is that a retain/release could act as a last
use/first decrement for a different retain/release. If process the different
retain/release later, we will be touching a dangling pointer.

Swift SVN r32867

include/swift/SILAnalysis/ARCAnalysis.h

@@ -111,6 +111,13 @@ struct ARCMatchingSet {
   llvm::SetVector<SILInstruction *> Decrements;
   llvm::SetVector<SILInstruction *> DecrementInsertPts;
 
+  // This is a data structure that can not be moved.
+  ARCMatchingSet() = default;
+  ARCMatchingSet(const ARCMatchingSet &) = delete;
+  ARCMatchingSet(ARCMatchingSet &&) = delete;
+  ARCMatchingSet &operator=(const ARCMatchingSet &) = delete;
+  ARCMatchingSet &operator=(ARCMatchingSet &&) = delete;
+
   void clear() {
     Ptr = SILValue();
     Increments.clear();
@@ -135,16 +142,33 @@ ARCMatchingSetComputationContext *createARCMatchingSetComputationContext(
 void
 destroyARCMatchingSetComputationContext(ARCMatchingSetComputationContext *Ctx);
 
+/// A structure that via its virtual calls recieves the results of the analysis.
+///
+/// TODO: We could potentially pass in all of the matching sets as a list and
+/// use less virtual calls at the cost of potentially greater numbers of moves.
+struct ARCMatchingSetCallback {
+  /// This call should process \p Set and modify any internal state of
+  /// ARCMatchingSetCallback given \p Set. This call should not remove any
+  /// instructions since any removed instruction might be used as an insertion
+  /// point for another retain, release pair.
+  virtual void processMatchingSet(ARCMatchingSet &Set) {}
+
+  /// This call should perform any deletion of instructions necessary. It is run
+  /// strictly after all computation has been completed.
+  virtual void finalize() {}
+};
+
 /// Use the opaque context to recompute the matching set for the input function.
 ///
 /// \param Ctx The opaque context for the computation.
 /// \param FreezeOwningPtrEpiloqueReleases Should we not attempt to move, remove
 /// epilogue release pointers and instead use them as post dominating releases
 /// for other pointers.
-/// \param Fun The function to call with the ARC matching
+/// \param Callback The callback used to propagate information to analysis
+/// users.
 bool computeARCMatchingSet(ARCMatchingSetComputationContext *Ctx,
                            bool FreezeOwningPtrEpiloqueReleases,
-                           std::function<void (ARCMatchingSet&)> Fun);
+                           ARCMatchingSetCallback &Callback);
 
 /// A class that attempts to match owned arguments and corresponding epilogue
 /// releases for a specific function.

lib/SILAnalysis/ARC/GlobalARCPairingAnalysis.cpp

@@ -34,6 +34,10 @@
 
 using namespace swift;
 
+//===----------------------------------------------------------------------===//
+//                          ARC Matching Set Builder
+//===----------------------------------------------------------------------===//
+
 namespace {
 
 struct MatchingSetFlags {
@@ -88,6 +92,7 @@ struct ARCMatchingSetBuilder {
 
   bool matchUpIncDecSetsForPtr();
 
+  // We only allow for get result when this object is invalidated via a move.
   ARCMatchingSet &getResult() {
     return MatchSet;
   }
@@ -366,14 +371,14 @@ struct ARCMatchingSetComputationContext {
       : F(F), DecToIncStateMap(), IncToDecStateMap(), RCIA(RCIA) {}
   virtual ~ARCMatchingSetComputationContext() {}
   virtual bool run(bool FreezePostDomReleases,
-                   std::function<void (ARCMatchingSet &)> Fun) = 0;
-  bool performMatching(std::function<void (ARCMatchingSet &)> Fun);
+                   ARCMatchingSetCallback &Callback) = 0;
+  bool performMatching(ARCMatchingSetCallback &Callback);
 };
 
 } // end swift namespace
 
 bool ARCMatchingSetComputationContext::performMatching(
-    std::function<void(ARCMatchingSet &)> Fun) {
+    ARCMatchingSetCallback &Callback) {
   bool MatchedPair = false;
 
   DEBUG(llvm::dbgs() << "**** Computing ARC Matching Sets for " << F.getName()
@@ -396,10 +401,18 @@ bool ARCMatchingSetComputationContext::performMatching(
         IncToDecStateMap.blot(I);
       for (auto *I : Set.Decrements)
         DecToIncStateMap.blot(I);
-      Fun(Set);
+
+      // Add the Set to the callback. *NOTE* No instruction destruction can
+      // happen here since we may remove instructions that are insertion points
+      // for other instructions.
+      Callback.processMatchingSet(Set);
     }
   }
 
+  // Then finalize the callback. This is the only place instructions can be
+  // deleted.
+  Callback.finalize();
+
   DecToIncStateMap.clear();
   IncToDecStateMap.clear();
 
@@ -425,11 +438,11 @@ struct BlockARCMatchingSetComputationContext
   virtual ~BlockARCMatchingSetComputationContext() override final {}
 
   virtual bool run(bool FreezePostDomReleases,
-                   std::function<void(ARCMatchingSet &)> Fun) override final {
+                   ARCMatchingSetCallback &Callback) override final {
     bool NestingDetected = Evaluator.run(FreezePostDomReleases);
     Evaluator.clear();
 
-    bool MatchedPair = performMatching(Fun);
+    bool MatchedPair = performMatching(Callback);
     return NestingDetected && MatchedPair;
   }
 };
@@ -454,11 +467,11 @@ struct LoopARCMatchingSetComputationContext : ARCMatchingSetComputationContext {
   virtual ~LoopARCMatchingSetComputationContext() override final {}
 
   virtual bool run(bool FreezePostDomReleases,
-                   std::function<void(ARCMatchingSet &)> Fun) override final {
+                   ARCMatchingSetCallback &Callback) override final {
     bool NestingDetected = Evaluator.run(FreezePostDomReleases);
     Evaluator.clear();
 
-    bool MatchedPair = performMatching(Fun);
+    bool MatchedPair = performMatching(Callback);
     return NestingDetected && MatchedPair;
   }
 };
@@ -495,10 +508,10 @@ destroyARCMatchingSetComputationContext(ARCMatchingSetComputationContext *Ctx) {
 
 bool swift::computeARCMatchingSet(ARCMatchingSetComputationContext *Ctx,
                                   bool FreezePostDomReleases,
-                                  std::function<void(ARCMatchingSet &)> Fun) {
+                                  ARCMatchingSetCallback &Callback) {
 
   DEBUG(llvm::dbgs() << "**** Performing ARC Dataflow for " << Ctx->F.getName()
                      << " ****\n");
 
-  return Ctx->run(FreezePostDomReleases, Fun);
+  return Ctx->run(FreezePostDomReleases, Callback);
 }

lib/SILPasses/GlobalARCOpts.cpp

@@ -80,15 +80,34 @@ static SILInstruction *createDecrement(SILValue Ptr, SILInstruction *InsertPt) {
   return B.createReleaseValue(Loc, Ptr);
 }
 
-/// This routine takes in the ARCMatchingSet \p MatchSEt and inserts new
-/// increments, decrements at the insertion points and adds the old increment,
-/// decrements to the delete list \p DelList.
-static bool
-optimizeReferenceCountMatchingSet(ARCMatchingSet &MatchSet,
-                                  SmallVectorImpl<SILInstruction *> &DelList) {
-  DEBUG(llvm::dbgs() << "**** Optimizing Matching Set ****\n");
+namespace {
 
+class CodeMotionOrDeleteCallback : public ARCMatchingSetCallback {
   bool Changed = false;
+  llvm::SmallVector<SILInstruction *, 16> InstructionsToDelete;
+
+public:
+  virtual void processMatchingSet(ARCMatchingSet &Set) override final;
+
+  // Delete instructions after we have processed all matching sets so that we do
+  // not remove instructions that may be insertion points for other retain,
+  // releases.
+  virtual void finalize() override final {
+    while (!InstructionsToDelete.empty()) {
+      InstructionsToDelete.pop_back_val()->eraseFromParent();
+    }
+  }
+
+  bool madeChange() const { return Changed; }
+};
+}
+
+// This routine takes in the ARCMatchingSet \p MatchSet and inserts new
+// increments, decrements at the insertion points and adds the old increment,
+// decrements to the delete list. Sets changed to true if anything was moved or
+// deleted.
+void CodeMotionOrDeleteCallback::processMatchingSet(ARCMatchingSet &MatchSet) {
+  DEBUG(llvm::dbgs() << "**** Optimizing Matching Set ****\n");
 
   // Insert the new increments.
   for (SILInstruction *InsertPt : MatchSet.IncrementInsertPts) {
@@ -126,20 +145,17 @@ optimizeReferenceCountMatchingSet(ARCMatchingSet &MatchSet,
   for (SILInstruction *Increment : MatchSet.Increments) {
     Changed = true;
     DEBUG(llvm::dbgs() << "    Deleting increment: " << *Increment);
-    DelList.push_back(Increment);
+    InstructionsToDelete.push_back(Increment);
     ++NumRefCountOpsRemoved;
   }
 
   // Add the old decrements to the delete list.
   for (SILInstruction *Decrement : MatchSet.Decrements) {
     Changed = true;
     DEBUG(llvm::dbgs() << "    Deleting decrement: " << *Decrement);
-    DelList.push_back(Decrement);
+    InstructionsToDelete.push_back(Decrement);
     ++NumRefCountOpsRemoved;
   }
-
-  // Return if we made any changes.
-  return Changed;
 }
 
 //===----------------------------------------------------------------------===//
@@ -160,7 +176,6 @@ static bool processFunction(SILFunction &F, bool FreezePostDomRelease,
   DEBUG(llvm::dbgs() << "***** Processing " << F.getName() << " *****\n");
 
   bool Changed = false;
-  llvm::SmallVector<SILInstruction *, 16> InstructionsToDelete;
 
   // Construct our context once. A context contains the RPOT as well as maps
   // that contain state for each BB in F. This is a major place where the
@@ -176,6 +191,7 @@ static bool processFunction(SILFunction &F, bool FreezePostDomRelease,
     return false;
   }
 
+  CodeMotionOrDeleteCallback Callback;
   // Until we do not remove any instructions or have nested increments,
   // decrements...
   while (true) {
@@ -185,19 +201,10 @@ static bool processFunction(SILFunction &F, bool FreezePostDomRelease,
     // We need to blot pointers we remove after processing an individual pointer
     // so we don't process pairs after we have paired them up. Thus we pass in a
     // lambda that performs the work for us.
-    bool ShouldRunAgain = computeARCMatchingSet(Ctx, FreezePostDomRelease,
-      // Remove the increments, decrements and insert new increments, decrements
-      // at the insertion points associated with a specific pointer.
-      [&Changed, &InstructionsToDelete](ARCMatchingSet &Set) {
-        Changed |= optimizeReferenceCountMatchingSet(Set, InstructionsToDelete);
-      }
-    );
+    bool ShouldRunAgain =
+        computeARCMatchingSet(Ctx, FreezePostDomRelease, Callback);
 
-    // Now that it is safe to do so and avoid iterator invalidation, delete all
-    // instructions from the delete list.
-    while (!InstructionsToDelete.empty()) {
-      InstructionsToDelete.pop_back_val()->eraseFromParent();
-    }
+    Changed |= Callback.madeChange();
 
     // If we did not remove any instructions or have any nested increments, do
     // not perform another iteration.