[LexicalDestroyFolding] Adopt iterative dataflow.

Adopt IterativeBackwardReachability.  Enables hoisting destroy_values
over loops.

rdar://93369506

Author: nate-chandler

lib/SILOptimizer/Utils/LexicalDestroyHoisting.cpp

@@ -43,14 +43,16 @@ struct Context final {
   /// value->getDefiningInstruction()
   SILInstruction *const definition;
 
+  SILBasicBlock *defBlock;
+
   SILFunction &function;
 
   InstructionDeleter &deleter;
 
   Context(SILValue const &value, SILFunction &function,
           InstructionDeleter &deleter)
       : value(value), definition(value->getDefiningInstruction()),
-        function(function), deleter(deleter) {
+        defBlock(value->getParentBlock()), function(function), deleter(deleter) {
     assert(value->isLexical());
     assert(value->getOwnershipKind() == OwnershipKind::Owned);
   }
@@ -63,7 +65,7 @@ struct Usage final {
   /// Instructions which are users of the simple (i.e. not reborrowed) value.
   SmallPtrSet<SILInstruction *, 16> users;
   // The instructions from which the hoisting starts, the destroy_values.
-  llvm::SmallSetVector<SILInstruction *, 4> ends;
+  llvm::SmallVector<SILInstruction *, 4> ends;
 
   Usage(){};
   Usage(Usage const &) = delete;
@@ -85,7 +87,7 @@ bool findUsage(Context const &context, Usage &usage) {
     // flow and determine whether any were reused.  They aren't uses over which
     // we can't hoist though.
     if (isa<DestroyValueInst>(use->getUser())) {
-      usage.ends.insert(use->getUser());
+      usage.ends.push_back(use->getUser());
     } else {
       usage.users.insert(use->getUser());
     }
@@ -95,80 +97,66 @@ bool findUsage(Context const &context, Usage &usage) {
 
 /// How destroy_value hoisting is obstructed.
 struct DeinitBarriers final {
-  /// Blocks up to "before the beginning" of which hoisting was able to proceed.
-  BasicBlockSetVector hoistingReachesBeginBlocks;
-
-  /// Blocks to "after the end" of which hoisting was able to proceed.
-  BasicBlockSet hoistingReachesEndBlocks;
-
   /// Instructions above which destroy_values cannot be hoisted.
-  SmallVector<SILInstruction *, 4> barriers;
+  SmallSetVector<SILInstruction *, 4> instructions;
 
   /// Blocks one of whose phis is a barrier and consequently out of which
   /// destroy_values cannot be hoisted.
-  SmallVector<SILBasicBlock *, 4> phiBarriers;
+  SmallSetVector<SILBasicBlock *, 4> phis;
+
+  SmallSetVector<SILBasicBlock *, 4> blocks;
 
-  DeinitBarriers(Context &context)
-      : hoistingReachesBeginBlocks(&context.function),
-        hoistingReachesEndBlocks(&context.function) {}
+  DeinitBarriers(Context &context) {}
   DeinitBarriers(DeinitBarriers const &) = delete;
   DeinitBarriers &operator=(DeinitBarriers const &) = delete;
 };
 
 /// Works backwards from the current location of destroy_values to the earliest
 /// place they can be hoisted to.
 ///
-/// Implements BackwardReachability::BlockReachability.
+/// Implements IterativeBackwardReachability::Effects
 class DataFlow final {
+  using Reachability = IterativeBackwardReachability<DataFlow>;
+  using Effect = Reachability::Effect;
   Context const &context;
   Usage const &uses;
-  DeinitBarriers &result;
+  DeinitBarriers &barriers;
+  Reachability::Result result;
+  Reachability reachability;
 
   enum class Classification { Barrier, Other };
 
-  BackwardReachability<DataFlow> reachability;
-
 public:
-  DataFlow(Context const &context, Usage const &uses, DeinitBarriers &result)
-      : context(context), uses(uses), result(result),
-        reachability(&context.function, *this) {
-    // Seed reachability with the scope ending uses from which the backwards
-    // data flow will begin.
-    for (auto *end : uses.ends) {
-      reachability.initLastUse(end);
-    }
+  DataFlow(Context const &context, Usage const &uses, DeinitBarriers &barriers)
+      : context(context), uses(uses), barriers(barriers), result(&context.function),
+        reachability(&context.function, context.defBlock, *this, result) {
   }
   DataFlow(DataFlow const &) = delete;
   DataFlow &operator=(DataFlow const &) = delete;
 
-  void run() { reachability.solveBackward(); }
+  void run();
 
 private:
-  friend class BackwardReachability<DataFlow>;
-
-  bool hasReachableBegin(SILBasicBlock *block) {
-    return result.hoistingReachesBeginBlocks.contains(block);
-  }
-
-  void markReachableBegin(SILBasicBlock *block) {
-    result.hoistingReachesBeginBlocks.insert(block);
-  }
-
-  void markReachableEnd(SILBasicBlock *block) {
-    result.hoistingReachesEndBlocks.insert(block);
-  }
+  friend Reachability;
 
   Classification classifyInstruction(SILInstruction *);
 
   bool classificationIsBarrier(Classification);
 
-  void visitedInstruction(SILInstruction *, Classification);
+  ArrayRef<SILInstruction *> gens();
 
-  bool checkReachableBarrier(SILInstruction *);
+  Optional<Effect> effectForInstruction(SILInstruction *);
 
-  bool checkReachablePhiBarrier(SILBasicBlock *);
+  Optional<Effect> effectForPhi(SILBasicBlock *);
 };
 
+void DataFlow::run() {
+  reachability.initialize();
+  reachability.solve();
+  reachability.findBarriers(barriers.instructions, barriers.phis,
+                            barriers.blocks);
+}
+
 DataFlow::Classification
 DataFlow::classifyInstruction(SILInstruction *instruction) {
   if (instruction == context.definition) {
@@ -193,26 +181,18 @@ bool DataFlow::classificationIsBarrier(Classification classification) {
   llvm_unreachable("exhaustive switch not exhaustive?!");
 }
 
-void DataFlow::visitedInstruction(SILInstruction *instruction,
-                                  Classification classification) {
-  assert(classifyInstruction(instruction) == classification);
-  switch (classification) {
-  case Classification::Barrier:
-    result.barriers.push_back(instruction);
-    return;
-  case Classification::Other:
-    return;
-  }
-  llvm_unreachable("exhaustive switch not exhaustive?!");
-}
+ArrayRef<SILInstruction *> DataFlow::gens() { return uses.ends; }
 
-bool DataFlow::checkReachableBarrier(SILInstruction *instruction) {
+Optional<DataFlow::Effect> DataFlow::effectForInstruction(SILInstruction *instruction) {
+  if (llvm::find(uses.ends, instruction) != uses.ends.end())
+    return {Effect::Gen};
   auto classification = classifyInstruction(instruction);
-  visitedInstruction(instruction, classification);
-  return classificationIsBarrier(classification);
+  return classificationIsBarrier(classification)
+             ? Optional<Effect>{Effect::Kill}
+             : llvm::None;
 }
 
-bool DataFlow::checkReachablePhiBarrier(SILBasicBlock *block) {
+Optional<DataFlow::Effect> DataFlow::effectForPhi(SILBasicBlock *block) {
   assert(llvm::all_of(block->getArguments(),
                       [&](auto argument) { return PhiValue(argument); }));
 
@@ -221,10 +201,7 @@ bool DataFlow::checkReachablePhiBarrier(SILBasicBlock *block) {
         return classificationIsBarrier(
             classifyInstruction(predecessor->getTerminator()));
       });
-  if (isBarrier) {
-    result.phiBarriers.push_back(block);
-  }
-  return isBarrier;
+  return isBarrier ? Optional<Effect>{Effect::Kill} : llvm::None;
 }
 
 /// Hoist the destroy_values of %value.
@@ -256,7 +233,7 @@ bool Rewriter::run() {
   //
   // A block is a phi barrier iff any of its predecessors' terminators get
   // classified as barriers.
-  for (auto *block : barriers.phiBarriers) {
+  for (auto *block : barriers.phis) {
     madeChange |= createDestroyValue(&block->front());
   }
 
@@ -271,13 +248,9 @@ bool Rewriter::run() {
   // have returned true for P, so none of its instructions would ever have been
   // classified (except for via checkReachablePhiBarrier, which doesn't record
   // terminator barriers).
-  for (auto instruction : barriers.barriers) {
+  for (auto instruction : barriers.instructions) {
     if (auto *terminator = dyn_cast<TermInst>(instruction)) {
       auto successors = terminator->getParentBlock()->getSuccessorBlocks();
-      // In order for the instruction to have been classified as a barrier,
-      // reachability would have had to reach the block containing it.
-      assert(barriers.hoistingReachesEndBlocks.contains(
-                terminator->getParentBlock()));
       for (auto *successor : successors) {
         madeChange |= createDestroyValue(&successor->front());
       }
@@ -301,12 +274,8 @@ bool Rewriter::run() {
   // P not having a reachable end--see BackwardReachability::meetOverSuccessors.
   //
   // control-flow-boundary(B) := beginning-reachable(B) && !end-reachable(P)
-  for (auto *block : barriers.hoistingReachesBeginBlocks) {
-    if (auto *predecessor = block->getSinglePredecessorBlock()) {
-      if (!barriers.hoistingReachesEndBlocks.contains(predecessor)) {
-        madeChange |= createDestroyValue(&block->front());
-      }
-    }
+  for (auto *block : barriers.blocks) {
+    madeChange |= createDestroyValue(&block->front());
   }
 
   if (madeChange) {
@@ -324,7 +293,7 @@ bool Rewriter::run() {
 
 bool Rewriter::createDestroyValue(SILInstruction *insertionPoint) {
   if (auto *ebi = dyn_cast<DestroyValueInst>(insertionPoint)) {
-    if (uses.ends.contains(insertionPoint)) {
+    if (llvm::find(uses.ends, insertionPoint) != uses.ends.end()) {
       reusedDestroyValueInsts.insert(insertionPoint);
       return false;
     }

test/SILOptimizer/lexical_destroy_hoisting.sil

@@ -237,11 +237,11 @@ exit(%thing : @owned $C):
 
 // Don't hoist over loop without uses.
 //
-// TODO: Eventually, we should hoist over such loops.
 // CHECK-LABEL: sil [ossa] @hoist_over_loop_1 : {{.*}} {
 // CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
 // CHECK:         [[CALLEE_GUARANTEED:%[^,]+]] = function_ref @callee_guaranteed
 // CHECK:         apply [[CALLEE_GUARANTEED]]([[INSTANCE]])
+// CHECK:         destroy_value [[INSTANCE]]
 // CHECK:         br [[LOOP_HEADER:bb[0-9]+]]
 // CHECK:       [[LOOP_HEADER]]:
 // CHECK:         br [[LOOP_BODY:bb[0-9]+]]
@@ -252,7 +252,6 @@ exit(%thing : @owned $C):
 // CHECK:       [[LOOP_BACKEDGE]]:
 // CHECK:         br [[LOOP_HEADER]]
 // CHECK:       [[EXIT]]:
-// CHECK:         destroy_value [[INSTANCE]]
 // CHECK-LABEL: } // end sil function 'hoist_over_loop_1'
 sil [ossa] @hoist_over_loop_1 : $@convention(thin) (@owned C) -> () {
 entry(%instance: @owned $C):