Revert "Fix post-dominator tree in stack promotion"

This broke the test suite under optimizations with a SIL verifier error: "stack dealloc does
not match most recent stack alloc".

This reverts commit 7a2ca23, reversing
changes made to 4c55e8d.

Author: jrose-apple

lib/SILOptimizer/Transforms/StackPromotion.cpp

@@ -17,9 +17,6 @@
 #include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBuilder.h"
-#include "swift/SIL/CFG.h"
-#include "llvm/Support/GenericDomTree.h"
-#include "llvm/Support/GenericDomTreeConstruction.h"
 #include "llvm/ADT/Statistic.h"
 
 STATISTIC(NumStackPromoted, "Number of objects promoted to the stack");
@@ -51,23 +48,9 @@ class StackPromoter {
   SILFunction *F;
   EscapeAnalysis::ConnectionGraph *ConGraph;
   DominanceInfo *DT;
+  PostDominanceInfo *PDT;
   EscapeAnalysis *EA;
 
-  // We use our own post-dominator tree instead of PostDominatorAnalysis,
-  // because we ignore unreachable blocks (actually all unreachable sub-graphs).
-  // Example:
-  //              |
-  //             bb1
-  //            /   \
-  //   unreachable  bb2
-  //                  \
-  //
-  // We want to get bb2 as immediate post-domiator of bb1. This is not the case
-  // with the regualar post-dominator tree.
-  llvm::DominatorTreeBase<SILBasicBlock> PostDomTree;
-
-  bool PostDomTreeValid;
-
   // Pseudo-functions for (de-)allocating array buffers on the stack.
 
   SILFunction *BufferAllocFunc = nullptr;
@@ -142,41 +125,25 @@ class StackPromoter {
   }
 
   bool strictlyPostDominates(SILBasicBlock *A, SILBasicBlock *B) {
-    calculatePostDomTree();
-    return A != B && PostDomTree.dominates(A, B);
-  }
-
-  bool postDominates(SILBasicBlock *A, SILBasicBlock *B) {
-    calculatePostDomTree();
-    return PostDomTree.dominates(A, B);
+    return A != B && PDT->dominates(A, B);
   }
 
   SILBasicBlock *getImmediatePostDom(SILBasicBlock *BB) {
-    calculatePostDomTree();
-    auto *Node = PostDomTree.getNode(BB);
+    auto *Node = PDT->getNode(BB);
     if (!Node)
       return nullptr;
     auto *IDomNode = Node->getIDom();
     if (!IDomNode)
       return nullptr;
     return IDomNode->getBlock();
   }
-  
-  void calculatePostDomTree() {
-    if (!PostDomTreeValid) {
-      // The StackPromoter acts as a "graph" for which the post-dominator-tree
-      // is calculated.
-      PostDomTree.recalculate(*this);
-      PostDomTreeValid = true;
-    }
-  }
 
 public:
 
   StackPromoter(SILFunction *F, EscapeAnalysis::ConnectionGraph *ConGraph,
-                DominanceInfo *DT, EscapeAnalysis *EA) :
-    F(F), ConGraph(ConGraph), DT(DT), EA(EA), PostDomTree(true),
-    PostDomTreeValid(false) { }
+                DominanceInfo *DT, PostDominanceInfo *PDT,
+                EscapeAnalysis *EA) :
+    F(F), ConGraph(ConGraph), DT(DT), PDT(PDT), EA(EA) { }
 
   /// What did the optimization change?
   enum class ChangeState {
@@ -185,8 +152,6 @@ class StackPromoter {
     Calls
   };
 
-  SILFunction *getFunction() const { return F; }
-
   /// The main entry point for the optimization.
   ChangeState promote();
 };
@@ -319,87 +284,6 @@ SILFunction *StackPromoter::getBufferDeallocFunc(SILFunction *OrigFunc,
   return BufferDeallocFunc;
 }
 
