Remove last bit of retain release code motion in SILCodeMotion. All these code are

replaced by retain release code motion. This code has been disabled for sometime now.

This should bring the retain release code motion into a close. The retain release
code motion pipeline looks like this. There could be some minor cleanups after this though.

1. We perform a global data flow for retain release code motion in RRCM (RetainReleaseCodeMotion)
2. We perform a local form of retain release code motion in SILCodeMotion. This is more
for cases which can not be handled in RRCM. e.g. sinking into a switch is more efficiently
done in a local form, the retain is not needed on the None block. Release on SILArgument needs
to be split to incoming values, this can not be done in RRCM and other cases.
3. We do not perform code motion in ASO, only elimination which are very important.

Some modifications to test cases, they look different, but functionally the same.
RRCM has this canonicalization effect, i.e. it uses the rc root, instead of
the SSA value the retain/release is currently using. As a result some test cases need
to be modified.

I also removed some test cases that do not make sense anymore and lot of duplicate test
cases between earlycodemotion.sil and latecodemotion.sil. These tests cases only have retains
and should be used to test early code motion.

Author: trentxintong

include/swift/SILOptimizer/Utils/CFG.h

@@ -115,6 +115,9 @@ SILBasicBlock *splitBasicBlockAndBranch(SILBuilder &B,
                                         SILInstruction *SplitBeforeInst,
                                         DominanceInfo *DT, SILLoopInfo *LI);
 
+/// \brief Return true if the function has a critical edge, false otherwise.
+bool hasCriticalEdges(SILFunction &F, bool OnlyNonCondBr);
+
 /// \brief Split all critical edges in the function updating the dominator tree
 /// and loop information (if they are not set to null). If \p OnlyNonCondBr is
 /// true this will not split cond_br edges (Only edges which can't carry

lib/SILOptimizer/Transforms/RetainReleaseCodeMotion.cpp

