Merge pull request #41519 from nate-chandler/lexical_lifetimes/lexical_destroy_addr_hoisting/avoid-second-dataflow

[SSADestroyHoisting] Avoid second per-var dataflow.

Author: nate-chandler

lib/SILOptimizer/Transforms/SSADestroyHoisting.cpp

@@ -90,6 +90,7 @@
 #define DEBUG_TYPE "ssa-destroy-hoisting"
 
 #include "swift/Basic/GraphNodeWorklist.h"
+#include "swift/Basic/SmallPtrSetVector.h"
 #include "swift/SIL/BasicBlockDatastructures.h"
 #include "swift/SIL/MemAccessUtils.h"
 #include "swift/SIL/SILBasicBlock.h"
@@ -192,11 +193,11 @@ class DeinitBarriers {
 public:
   // Data flow state: blocks whose beginning is backward reachable from a
   // destroy without first reaching a barrier or storage use.
-  BasicBlockSetVector destroyReachesBeginBlocks;
+  SmallPtrSetVector<SILBasicBlock *, 4> destroyReachesBeginBlocks;
 
   // Data flow state: blocks whose end is backward reachable from a destroy
   // without first reaching a barrier or storage use.
-  BasicBlockSet destroyReachesEndBlocks;
+  SmallPtrSet<SILBasicBlock *, 4> destroyReachesEndBlocks;
 
   // Deinit barriers or storage uses within a block, reachable from a destroy.
   SmallVector<SILInstruction *, 4> barriers;
@@ -215,8 +216,7 @@ class DeinitBarriers {
   explicit DeinitBarriers(bool ignoreDeinitBarriers,
                           const KnownStorageUses &knownUses,
                           SILFunction *function)
-      : destroyReachesBeginBlocks(function), destroyReachesEndBlocks(function),
-        ignoreDeinitBarriers(ignoreDeinitBarriers), knownUses(knownUses) {
+      : ignoreDeinitBarriers(ignoreDeinitBarriers), knownUses(knownUses) {
     auto rootValue = knownUses.getStorage().getRoot();
     assert(rootValue && "HoistDestroys requires a single storage root");
     // null for function args
@@ -225,6 +225,12 @@ class DeinitBarriers {
 
   void compute() {
     FindBarrierAccessScopes(*this).solveBackward();
+    if (barrierAccessScopes.size() == 0)
+      return;
+    destroyReachesBeginBlocks.clear();
+    destroyReachesEndBlocks.clear();
+    barriers.clear();
+    deadUsers.clear();
     DestroyReachability(*this).solveBackward();
   }
 
@@ -289,7 +295,6 @@ class DeinitBarriers {
   // open access scopes in the block's predecessors.
   class FindBarrierAccessScopes {
     DeinitBarriers &result;
-    BasicBlockSetVector destroyReachesBeginBlocks;
     llvm::DenseMap<SILBasicBlock *, llvm::SmallPtrSet<BeginAccessInst *, 2>>
         liveInAccessScopes;
     llvm::SmallPtrSet<BeginAccessInst *, 2> runningLiveAccessScopes;
@@ -298,9 +303,7 @@ class DeinitBarriers {
 
   public:
     FindBarrierAccessScopes(DeinitBarriers &result)
-        : result(result),
-          destroyReachesBeginBlocks(result.knownUses.getFunction()),
-          reachability(result.knownUses.getFunction(), *this) {
+        : result(result), reachability(result.knownUses.getFunction(), *this) {
       // Seed backward reachability with destroy points.
       for (SILInstruction *destroy : result.knownUses.originalDestroys) {
         reachability.initLastUse(destroy);
@@ -314,17 +317,18 @@ class DeinitBarriers {
     }
 
     bool hasReachableBegin(SILBasicBlock *block) {
-      return destroyReachesBeginBlocks.contains(block);
+      return result.destroyReachesBeginBlocks.contains(block);
     }
 
     void markReachableBegin(SILBasicBlock *block) {
-      destroyReachesBeginBlocks.insert(block);
+      result.destroyReachesBeginBlocks.insert(block);
       if (!runningLiveAccessScopes.empty()) {
         liveInAccessScopes[block] = runningLiveAccessScopes;
       }
     }
 
     void markReachableEnd(SILBasicBlock *block) {
+      result.destroyReachesEndBlocks.insert(block);
       runningLiveAccessScopes.clear();
       for (auto *predecessor : block->getPredecessorBlocks()) {
         auto iterator = liveInAccessScopes.find(predecessor);
@@ -347,7 +351,9 @@ class DeinitBarriers {
       } else if (auto *bai = dyn_cast<BeginAccessInst>(inst)) {
         runningLiveAccessScopes.erase(bai);
       }
-      bool isBarrier = result.isBarrier(inst);
+      auto classification = result.classifyInstruction(inst);
+      result.visitedInstruction(inst, classification);
+      auto isBarrier = result.classificationIsBarrier(classification);
       if (isBarrier) {
         markLiveAccessScopesAsBarriers();
       }
@@ -361,7 +367,7 @@ class DeinitBarriers {
     bool checkReachablePhiBarrier(SILBasicBlock *block) {
       bool isBarrier =
           llvm::any_of(block->getPredecessorBlocks(), [&](auto *predecessor) {
-            return result.isBarrier(block->getTerminator());
+            return result.isBarrier(predecessor->getTerminator());
           });
       if (isBarrier) {
         // If there's a barrier preventing us from hoisting out of this block,
@@ -484,8 +490,7 @@ bool DeinitBarriers::DestroyReachability::checkReachablePhiBarrier(
   assert(llvm::all_of(block->getArguments(),
                       [&](auto argument) { return PhiValue(argument); }));
   return llvm::any_of(block->getPredecessorBlocks(), [&](auto *predecessor) {
-    return result.classificationIsBarrier(
-        result.classifyInstruction(predecessor->getTerminator()));
+    return result.isBarrier(predecessor->getTerminator());
   });
 }
 

test/SILOptimizer/hoist_destroy_addr.sil

@@ -43,12 +43,22 @@ struct TrivialStruct {
   var e: E
 }
 
+enum Change {
+case insert(offset: Int, element: X)
+case remove(offset: Int, element: X)
+}
+
+struct Int {
+    @_hasStorage var _value : Builtin.Int64
+}
+
 sil @unknown : $@convention(thin) () -> ()
 sil @use_S : $@convention(thin) (@in_guaranteed S) -> ()
 
 sil @f_out : $@convention(thin) <T> () -> @out T
 sil @f_bool : $@convention(thin) () -> Builtin.Int1
 sil [ossa] @take_trivial_struct : $@convention(thin) (TrivialStruct) -> ()
+sil [ossa] @get_change_out : $@convention(thin) () -> @out Change
 
 // CHECK-LABEL: sil [ossa] @test_simple
 // CHECK:      bb0(%0 : $*S):
@@ -270,6 +280,53 @@ entry(%addr : $*X):
   return %tuple : $()
 }
 
+// Hoist a destroy_addr over a phi.
+//
+// CHECK-LABEL: sil [ossa] @hoist_over_undef_phi : {{.*}} {
+// CHECK:         [[STACK1:%[^,]+]] = alloc_stack
+// CHECK-NEXT:    apply undef
+// CHECK-NEXT:    destroy_addr [[STACK1]]
+// CHECK-LABEL: } // end sil function 'hoist_over_undef_phi'
+sil [ossa] @hoist_over_undef_phi : $@convention(thin) () -> () {
+entry:
+  br latch
+
+latch:
+  cond_br undef, top, exit
+
+top:
+  %stack1 = alloc_stack $Change
+  apply undef(%stack1) : $@convention(thin) () -> @out Change 
+  %access = begin_access [static] [modify] %stack1 : $*Change
+  br top2(undef : $())
+
+top2(%66 : $()):
+  end_access %access : $*Change
+  destroy_addr %stack1 : $*Change
+  dealloc_stack %stack1 : $*Change
+  %stack2 = alloc_stack $Change
+  apply undef(%stack2) : $@convention(thin) () -> @out Change 
+  switch_enum_addr %stack2 : $*Change, case #Change.insert!enumelt: left, case #Change.remove!enumelt: right
+
+left:
+  br bottom
+
+right:
+  br bottom
+
+bottom:
+  destroy_addr %stack2 : $*Change
+  dealloc_stack %stack2 : $*Change
+  br backedge
+
+backedge:
+  br latch
+
+exit:
+  %321 = tuple ()
+  return %321 : $()
+}
+
 // Fold destroy_addr and a load [copy] into a load [take] even when that
 // load [take] is guarded by an access scope.
 //