-namespace {
-
-/// Iterator which iterates over all basic blocks of a function which are not
-/// terminated by an unreachable inst.
-class NonUnreachableBlockIter :
-public std::iterator<std::forward_iterator_tag, SILBasicBlock, ptrdiff_t> {
-
-  SILFunction::iterator BaseIterator;
-  SILFunction::iterator End;
-
-  void skipUnreachables() {
-    while (true) {
-      if (BaseIterator == End)
-        return;
-      if (!isa<UnreachableInst>(BaseIterator->getTerminator()))
-        return;
-      BaseIterator++;
-    }
-  }
-
-public:
-  NonUnreachableBlockIter(SILFunction::iterator BaseIterator,
-                          SILFunction::iterator End) :
-      BaseIterator(BaseIterator), End(End) {
-    skipUnreachables();
-  }
-
-  NonUnreachableBlockIter() = default;
-  
-  SILBasicBlock &operator*() const { return *BaseIterator; }
-  SILBasicBlock &operator->() const { return *BaseIterator; }
-  
-  NonUnreachableBlockIter &operator++() {
-    BaseIterator++;
-    skipUnreachables();
-    return *this;
-  }
-  
-  NonUnreachableBlockIter operator++(int unused) {
-    NonUnreachableBlockIter Copy = *this;
-    ++*this;
-    return Copy;
-  }
-
-  friend bool operator==(NonUnreachableBlockIter lhs,
-                         NonUnreachableBlockIter rhs) {
-    return lhs.BaseIterator == rhs.BaseIterator;
-  }
-  friend bool operator!=(NonUnreachableBlockIter lhs,
-                         NonUnreachableBlockIter rhs) {
-    return !(lhs == rhs);
-  }
-};
-}
-
-namespace llvm {
-
-/// Use the StackPromoter as a wrapper for the function. It holds the list of
-/// basic blocks excluding all unreachable blocks.
-template <> struct GraphTraits<StackPromoter *>
-    : public GraphTraits<swift::SILBasicBlock*> {
-  typedef StackPromoter *GraphType;
-
-  static NodeType *getEntryNode(GraphType SP) {
-    return &SP->getFunction()->front();
-  }
-
-  typedef NonUnreachableBlockIter nodes_iterator;
-  static nodes_iterator nodes_begin(GraphType SP) {
-    return nodes_iterator(SP->getFunction()->begin(), SP->getFunction()->end());
-  }
-  static nodes_iterator nodes_end(GraphType SP) {
-    return nodes_iterator(SP->getFunction()->end(), SP->getFunction()->end());
-  }
-  static unsigned size(GraphType SP) {
-    return std::distance(nodes_begin(SP), nodes_end(SP));
-  }
-};
-
-}
-
 bool StackPromoter::canPromoteAlloc(SILInstruction *AI,
                                     SILInstruction *&AllocInsertionPoint,
                                     SILInstruction *&DeallocInsertionPoint) {
@@ -510,7 +394,7 @@ bool StackPromoter::canPromoteAlloc(SILInstruction *AI,
           if (!Alloc)
             return false;
           // This should always be the case, but let's be on the safe side.
-          if (!postDominates(StartBlock, Alloc->getParent()))
+          if (!PDT->dominates(StartBlock, Alloc->getParent()))
             return false;
           AllocInsertionPoint = Alloc;
           StackDepth++;
@@ -582,18 +466,20 @@ class StackPromotion : public SILFunctionTransform {
 
     auto *EA = PM->getAnalysis<EscapeAnalysis>();
     auto *DA = PM->getAnalysis<DominanceAnalysis>();
+    auto *PDA = PM->getAnalysis<PostDominanceAnalysis>();
 
     SILFunction *F = getFunction();
     if (auto *ConGraph = EA->getConnectionGraph(F)) {
-      StackPromoter promoter(F, ConGraph, DA->get(F), EA);
+      StackPromoter promoter(F, ConGraph, DA->get(F), PDA->get(F), EA);
       switch (promoter.promote()) {
         case StackPromoter::ChangeState::None:
           break;
         case StackPromoter::ChangeState::Insts:
           invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
           break;
-        case StackPromoter::ChangeState::Calls:
+        case StackPromoter::ChangeState::Calls: {
           invalidateAnalysis(SILAnalysis::InvalidationKind::CallsAndInstructions);
+        }
           break;
       }
     }

lib/SILOptimizer/Utils/Local.cpp

@@ -1134,7 +1134,7 @@ SILInstruction *ValueLifetimeAnalysis:: findLastUserInBlock(SILBasicBlock *BB) {
 bool ValueLifetimeAnalysis::computeFrontier(Frontier &Fr, Mode mode) {
   bool NoCriticalEdges = true;
 
-  // Exit-blocks from the lifetime region. The value is live at the end of
+  // Exit-blocks from the lifetime region. The value if live at the end of
   // a predecessor block but not in the frontier block itself.
   llvm::SmallSetVector<SILBasicBlock *, 16> FrontierBlocks;
 

test/SILOptimizer/stack_promotion.sil

@@ -88,49 +88,6 @@ bb0:
   return %t : $()
 }
 
-// CHECK-LABEL: sil @promote_with_unreachable_block
-// CHECK: [[O:%[0-9]+]] = alloc_ref [stack] $XX
-// CHECK: bb1:
-// CHECK-NEXT: unreachable
-// CHECK: bb2:
-// CHECK: strong_release
-// CHECK: dealloc_ref [stack] [[O]] : $XX
-// CHECK: return
-sil @promote_with_unreachable_block : $@convention(thin) () -> Int32 {
-bb0:
-  %o1 = alloc_ref $XX
-  %f1 = function_ref @xx_init : $@convention(thin) (@guaranteed XX) -> XX
-  %n1 = apply %f1(%o1) : $@convention(thin) (@guaranteed XX) -> XX
-  cond_br undef, bb1, bb2
-
-bb1:
-  unreachable
-
-bb2:
-  %l1 = ref_element_addr %n1 : $XX, #XX.x
-  %l2 = load %l1 : $*Int32
-  strong_release %n1 : $XX
-  return %l2 : $Int32
-}
-
-// CHECK-LABEL: sil @no_return_function
-// Just check that we don't crash on this.
-// It's a corner case, so we don't care if stack promotion is done or not.
-// CHECK: unreachable
-sil @no_return_function : $@convention(thin) () -> Int32 {
-bb0:
-  %o1 = alloc_ref $XX
-  %f1 = function_ref @xx_init : $@convention(thin) (@guaranteed XX) -> XX
-  %n1 = apply %f1(%o1) : $@convention(thin) (@guaranteed XX) -> XX
-  br bb1
-
-bb1:
-  %l1 = ref_element_addr %n1 : $XX, #XX.x
-  %l2 = load %l1 : $*Int32
-  strong_release %n1 : $XX
-  unreachable
-}
-
 // CHECK-LABEL: sil @promote_in_loop_with_if
 // CHECK: [[O:%[0-9]+]] = alloc_ref [stack] $XX
 // CHECK: {{^}}bb4({{.*}}):