@@ -94,6 +94,7 @@ 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 
@@ -1005,6 +1006,11 @@ class RRCodeMotion : public SILFunctionTransform {
     if (!F->shouldOptimize())
       return;
 
+    // Return if there is critical edge and we are disabling critical edge
+    // splitting.
+    if (DisableIfWithCriticalEdge && hasCriticalEdges(*F, false))
+      return;
+
     DEBUG(llvm::dbgs() << "*** RRCM on function: " << F->getName() << " ***\n");
     // Split all critical edges.
     //

lib/SILOptimizer/Transforms/SILCodeMotion.cpp

@@ -852,10 +852,11 @@ static bool tryToSinkRefCountAcrossSelectEnum(CondBranchInst *CondBr,
   return true;
 }
 
-static bool tryToSinkRefCountInst(SILBasicBlock::iterator T,
-                                  SILBasicBlock::iterator I,
-                                  bool CanSinkToSuccessors, AliasAnalysis *AA,
-                                  RCIdentityFunctionInfo *RCIA) {
+static bool tryTosinkIncrementsIntoSwitchRegions(SILBasicBlock::iterator T,
+                                                 SILBasicBlock::iterator I,
+                                                 bool CanSinkToSuccessors,
+                                                 AliasAnalysis *AA,
+                                                 RCIdentityFunctionInfo *RCIA) {
   // The following methods should only be attempted if we can sink to our
   // successor.
   if (CanSinkToSuccessors) {
@@ -874,168 +875,15 @@ static bool tryToSinkRefCountInst(SILBasicBlock::iterator T,
         return true;
   }
 
-  if (!isa<StrongRetainInst>(I) && !isa<RetainValueInst>(I))
-    return false;
-
-  SILValue Ptr = I->getOperand(0);
-  if (auto B = valueHasARCDecrementOrCheckInInstructionRange(Ptr, std::next(I),
-                                                             T, AA)) {
-    DEBUG(llvm::dbgs() << "    Moving " << *I);
-    I->moveBefore(&**B);
-    return true;
-  }
-
-  // Ok, we have a ref count instruction that *could* be sunk. If we have a
-  // terminator that we cannot sink through or the cfg will not let us sink
-  // into our predecessors, just move the increment before the terminator.
-  if (!CanSinkToSuccessors ||
-      (!isa<CheckedCastBranchInst>(T) && !isa<CondBranchInst>(T))) {
-    DEBUG(llvm::dbgs() << "    Moving " << *I);
-    I->moveBefore(&*T);
-    return true;
-  }
-
-  // Ok, it is legal for us to sink this increment to our successors. Create a
-  // copy of this instruction in each one of our successors unless they are
-  // ignorable trap blocks.
-  DEBUG(llvm::dbgs() << "    Sinking " << *I);
-  SILBuilderWithScope Builder(T, &*I);
-  for (auto &Succ : T->getParent()->getSuccessors()) {
-    SILBasicBlock *SuccBB = Succ.getBB();
-
-    if (isARCInertTrapBB(SuccBB))
-      continue;
-
-    Builder.setInsertionPoint(&*SuccBB->begin());
-    if (isa<StrongRetainInst>(I)) {
-      Builder.createStrongRetain(I->getLoc(), Ptr, Atomicity::Atomic);
-    } else {
-      assert(isa<RetainValueInst>(I) && "This can only be retain_value");
-      Builder.createRetainValue(I->getLoc(), Ptr, Atomicity::Atomic);
-    }
-  }
-
-  // Then erase this instruction.
-  I->eraseFromParent();
-  NumSunk++;
-  return true;
-}
-
-static bool isRetainAvailableInSomeButNotAllPredecessors(
-    SILValue Ptr, SILBasicBlock *BB, AliasAnalysis *AA,
-    RCIdentityFunctionInfo *RCIA,
-    llvm::SmallDenseMap<SILBasicBlock *, Optional<SILInstruction *>, 4>
-        &CheckUpToInstruction) {
-  bool AvailInSome = false;
-  bool NotAvailInSome = false;
-
-  Ptr = RCIA->getRCIdentityRoot(Ptr);
-
-  // Check whether a retain on the pointer is available in the predecessors.
-  for (auto *Pred : BB->getPreds()) {
-
-    // Find the first retain of the pointer.
-    auto Retain = std::find_if(
-        Pred->rbegin(), Pred->rend(), [=](const SILInstruction &I) -> bool {
-          if (!isa<StrongRetainInst>(I) && !isa<RetainValueInst>(I))
-            return false;
-
-          return Ptr == RCIA->getRCIdentityRoot(I.getOperand(0));
-        });
-
-    // Check that there is no decrement or check from the increment to the end
-    // of the basic block. After we have hoisted the first release this release
-    // would prevent further hoisting. Instead we check that no decrement or
-    // check occurs up to this hoisted release.
-    auto End = CheckUpToInstruction[Pred];
-    auto EndIt = SILBasicBlock::iterator(End ? *End : Pred->getTerminator());
-    if (Retain == Pred->rend() || valueHasARCDecrementOrCheckInInstructionRange(
-                                      Ptr, Retain->getIterator(), EndIt, AA)) {
-      NotAvailInSome = true;
-      continue;
-    }
-
-    // Alright, the retain is 'available' for merging with a release from a
-    // successor block.
-    AvailInSome = true;
-  }
-
-  return AvailInSome && NotAvailInSome;
+  // At this point, this is a retain on a regular SSA value, leave it to retain
+  // release code motion to sink.
+  return false;
 }
 
-static bool hoistDecrementsToPredecessors(SILBasicBlock *BB, AliasAnalysis *AA,
-                                          RCIdentityFunctionInfo *RCIA) {
-  if (BB->getSinglePredecessor())
-    return false;
-
-  // Make sure we can move potential decrements to the predecessors and collect
-  // retains we could match.
-  for (auto *Pred : BB->getPreds())
-    if (!Pred->getSingleSuccessor())
-      return false;
-
-  bool HoistedDecrement = false;
-
-  // When we hoist a release to the predecessor block this release would block
-  // hoisting further releases because it looks like a ARC decrement in the
-  // predecessor block. Instead once we hoisted a release we scan only to this
-  // release when looking for ARC decrements or checks.
-  llvm::SmallDenseMap<SILBasicBlock *, Optional<SILInstruction *>, 4>
-      CheckUpToInstruction;
-
-  for (auto It = BB->begin(); It != BB->end();) {
-    auto *Inst = &*It;
-    ++It;
-
-    if (!isa<StrongReleaseInst>(Inst) && !isa<ReleaseValueInst>(Inst))
-      continue;
-
-    SILValue Ptr = Inst->getOperand(0);
-
-    // The pointer must be defined outside of this basic block.
-    if (Ptr->getParentBB() == BB)
-      continue;
-
-    // No arc use to the beginning of this block.
-    if (valueHasARCUsesInInstructionRange(Ptr, BB->begin(), Inst->getIterator(),
-                                          AA))
-      continue;
-
-    if (!isRetainAvailableInSomeButNotAllPredecessors(Ptr, BB, AA, RCIA,
-                                                      CheckUpToInstruction))
-      continue;
-
-    // Hoist decrement to predecessors.
-    DEBUG(llvm::dbgs() << "    Hoisting " << *Inst);
-    SILBuilderWithScope Builder(Inst);
-    for (auto *PredBB : BB->getPreds()) {
-      Builder.setInsertionPoint(PredBB->getTerminator());
-      SILInstruction *Release;
-      if (isa<StrongReleaseInst>(Inst)) {
-        Release =
-            Builder.createStrongRelease(Inst->getLoc(), Ptr, Atomicity::Atomic);
-      } else {
-        assert(isa<ReleaseValueInst>(Inst) && "This can only be retain_value");
-        Release =
-            Builder.createReleaseValue(Inst->getLoc(), Ptr, Atomicity::Atomic);
-      }
-      // Update the last instruction to consider when looking for ARC uses or
-      // decrements in predecessor blocks.
-      if (!CheckUpToInstruction[PredBB])
-        CheckUpToInstruction[PredBB] = Release;
-    }
-
-    Inst->eraseFromParent();
-    HoistedDecrement = true;
-  }
-
-  return HoistedDecrement;
-}
 /// Try sink a retain as far as possible.  This is either to successor BBs,
 /// or as far down the current BB as possible
-static bool sinkRefCountIncrement(SILBasicBlock *BB, AliasAnalysis *AA,
-                                  RCIdentityFunctionInfo *RCIA) {
-
+static bool sinkIncrementsIntoSwitchRegions(SILBasicBlock *BB, AliasAnalysis *AA,
+                                            RCIdentityFunctionInfo *RCIA) {
   // Make sure that each one of our successors only has one predecessor,
   // us.
   // If that condition is not true, we can still sink to the end of this BB,
@@ -1067,13 +915,16 @@ static bool sinkRefCountIncrement(SILBasicBlock *BB, AliasAnalysis *AA,
     //      terminator, sink the ref count inst into either our successors.
     //   2. If there are such decrements, move the retain right before that
     //      decrement.
-    Changed |= tryToSinkRefCountInst(S->getIterator(), Inst->getIterator(),
-                                     CanSinkToSuccessor, AA, RCIA);
+    Changed |= tryTosinkIncrementsIntoSwitchRegions(S->getIterator(),
+                                                    Inst->getIterator(),
+                                                    CanSinkToSuccessor, 
+                                                    AA, RCIA);
   }
 
   // Handle the first instruction in the BB.
   Changed |=
-      tryToSinkRefCountInst(S->getIterator(), SI, CanSinkToSuccessor, AA, RCIA);
+      tryTosinkIncrementsIntoSwitchRegions(S->getIterator(), SI,
+                                           CanSinkToSuccessor, AA, RCIA);
   return Changed;
 }
 
@@ -1747,12 +1598,17 @@ static bool processFunction(SILFunction *F, AliasAnalysis *AA,
     DEBUG(llvm::dbgs() << "    Attempting to move releases into "
           "predecessors!\n");
 
-    if (HoistReleases)
+    // Perform a relatively local forms of retain sinking and release hoisting
+    // regarding switch regions and SILargument. This are not handled by retain
+    // release code motion.
+    if (HoistReleases) {
       Changed |= State.hoistDecrementsIntoSwitchRegions(AA);
+    }
 
+    // Sink switch related retains.
+    Changed |= sinkIncrementsIntoSwitchRegions(State.getBB(), AA, RCIA);
     Changed |= State.sinkIncrementsOutOfSwitchRegions(AA, RCIA);
 
-
     // Then attempt to sink code from predecessors. This can include retains
     // which is why we always attempt to move releases up the CFG before sinking
     // code from predecessors. We will never sink the hoisted releases from
@@ -1768,15 +1624,6 @@ static bool processFunction(SILFunction *F, AliasAnalysis *AA,
     // Then perform the dataflow.
     DEBUG(llvm::dbgs() << "    Performing the dataflow!\n");
     Changed |= State.process();
-
-    // Finally we try to sink retain instructions from this BB to the next BB.
-    if (!DisableSILRRCodeMotion)
-      Changed |= sinkRefCountIncrement(State.getBB(), AA, RCIA);
-
-    // And hoist decrements to predecessors. This is beneficial if we can then
-    // match them up with an increment in some of the predecessors.
-    if (!DisableSILRRCodeMotion && HoistReleases)
-      Changed |= hoistDecrementsToPredecessors(State.getBB(), AA, RCIA);
   }
 
   return Changed;

lib/SILOptimizer/Utils/CFG.cpp

@@ -687,6 +687,23 @@ SILBasicBlock *swift::splitCriticalEdge(TermInst *T, unsigned EdgeIdx,
   return splitEdge(T, EdgeIdx, DT, LI);
 }
 
+bool swift::hasCriticalEdges(SILFunction &F, bool OnlyNonCondBr) {
+  for (SILBasicBlock &BB : F) {
+    // Only consider critical edges for terminators that don't support block
+    // arguments.
+    if (OnlyNonCondBr && isa<CondBranchInst>(BB.getTerminator()))
+      continue;
+
+    if (isa<BranchInst>(BB.getTerminator()))
+      continue;
+
+    for (unsigned Idx = 0, e = BB.getSuccessors().size(); Idx != e; ++Idx)
+      if (isCriticalEdge(BB.getTerminator(), Idx))
+        return true;
+  }
+  return false;
+}
+
 /// Split all critical edges in the function updating the dominator tree and
 /// loop information (if they are not set to null).
 bool swift::splitAllCriticalEdges(SILFunction &F, bool OnlyNonCondBr,

test/SILOptimizer/devirt_covariant_return.swift

@@ -276,7 +276,7 @@ class EEE : CCC {
 
 // Check that c.foo() is devirtualized, because the optimizer can handle the casting the return type
 // correctly, i.e. it can cast (BBB, BBB) into (AAA, AAA)
-// CHECK-LABEL: sil hidden @_TTSf4g_n___TF23devirt_covariant_return37testDevirtOfMethodReturningTupleTypesFTCS_3CCC1bCS_2BB_TCS_2AAS2__
+// CHECK-LABEL: sil hidden @_TTSf4g_g___TF23devirt_covariant_return37testDevirtOfMethodReturningTupleTypesFTCS_3CCC1bCS_2BB_TCS_2AAS2__
 // CHECK: checked_cast_br [exact] %{{.*}} : $CCC to $CCC
 // CHECK: checked_cast_br [exact] %{{.*}} : $CCC to $DDD
 // CHECK: checked_cast_br [exact] %{{.*}} : $CCC to $EEE