Fix computation of dominance frontier (#59278)

Author: meg-gupta

include/swift/SIL/Dominance.h

@@ -92,14 +92,12 @@ class DominanceInfo : public DominatorTreeBase {
 ///
 /// Precondition: no critical edges (OSSA)
 ///
-/// Postcondition: each block in \p frontier is dominated by \p root and either
-/// exits the function or has a single successor that is not dominated by \p
-/// root.
-///
-/// With no critical edges, the dominance frontier is identified simply by leaf
-/// blocks in the dominance subtree.
-void computeDominanceFrontier(SILBasicBlock *root, DominanceInfo *domTree,
-                              SmallVectorImpl<SILBasicBlock *> &frontier);
+/// Postcondition: each block in \p boundary is dominated by \p root and either
+/// exits the function or has a single successor which has a predecessor that is
+/// not dominated by \p root.
+
+void computeDominatedBoundaryBlocks(SILBasicBlock *root, DominanceInfo *domTree,
+                                    SmallVectorImpl<SILBasicBlock *> &boundary);
 
 /// Helper class for visiting basic blocks in dominance order, based on a
 /// worklist algorithm. Example usage:

lib/SIL/Utils/Dominance.cpp

@@ -142,21 +142,27 @@ void PostDominanceInfo::verify() const {
   }
 }
 
-void
-swift::computeDominanceFrontier(SILBasicBlock *root,
-                                DominanceInfo *domTree,
-                                SmallVectorImpl<SILBasicBlock *> &frontier) {
+void swift::computeDominatedBoundaryBlocks(
+    SILBasicBlock *root, DominanceInfo *domTree,
+    SmallVectorImpl<SILBasicBlock *> &boundary) {
   auto *function = root->getParent();
   assert(function->hasOwnership());
-  assert(frontier.empty());
+  assert(boundary.empty());
 
   DominanceOrder domOrder(root, domTree);
   while (SILBasicBlock *block = domOrder.getNext()) {
     DominanceInfoNode *domNode = domTree->getNode(block);
-    if (domNode->isLeaf()) {
-      frontier.push_back(block);
+    if (!domNode->isLeaf()) {
+      domOrder.pushChildren(block);
       continue;
     }
-    domOrder.pushChildren(block);
+    if (block->getNumSuccessors() == 0) {
+      boundary.push_back(block);
+      continue;
+    }
+    auto *succ = block->getSingleSuccessorBlock();
+    if (!domTree->properlyDominates(root, succ)) {
+      boundary.push_back(block);
+    }
   }
 }

lib/SILOptimizer/Mandatory/AddressLowering.cpp

@@ -1244,12 +1244,13 @@ AllocStackInst *OpaqueStorageAllocation::createStackAllocation(SILValue value) {
     deallocBuilder.createDeallocStack(pass.genLoc(), alloc);
   };
   if (latestOpeningInst) {
-    // Deallocate at the dominance frontier to ensure that allocation encloses
-    // not only the uses of the current value, but also of any values reusing
-    // this storage as a use projection.
-    SmallVector<SILBasicBlock *, 4> frontier;
-    computeDominanceFrontier(alloc->getParent(), pass.domInfo, frontier);
-    for (SILBasicBlock *deallocBlock : frontier) {
+    // Deallocate at the predecessors of dominance frontier blocks that are
+    // dominated by the alloc to ensure that allocation encloses not only the
+    // uses of the current value, but also of any values reusing this storage as
+    // a use projection.
+    SmallVector<SILBasicBlock *, 4> boundary;
+    computeDominatedBoundaryBlocks(alloc->getParent(), pass.domInfo, boundary);
+    for (SILBasicBlock *deallocBlock : boundary) {
       dealloc(deallocBlock->getTerminator()->getIterator());
     }
   } else {

test/SILOptimizer/address_lowering.sil

@@ -973,6 +973,54 @@ bb0(%0 : @owned $P):
   return %16 : $()
 }
 
+// CHECK-LABEL: sil [ossa] @f162_testOpenedArchetype :
+// CHECK:  [[STK:%.*]] = alloc_stack $Optional<@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P>
+// CHECK: bb6:
+// CHECK:   dealloc_stack [[STK]] : $*Optional<@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P>
+// CHECK:   br bb7
+// CHECK-LABEL: } // end sil function 'f162_testOpenedArchetype'
+sil [ossa] @f162_testOpenedArchetype : $@convention(thin) (@in P) -> () {
+bb0(%0 : @owned $P):
+  cond_br undef, bb1, bb5
+
+bb1:
+  %2 = alloc_stack $P, var, name "q"
+  %3 = copy_value %0 : $P
+  store %3 to [init] %2 : $*P
+  %5 = begin_borrow %0 : $P
+  %6 = open_existential_value %5 : $P to $@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P
+  %7 = witness_method $@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P, #P.foo : <Self where Self : P> (Self) -> () -> (), %6 : $@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> ()
+  %8 = copy_value %6 : $@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P
+  end_borrow %5 : $P
+  %10 = enum $Optional<@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P>, #Optional.some!enumelt, %8 : $@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P
+  %11 = unchecked_enum_data %10 : $Optional<@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P>, #Optional.some!enumelt
+  %12 = apply %7<@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P>(%11) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> ()
+  destroy_value %11 : $@opened("EF755EF2-B636-11E7-B7B4-A45E60ECC541") P
+  destroy_addr %2 : $*P
+  dealloc_stack %2 : $*P
+  cond_br undef, bb2, bb3
+
+bb2:
+  br bb4
+
+bb3:
+  br bb4
+
+bb4:
+  br bb6
+
+bb5:
+  br bb7
+
+bb6:
+  br bb7
+
+bb7:
+  destroy_value %0 : $P
+  %23 = tuple ()
+  return %23 : $()
+}
+
 // CHECK-LABEL: sil [ossa] @f170_compare : $@convention(thin) <T where T : Comparable> (@in_guaranteed T, @in_guaranteed T) -> @out T {
 // CHECK: bb0(%0 : $*T, %1 : $*T, %2 : $*T):
 // CHECK:   [[WT:%.*]] = witness_method $T, #Comparable."<" : <Self where Self : Comparable> (Self.Type) -> (Self, Self) -> Builtin.Int1 : $@convention(witness_method: Comparable) <τ_0_0 where τ_0_0 : Comparable> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_0, @thick τ_0_0.Type) -> Builtin.Int1