Merge pull request #4220 from trentxintong/Refactor

A few major refacting in RRCM and DSE

Author: trentxintong

lib/SILOptimizer/Transforms/DeadStoreElimination.cpp

@@ -62,7 +62,6 @@
 #include "swift/SIL/SILBuilder.h"
 #include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
 #include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
-#include "swift/SILOptimizer/Analysis/ValueTracking.h"
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 #include "swift/SILOptimizer/Utils/CFG.h"
@@ -77,15 +76,15 @@
 
 using namespace swift;
 
+STATISTIC(NumDeadStores, "Number of dead stores removed");
+STATISTIC(NumPartialDeadStores, "Number of partial dead stores removed");
+
 /// If a large store is broken down to too many smaller stores, bail out.
 /// Currently, we only do partial dead store if we can form a single contiguous
-/// non-dead store.
+/// live store.
 static llvm::cl::opt<unsigned>
 MaxPartialStoreCount("max-partial-store-count", llvm::cl::init(1), llvm::cl::Hidden);
 
-STATISTIC(NumDeadStores, "Number of dead stores removed");
-STATISTIC(NumPartialDeadStores, "Number of partial dead stores removed");
-
 /// ComputeMaxStoreSet - If we ignore all reads, what is the max store set that
 /// can reach a particular point in a basic block. This helps in generating
 /// the genset and killset. i.e. if there is no upward visible store that can
