[loop-arc] Change Loop ARC Pairing Context to be a Loop Visitor

Author: gottesmm

lib/SILPasses/ARC/ARCSequenceOpts.cpp

@@ -77,27 +77,9 @@ static SILInstruction *createDecrement(SILValue Ptr, SILInstruction *InsertPt) {
   return B.createReleaseValue(Loc, Ptr);
 }
 
-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; }
-};
-}
+//===----------------------------------------------------------------------===//
+//                             Mutating Callback
+//===----------------------------------------------------------------------===//
 
 // This routine takes in the ARCMatchingSet \p MatchSet and inserts new
 // increments, decrements at the insertion points and adds the old increment,
@@ -222,35 +204,8 @@ processFunctionWithLoopSupport(SILFunction &F, bool FreezePostDomReleases,
 
   DEBUG(llvm::dbgs() << "***** Processing " << F.getName() << " *****\n");
 
-  bool Changed = false;
   LoopARCPairingContext Context(F, AA, LRFI, LI, RCFI);
-  CodeMotionOrDeleteCallback Callback;
-  // Until we do not remove any instructions or have nested increments,
-  // decrements...
-  while (true) {
-    // Compute matching sets of increments, decrements, and their insertion
-    // points.
-    //
-    // 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 = Context.run(FreezePostDomReleases, Callback);
-
-    Changed |= Callback.madeChange();
-
-    // If we did not remove any instructions or have any nested increments, do
-    // not perform another iteration.
-    if (!ShouldRunAgain)
-      break;
-
-    // Otherwise, perform another iteration.
-    DEBUG(llvm::dbgs() << "\n<<< Made a Change! Reprocessing Function! >>>\n");
-  }
-
-  DEBUG(llvm::dbgs() << "\n");
-
-  // Return true if we moved or deleted any instructions.
-  return Changed;
+  return Context.process(FreezePostDomReleases);
 }
 
 //===----------------------------------------------------------------------===//

lib/SILPasses/ARC/GlobalARCPairingAnalysis.cpp

