[SILOpt] Reachability handles phis.

Previously, Reachability assumed that phis were not barriers.  Here,
handling for barrier phis is added.  To that end, a new delegate
callback `checkReachableBarrier(PhiValue)` is added.  Before marking the
beginning of a block as reached (or any of its predecessors), check
whether each argument that is a phi is a barrier.  If any is, then
reachability is done.

Implemented the new method in SSADestroyHoisting by splitting apart the
classification of an instruction and the work to do in response to
visiting an instruction.  Then, when visiting a PhiValue, just check
whether any of the predecessors terminators are classified as barriers.
That way, seeing that they're classified that way doesn't result in
noting down that those terminators had been reached (which indeed they
will not have been if any of the terminators from which the values are
flowing into the phi are barriers).

Author: nate-chandler

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()) {

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