@@ -154,7 +153,6 @@ static bool isDeadStoreInertInstruction(SILInstruction *Inst) {
 //===----------------------------------------------------------------------===//
 
 namespace {
-
 /// If this function has too many basic blocks or too many locations, it may
 /// take a long time to compute the genset and killset. The number of memory
 /// behavior or alias query we need to do in worst case is roughly linear to
@@ -295,7 +293,6 @@ bool BlockState::isTrackingLocation(llvm::SmallBitVector &BV, unsigned i) {
 //===----------------------------------------------------------------------===//
 
 namespace {
-
 /// The dead store elimination context, keep information about stores in a basic
 /// block granularity.
 class DSEContext {
@@ -326,7 +323,7 @@ enum class ProcessKind {
   llvm::SpecificBumpPtrAllocator<BlockState> &BPA;
 
   /// The epilogue release matcher we are using.
-  ConsumedArgToEpilogueReleaseMatcher& ERM;
+  ConsumedArgToEpilogueReleaseMatcher &ERM;
 
   /// Map every basic block to its location state.
   llvm::SmallDenseMap<SILBasicBlock *, BlockState *> BBToLocState;
@@ -1099,7 +1096,7 @@ void DSEContext::runIterativeDSE() {
   // Generate the genset and killset for each basic block. We can process the
   // basic blocks in any order.
   // 
-  // We also Compute the max store set at the beginning of the basic block.
+  // We also compute the max store set at the beginning of the basic block.
   //
   auto *PO = PM->getAnalysis<PostOrderAnalysis>()->get(F);
   for (SILBasicBlock *B : PO->getPostOrder()) {

lib/SILOptimizer/Transforms/RetainReleaseCodeMotion.cpp

@@ -93,6 +93,8 @@ STATISTIC(NumRetainsSunk, "Number of retains sunk");
 STATISTIC(NumReleasesHoisted, "Number of releases hoisted");
 
 llvm::cl::opt<bool> DisableRRCodeMotion("disable-rr-cm", llvm::cl::init(false));
+
+/// Disable optimization if we have to break critical edges in the function.
 llvm::cl::opt<bool>
 DisableIfWithCriticalEdge("disable-with-critical-edge", llvm::cl::init(false));
 
@@ -127,6 +129,7 @@ struct BlockState {
   /// RCRootVault. If this bit is set, that means this is potentially a retain
   /// or release that can be sunk or hoisted to this point. This is used to
   /// optimize the time for computing genset and killset.
+  ///
   /// NOTE: this vector contains an approximation of whether there will be a
   /// retain or release to a certain point of a basic block.
   llvm::SmallBitVector BBMaxSet;
@@ -275,6 +278,8 @@ class RetainBlockState : public BlockState {
   RetainBlockState(bool IsEntry, unsigned size, bool MultiIteration) {
     // Iterative forward data flow.
     BBSetIn.resize(size, false);
+    // Initilize to true if we are running optimistic data flow, i.e.
+    // MultiIteration is true.
     BBSetOut.resize(size, MultiIteration);
     BBMaxSet.resize(size, !IsEntry && MultiIteration);
 
@@ -316,7 +321,7 @@ class RetainCodeMotionContext : public CodeMotionContext {
   RetainCodeMotionContext(llvm::SpecificBumpPtrAllocator<BlockState> &BPA,
                           SILFunction *F, PostOrderFunctionInfo *PO,
                           AliasAnalysis *AA, RCIdentityFunctionInfo *RCFI)
-      : CodeMotionContext(BPA, F, PO, AA, RCFI) {
+    : CodeMotionContext(BPA, F, PO, AA, RCFI) {
     MultiIteration = requireIteration();
   }
 
@@ -597,6 +602,8 @@ class ReleaseBlockState : public BlockState {
   /// constructor.
   ReleaseBlockState(bool IsExit, unsigned size, bool MultiIteration) {
     // backward data flow.
+    // Initilize to true if we are running optimistic data flow, i.e.
+    // MultiIteration is true.
     BBSetIn.resize(size, MultiIteration);
     BBSetOut.resize(size, false);
     BBMaxSet.resize(size, !IsExit && MultiIteration);
@@ -645,10 +652,10 @@ class ReleaseCodeMotionContext : public CodeMotionContext {
                            AliasAnalysis *AA, RCIdentityFunctionInfo *RCFI,
                            bool FreezeEpilogueReleases,
                            ConsumedArgToEpilogueReleaseMatcher &ERM)
-      : CodeMotionContext(BPA, F, PO, AA, RCFI),
-        FreezeEpilogueReleases(FreezeEpilogueReleases), ERM(ERM) {
-    MultiIteration = requireIteration(); 
-  }
+    : CodeMotionContext(BPA, F, PO, AA, RCFI),
+      FreezeEpilogueReleases(FreezeEpilogueReleases), ERM(ERM) {
+    MultiIteration = requireIteration();
+  } 
 
   /// virtual destructor.
   virtual ~ReleaseCodeMotionContext() {}
@@ -717,16 +724,10 @@ void ReleaseCodeMotionContext::initializeCodeMotionDataFlow() {
   }
 
   // Initialize all the data flow bit vector for all basic blocks.
-  llvm::SmallPtrSet<SILBasicBlock *, 2> Exits;
-  auto Return = F->findReturnBB();
-  auto Throw = F->findThrowBB();
-  if (Return != F->end())
-    Exits.insert(&*Return);
-  if (Throw != F->end())
-    Exits.insert(&*Throw);
   for (auto &BB : *F) {
-    BlockStates[&BB] = new (BPA.Allocate()) ReleaseBlockState(Exits.count(&BB),
-                                            RCRootVault.size(), MultiIteration);
+    BlockStates[&BB] = new (BPA.Allocate())
+            ReleaseBlockState(BB.getTerminator()->isFunctionExiting(),
+                              RCRootVault.size(), MultiIteration);
   }
 }
 
@@ -1050,7 +1051,8 @@ SILTransform *swift::createRetainSinking() {
   return new RRCodeMotion(CodeMotionKind::Retain, false);
 }
 
-/// Hoist releases, but not epilogue release.
+/// Hoist releases, but not epilogue release. ASO relies on epilogue releases
+/// to prove knownsafety on enclosed releases.
 SILTransform *swift::createReleaseHoisting() {
   return new RRCodeMotion(CodeMotionKind::Release, true);
 }