Merge pull request #4183 from trentxintong/RRCM

Small refactoring in RRCM.

Author: trentxintong

include/swift/SILOptimizer/Analysis/ARCAnalysis.h

@@ -39,6 +39,11 @@ class SILFunction;
 } // end namespace swift
 
 namespace swift {
+/// Return true if this is a retain instruction.
+bool isRetainInstruction(SILInstruction *II);
+
+/// Return true if this is a release instruction.
+bool isReleaseInstruction(SILInstruction *II);
 
 using RetainList = llvm::SmallVector<SILInstruction *, 1>;
 using ReleaseList = llvm::SmallVector<SILInstruction *, 1>;

lib/SILOptimizer/Analysis/ARCAnalysis.cpp

@@ -31,6 +31,19 @@ using namespace swift;
 
 using BasicBlockRetainValue = std::pair<SILBasicBlock *, SILValue>;
 
+//===----------------------------------------------------------------------===//
+//                             Utility Analysis
+//===----------------------------------------------------------------------===//
+
+bool swift::isRetainInstruction(SILInstruction *I) {
+  return isa<StrongRetainInst>(I) || isa<RetainValueInst>(I);
+}
+
+
+bool swift::isReleaseInstruction(SILInstruction *I) {
+  return isa<StrongReleaseInst>(I) || isa<ReleaseValueInst>(I);
+}
+
 //===----------------------------------------------------------------------===//
 //                             Decrement Analysis
 //===----------------------------------------------------------------------===//

lib/SILOptimizer/Transforms/RetainReleaseCodeMotion.cpp

@@ -40,11 +40,11 @@
 /// Region B to Region A, that means there is no blocking operation from where
 /// the release was in Region B and where the release is hoisted to in Region A.
 ///
-/// The question is whether we can introduce such operation while we hoist other
-/// releases. The answer is NO. because if such releases exist, they would be
-/// blocked by the old release (we remove old release and recreate new ones at
-/// the end of the pass) and will not be able to be hoisted beyond the old 
-/// release.
+/// The question is whether we can introduce such operation while we hoist
+/// other releases. The answer is NO. because if such releases exist, they
+/// would be blocked by the old release (we remove old release and recreate new
+/// ones at the end of the pass) and will not be able to be hoisted beyond the
+/// old release.
 ///
 /// This proof also hinges on the fact that if release A interferes with
 /// releases B then release B must interfere with release A. i.e. the 2
@@ -64,19 +64,18 @@
 ///
 /// TODO: There are a lot of code duplications between retain and release code
 /// motion in the data flow part. Consider whether we can share them.
-/// Essentially, we can implement the release code motion by inverting the retain
-/// code motion, but this can also make the code less readable.
+/// Essentially, we can implement the release code motion by inverting the 
+/// retain code motion, but this can also make the code less readable.
 ///
 //===----------------------------------------------------------------------===//
 
 #define DEBUG_TYPE "sil-rr-code-motion"
 #include "swift/SIL/SILBuilder.h"
 #include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
+#include "swift/SILOptimizer/Analysis/ARCAnalysis.h"
 #include "swift/SILOptimizer/Analysis/EscapeAnalysis.h"
-#include "swift/SILOptimizer/Analysis/LoopAnalysis.h"
 #include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
 #include "swift/SILOptimizer/Analysis/RCIdentityAnalysis.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"
@@ -94,19 +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));
-llvm::cl::opt<bool> DisableIfWithCriticalEdge("disable-with-critical-edge", llvm::cl::init(false));
-
-//===----------------------------------------------------------------------===//
-//                             Utility 
-//===----------------------------------------------------------------------===//
-
-static bool isRetainInstruction(SILInstruction *I) {
-  return isa<StrongRetainInst>(I) || isa<RetainValueInst>(I);
-}
-
-static bool isReleaseInstruction(SILInstruction *I) {
-  return isa<StrongReleaseInst>(I) || isa<ReleaseValueInst>(I);
-}
+llvm::cl::opt<bool>
+DisableIfWithCriticalEdge("disable-with-critical-edge", llvm::cl::init(false));
 
 //===----------------------------------------------------------------------===//
 //                             Block State 