@@ -360,7 +360,7 @@ bool ARCMatchingSetBuilder::matchUpIncDecSetsForPtr() {
 //                            ARC Pairing Context
 //===----------------------------------------------------------------------===//
 
-bool ARCPairingContext::performMatching(ARCMatchingSetCallback &Callback) {
+bool ARCPairingContext::performMatching(CodeMotionOrDeleteCallback &Callback) {
   bool MatchedPair = false;
 
   DEBUG(llvm::dbgs() << "**** Computing ARC Matching Sets for " << F.getName()
@@ -404,9 +404,15 @@ bool ARCPairingContext::performMatching(ARCMatchingSetCallback &Callback) {
 //                                  Loop ARC
 //===----------------------------------------------------------------------===//
 
-void LoopARCPairingContext::processLoop(const LoopRegion *Region,
-                                        bool FreezePostDomReleases,
-                                        ARCMatchingSetCallback &Callback) {
+void LoopARCPairingContext::runOnLoop(SILLoop *L) {
+  processRegion(LRFI->getRegion(L));
+}
+
+void LoopARCPairingContext::runOnFunction(SILFunction *F) {
+  processRegion(LRFI->getTopLevelRegion());
+}
+
+void LoopARCPairingContext::processRegion(const LoopRegion *Region) {
   bool NestingDetected = Evaluator.runOnLoop(Region, FreezePostDomReleases);
   bool MatchedPair = Context.performMatching(Callback);
   Context.DecToIncStateMap.clear();
@@ -422,35 +428,3 @@ void LoopARCPairingContext::processLoop(const LoopRegion *Region,
 
   Evaluator.summarizeLoop(Region);
 }
-
-bool LoopARCPairingContext::run(bool FreezePostDomReleases,
-                                ARCMatchingSetCallback &Callback) {
-  // We visit the loop nest inside out via a depth first, post order using this
-  // worklist.
-  llvm::SmallVector<std::pair<SILLoop *, bool>, 32> Worklist;
-  for (auto *L : SLI->getTopLevelLoops()) {
-    Worklist.push_back({L, L->empty()});
-  }
-
-  while (Worklist.size()) {
-    SILLoop *L;
-    bool Visited;
-    std::tie(L, Visited) = Worklist.pop_back_val();
-
-    if (!Visited) {
-      Worklist.push_back({L, true});
-      for (auto *SubLoop : L->getSubLoops()) {
-        Worklist.push_back({SubLoop, SubLoop->empty()});
-      }
-      continue;
-    }
-
-    processLoop(LRFI->getRegion(L), FreezePostDomReleases, Callback);
-  }
-
-  processLoop(LRFI->getTopLevelRegion(), FreezePostDomReleases, Callback);
-
-  // We always return false since we are iterating internally rather than
-  // externally.
-  return false;
-}

lib/SILPasses/ARC/GlobalARCPairingAnalysis.h

@@ -16,6 +16,7 @@
 #include "GlobalARCSequenceDataflow.h"
 #include "GlobalLoopARCSequenceDataflow.h"
 #include "swift/SIL/SILValue.h"
+#include "swift/SILPasses/Utils/LoopUtils.h"
 #include "llvm/ADT/SetVector.h"
 
 namespace swift {
@@ -31,13 +32,31 @@ class RCIdentityFunctionInfo;
 /// A set of matching reference count increments, decrements, increment
 /// insertion pts, and decrement insertion pts.
 struct ARCMatchingSet {
+
+  /// The pointer that this ARCMatchingSet is providing matching increment and
+  /// decrement sets for.
+  ///
+  /// TODO: This should really be called RCIdentity.
   SILValue Ptr;
+
+  /// The set of reference count increments that were paired.
   llvm::SetVector<SILInstruction *> Increments;
+
+  /// An insertion point for an increment means the earliest point in the
+  /// program after the increment has occured that the increment can be moved to
+  /// without moving the increment over an instruction that may decrement a
+  /// reference count.
   llvm::SetVector<SILInstruction *> IncrementInsertPts;
+
+  /// The set of reference count decrements that were paired.
   llvm::SetVector<SILInstruction *> Decrements;
+
+  /// An insertion point for a decrement means the latest point in the program
+  /// before the decrement that the optimizer conservatively assumes that a
+  /// reference counted value could be used.
   llvm::SetVector<SILInstruction *> DecrementInsertPts;
 
-  // This is a data structure that can not be moved.
+  // This is a data structure that can not be moved or copied.
   ARCMatchingSet() = default;
   ARCMatchingSet(const ARCMatchingSet &) = delete;
   ARCMatchingSet(ARCMatchingSet &&) = delete;
@@ -53,24 +72,31 @@ struct ARCMatchingSet {
   }
 };
 
-/// 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 {
-  virtual ~ARCMatchingSetCallback() = default;
+class CodeMotionOrDeleteCallback {
+  bool Changed = false;
+  llvm::SmallVector<SILInstruction *, 16> InstructionsToDelete;
 
+public:
   /// 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) {}
+  void processMatchingSet(ARCMatchingSet &Set);
+
+  // 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.
+  void finalize() {
+    while (!InstructionsToDelete.empty()) {
+      InstructionsToDelete.pop_back_val()->eraseFromParent();
+    }
+  }
 
-  /// This call should perform any deletion of instructions necessary. It is run
-  /// strictly after all computation has been completed.
-  virtual void finalize() {}
+  bool madeChange() const { return Changed; }
 };
 
+/// A wrapper around the results of the bottomup/topdown dataflow that knows how
+/// to pair the retains/releases in those results.
 struct ARCPairingContext {
   SILFunction &F;
   BlotMapVector<SILInstruction *, TopDownRefCountState> DecToIncStateMap;
@@ -79,9 +105,14 @@ struct ARCPairingContext {
 
   ARCPairingContext(SILFunction &F, RCIdentityFunctionInfo *RCIA)
       : F(F), DecToIncStateMap(), IncToDecStateMap(), RCIA(RCIA) {}
-  bool performMatching(ARCMatchingSetCallback &Callback);
+  bool performMatching(CodeMotionOrDeleteCallback &Callback);
 };
 
+/// A composition of an ARCSequenceDataflowEvaluator and an
+/// ARCPairingContext. The evaluator performs top down/bottom up dataflows
+/// clearing the dataflow at loop boundaries. Then the results of the evaluator
+/// are placed into the ARCPairingContext and then the ARCPairingContext is used
+/// to pair retains/releases.
 struct BlockARCPairingContext {
   ARCPairingContext Context;
   ARCSequenceDataflowEvaluator Evaluator;
@@ -91,7 +122,7 @@ struct BlockARCPairingContext {
       : Context(F, RCFI), Evaluator(F, AA, POTA, RCFI, Context.DecToIncStateMap,
                                     Context.IncToDecStateMap) {}
 
-  bool run(bool FreezePostDomReleases, ARCMatchingSetCallback &Callback) {
+  bool run(bool FreezePostDomReleases, CodeMotionOrDeleteCallback &Callback) {
     bool NestingDetected = Evaluator.run(FreezePostDomReleases);
     Evaluator.clear();
 
@@ -100,23 +131,36 @@ struct BlockARCPairingContext {
   }
 };
 
-struct LoopARCPairingContext {
+/// A composition of a LoopARCSequenceDataflowEvaluator and an
+/// ARCPairingContext. The loop nest is processed bottom up. For each loop, we
+/// run the evaluator on the loop and then use the ARCPairingContext to pair
+/// retains/releases and eliminate them.
+struct LoopARCPairingContext : SILLoopVisitor {
   ARCPairingContext Context;
   LoopARCSequenceDataflowEvaluator Evaluator;
   LoopRegionFunctionInfo *LRFI;
   SILLoopInfo *SLI;
+  CodeMotionOrDeleteCallback Callback;
+  bool FreezePostDomReleases = false;
 
   LoopARCPairingContext(SILFunction &F, AliasAnalysis *AA,
                         LoopRegionFunctionInfo *LRFI, SILLoopInfo *SLI,
                         RCIdentityFunctionInfo *RCFI)
-      : Context(F, RCFI),
+      : SILLoopVisitor(&F, SLI), Context(F, RCFI),
         Evaluator(F, AA, LRFI, SLI, RCFI, Context.DecToIncStateMap,
                   Context.IncToDecStateMap),
-        LRFI(LRFI), SLI(SLI) {}
+        LRFI(LRFI), SLI(SLI), Callback() {}
+
+  bool process(bool FreezePDReleases) {
+    FreezePostDomReleases = FreezePDReleases;
+    run();
+    return Callback.madeChange();
+  }
+
+  void runOnLoop(SILLoop *L) override;
+  void runOnFunction(SILFunction *F) override;
 
-  bool run(bool FreezePostDomReleases, ARCMatchingSetCallback &Callback);
-  void processLoop(const LoopRegion *R, bool FreezePostDomReleases,
-                   ARCMatchingSetCallback &Callback);
+  void processRegion(const LoopRegion *R);
 };
 
 } // end swift namespace