Merge pull request #41358 from nate-chandler/lexical_lifetimes/shrink-borrow-scope/adopt-reachability

[ShrinkBorrowScope] Adopt BackwardReachability.

Author: nate-chandler

include/swift/SIL/SILBasicBlock.h

@@ -426,6 +426,12 @@ public SwiftObjectHeader {
   /// the debug scope for newly created instructions.
   const SILDebugScope *getScopeOfFirstNonMetaInstruction();
 
+  /// Whether the block has any phi arguments.
+  ///
+  /// Note that a block could have an argument and still return false.  The
+  /// argument must also satisfy SILPhiArgument::isPhiArgument.
+  bool hasPhi() const;
+
   //===--------------------------------------------------------------------===//
   // Debugging
   //===--------------------------------------------------------------------===//

include/swift/SILOptimizer/Analysis/Reachability.h

@@ -26,6 +26,8 @@
 
 #include "swift/SIL/BasicBlockDatastructures.h"
 #include "swift/SIL/SILBasicBlock.h"
+#include "swift/SIL/SILSuccessor.h"
+#include "llvm/ADT/STLExtras.h"
 
 namespace swift {
 
@@ -44,16 +46,19 @@ namespace swift {
 ///   // Mark the beginning of a block reachable. Only called once per block.
 ///   // Typically a BasicBlockSet wrapper.
 ///   void markReachableBegin(SILBasicBlock *block);
-/// 
+///
 ///   // Mark the end of a block reachable. Only called once per block.
 ///   // Typically a BasicBlockSet wrapper.
 ///   void markReachableEnd(SILBasicBlock *block);
 ///
 ///   // Return true if \p inst is a barrier. Called once for each reachable
-///   // instruction, assuming that each lastUse is itself a barrier.
+///   // instruction, assuming that each lastUsePoint is itself a barrier.
 ///   // Used by the data flow client to perform additional book-keeping,
 ///   // such as recording debug_value instructions.
 ///   bool checkReachableBarrier(SILInstruction *inst);
+///
+///   // Return true if any of \p block's phis are barriers.
+///   bool checkReachablePhiBarrier(SILBasicBlock *block);
 /// };
 template <typename BlockReachability>
 class BackwardReachability {
@@ -70,7 +75,8 @@ class BackwardReachability {
   // Initialize data flow starting points before running solveBackward.
   void initLastUse(SILInstruction *lastUsePoint) {
     auto *lastUseBlock = lastUsePoint->getParent();
-    if (canReachBlockBegin(lastUsePoint)) {
+    auto *previous = lastUsePoint->getPreviousInstruction();
+    if (!previous || canReachBlockBegin(previous)) {
       pushPreds(lastUseBlock);
     }
   }
@@ -116,6 +122,10 @@ class BackwardReachability {
 
   // Propagate global data flow from \p succBB to its predecessors.
   void pushPreds(SILBasicBlock *succBB) {
+    if (succBB->hasPhi())
+      if (reachableBlocks.checkReachablePhiBarrier(succBB))
+        return;
+
     reachableBlocks.markReachableBegin(succBB);
 
     for (SILBasicBlock *predBB : succBB->getPredecessorBlocks()) {

include/swift/SILOptimizer/Utils/CanonicalizeBorrowScope.h

@@ -148,7 +148,7 @@ class CanonicalizeBorrowScope {
 };
 
 bool shrinkBorrowScope(
-    BeginBorrowInst *bbi, InstructionDeleter &deleter,
+    BeginBorrowInst const &bbi, InstructionDeleter &deleter,
     SmallVectorImpl<CopyValueInst *> &modifiedCopyValueInsts);
 
 bool foldDestroysOfCopiedLexicalBorrow(BeginBorrowInst *bbi,

lib/SIL/IR/SILBasicBlock.cpp

@@ -395,6 +395,15 @@ bool SILBasicBlock::isLegalToHoistInto() const {
   return true;
 }
 
+bool SILBasicBlock::hasPhi() const {
+  if (getArguments().size() == 0)
+    return false;
+  // It is sufficient to check whether the first argument is a phi.  A block
+  // can't have both phis and terminator results.
+  auto *argument = getArguments()[0];
+  return argument->isPhiArgument();
+}
+
 const SILDebugScope *SILBasicBlock::getScopeOfFirstNonMetaInstruction() {
   for (auto &Inst : *this)
     if (Inst.isMetaInstruction())

lib/SILOptimizer/Transforms/CopyPropagation.cpp

@@ -451,7 +451,7 @@ void CopyPropagation::run() {
     // at least once and then until each stops making changes.
     while (true) {
       SmallVector<CopyValueInst *, 4> modifiedCopyValueInsts;
-      auto shrunk = shrinkBorrowScope(bbi, deleter, modifiedCopyValueInsts);
+      auto shrunk = shrinkBorrowScope(*bbi, deleter, modifiedCopyValueInsts);
       for (auto *cvi : modifiedCopyValueInsts)
         defWorklist.updateForCopy(cvi);
       changed |= shrunk;

lib/SILOptimizer/Transforms/SSADestroyHoisting.cpp

@@ -215,6 +215,8 @@ class DeinitBarriers {
     DeinitBarriers &result;
     SILInstruction *storageDefInst = nullptr; // null for function args
 
+    enum class Classification { DeadUser, Barrier, Other };
+
     BackwardReachability<DestroyReachability> reachability;
 
   public:
@@ -245,37 +247,87 @@ class DeinitBarriers {
       result.destroyReachesEndBlocks.insert(block);
     }
 
-    bool checkReachableBarrier(SILInstruction *inst);
+    Classification classifyInstruction(SILInstruction *inst);
+
+    bool classificationIsBarrier(Classification classification);
+
+    void visitedInstruction(SILInstruction *instruction,
+                            Classification classification);
+
+    bool checkReachableBarrier(SILInstruction *);
+
+    bool checkReachablePhiBarrier(SILBasicBlock *);
 
     void solveBackward() { reachability.solveBackward(); }
   };
 };
 
-/// Return true if \p inst is a barrier.
-///
-/// Called exactly once for each reachable instruction. This is guaranteed to
-/// hold as a barrier occurs between any original destroys that are reachable
-/// from each. Any path reaching multiple destroys requires initialization,
-/// which is a storageUser and therefore a barrier.
-bool DeinitBarriers::DestroyReachability::checkReachableBarrier(
-    SILInstruction *inst) {
+DeinitBarriers::DestroyReachability::Classification
+DeinitBarriers::DestroyReachability::classifyInstruction(SILInstruction *inst) {
   if (knownUses.debugInsts.contains(inst)) {
-    result.deadUsers.push_back(inst);
-    return false;
+    return Classification::DeadUser;
   }
   if (inst == storageDefInst) {
     result.barriers.push_back(inst);
-    return true;
+    return Classification::Barrier;
   }
   if (knownUses.storageUsers.contains(inst)) {
-    result.barriers.push_back(inst);
-    return true;
+    return Classification::Barrier;
   }
   if (isDeinitBarrier(inst)) {
-    result.barriers.push_back(inst);
+    return Classification::Barrier;
+  }
+  return Classification::Other;
+}
+
+bool DeinitBarriers::DestroyReachability::classificationIsBarrier(
+    Classification classification) {
+  switch (classification) {
+  case Classification::DeadUser:
+  case Classification::Other:
+    return false;
+  case Classification::Barrier:
     return true;
   }
-  return false;
+  llvm_unreachable("exhaustive switch is not exhaustive?!");
+}
+
+void DeinitBarriers::DestroyReachability::visitedInstruction(
+    SILInstruction *instruction, Classification classification) {
+  assert(classifyInstruction(instruction) == classification);
+  switch (classification) {
+  case Classification::DeadUser:
+    result.deadUsers.push_back(instruction);
+    break;
+  case Classification::Barrier:
+    result.barriers.push_back(instruction);
+    break;
+  case Classification::Other:
+    break;
+  }
+}
+
+/// Return true if \p inst is a barrier.
+///
+/// Called exactly once for each reachable instruction. This is guaranteed to
+/// hold as a barrier occurs between any original destroys that are reachable
+/// from each. Any path reaching multiple destroys requires initialization,
+/// which is a storageUser and therefore a barrier.
+bool DeinitBarriers::DestroyReachability::checkReachableBarrier(
+    SILInstruction *instruction) {
+  auto classification = classifyInstruction(instruction);
+  visitedInstruction(instruction, classification);
+  return classificationIsBarrier(classification);
+}
+
+bool DeinitBarriers::DestroyReachability::checkReachablePhiBarrier(
+    SILBasicBlock *block) {
+  assert(llvm::all_of(block->getArguments(),
+                      [&](auto argument) { return PhiValue(argument); }));
+  return llvm::any_of(block->getPredecessorBlocks(), [&](auto *predecessor) {
+    return classificationIsBarrier(
+        classifyInstruction(predecessor->getTerminator()));
+  });
 }
 
 /// Algorithm for hoisting the destroys of a single uniquely identified storage