@@ -199,13 +187,14 @@ class CodeMotionContext {
   /// retain or release.
   SILValue getRCRoot(SILInstruction *I) {
     assert(isRetainInstruction(I) || isReleaseInstruction(I) &&
-           "Extracting RC root from Invalid instruction");
+           "Extracting RC root from invalid instruction");
     return getRCRoot(I->getOperand(0));
   }
 
 public:
   /// Constructor.
-  CodeMotionContext(llvm::SpecificBumpPtrAllocator<BlockState> &BPA, SILFunction *F,
+  CodeMotionContext(llvm::SpecificBumpPtrAllocator<BlockState> &BPA,
+                    SILFunction *F,
                     PostOrderFunctionInfo *PO, AliasAnalysis *AA,
                     RCIdentityFunctionInfo *RCFI)
     : MultiIteration(true), BPA(BPA), F(F), PO(PO), AA(AA), RCFI(RCFI) {}
@@ -396,8 +385,9 @@ void RetainCodeMotionContext::initializeCodeMotionDataFlow() {
 
   // Initialize all the data flow bit vector for all basic blocks.
   for (auto &BB : *F) {
-    BlockStates[&BB] = new (BPA.Allocate()) RetainBlockState(&BB == &*F->begin(),
-                                 RCRootVault.size(), MultiIteration);
+    BlockStates[&BB] = new (BPA.Allocate())
+                            RetainBlockState(&BB == &*F->begin(),
+                            RCRootVault.size(), MultiIteration);
   }
 }
 
@@ -981,11 +971,11 @@ void ReleaseCodeMotionContext::computeCodeMotionInsertPoints() {
 namespace {
 
 /// Code motion kind.
-enum CMKind : unsigned { Retain = 0, Release = 1};
+enum CodeMotionKind : unsigned { Retain = 0, Release = 1};
 
 class RRCodeMotion : public SILFunctionTransform {
   /// Whether to hoist releases or sink retains.
-  CMKind Kind;
+  CodeMotionKind Kind;
 
   /// Freeze epilogue release or not.
   bool FreezeEpilogueReleases;
@@ -994,10 +984,11 @@ class RRCodeMotion : public SILFunctionTransform {
   StringRef getName() override { return "SIL Retain Release Code Motion"; }
 
   /// Constructor.
-  RRCodeMotion(CMKind H, bool F) : Kind(H), FreezeEpilogueReleases(F) {}
+  RRCodeMotion(CodeMotionKind H, bool F) : Kind(H), FreezeEpilogueReleases(F) {}
 
   /// The entry point to the transformation.
   void run() override {
+    // Code motion disabled.
     if (DisableRRCodeMotion)
       return;
 
@@ -1056,15 +1047,15 @@ class RRCodeMotion : public SILFunctionTransform {
 
 /// Sink Retains.
 SILTransform *swift::createRetainSinking() {
-  return new RRCodeMotion(CMKind::Retain, false);
+  return new RRCodeMotion(CodeMotionKind::Retain, false);
 }
 
 /// Hoist releases, but not epilogue release.
 SILTransform *swift::createReleaseHoisting() {
-  return new RRCodeMotion(CMKind::Release, true);
+  return new RRCodeMotion(CodeMotionKind::Release, true);
 }
 
 /// Hoist all releases.
 SILTransform *swift::createLateReleaseHoisting() {
-  return new RRCodeMotion(CMKind::Release, false);
+  return new RRCodeMotion(CodeMotionKind::Release, false);
 }

lib/SILOptimizer/Transforms/SILCodeMotion.cpp

@@ -46,10 +46,6 @@ namespace {
 //                                  Utility
 //===----------------------------------------------------------------------===//
 
-static bool isReleaseInstruction(SILInstruction *I) {
-  return isa<StrongReleaseInst>(I) || isa<ReleaseValueInst>(I);
-}
-
 static void createRefCountOpForPayload(SILBuilder &Builder, SILInstruction *I,
                                        EnumElementDecl *EnumDecl,
                                        SILValue DefOfEnum = SILValue()) {