Merge pull request swiftlang#28381 from eeckstein/fix-destoy-hoisting

Author: swift-ci

include/swift/SIL/MemoryLifetime.h

@@ -287,6 +287,21 @@ class MemoryLocations {
 /// for memory locations. Consider renaming it.
 class MemoryDataflow {
 
+  /// What kind of terminators can be reached from a block.
+  enum class ExitReachability : uint8_t {
+    /// Worst case: the block is part of a cycle which neither reaches a
+    /// function-exit nor an unreachable-instruction.
+    InInfiniteLoop,
+
+    /// An unreachable-instruction can be reached from the block, but not a
+    /// function-exit (like "return" or "throw").
+    ReachesUnreachable,
+
+    /// A function-exit can be reached from the block.
+    /// This is the case for most basic blocks.
+    ReachesExit
+  };
+
 public:
   using Bits = MemoryLocations::Bits;
 
@@ -313,11 +328,10 @@ class MemoryDataflow {
     /// This flag is only computed if entryReachabilityAnalysis is called.
     bool reachableFromEntry = false;
 
-    /// True, if any function-exit block can be reached from this block, i.e. is
-    /// not a block which eventually ends in an unreachable instruction.
+    /// What kind of terminators can be reached from this block.
     ///
-    /// This flag is only computed if exitReachableAnalysis is called.
-    bool exitReachable = false;
+    /// This is only computed if exitReachableAnalysis is called.
+    ExitReachability exitReachability = ExitReachability::InInfiniteLoop;
 
     BlockState(SILBasicBlock *block = nullptr) : block(block) { }
 
@@ -336,6 +350,14 @@ class MemoryDataflow {
         killSet |= loc->subLocations;
       }
     }
+
+    bool exitReachable() const {
+      return exitReachability == ExitReachability::ReachesExit;
+    }
+
+    bool isInInfiniteLoop() const {
+      return exitReachability == ExitReachability::InInfiniteLoop;
+    }
   };
 
 private:

lib/SIL/MemoryLifetime.cpp

@@ -435,16 +435,21 @@ void MemoryDataflow::exitReachableAnalysis() {
   llvm::SmallVector<BlockState *, 16> workList;
   for (BlockState &state : blockStates) {
     if (state.block->getTerminator()->isFunctionExiting()) {
-      state.exitReachable = true;
+      state.exitReachability = ExitReachability::ReachesExit;
+      workList.push_back(&state);
+    } else if (isa<UnreachableInst>(state.block->getTerminator())) {
+      state.exitReachability = ExitReachability::ReachesUnreachable;
       workList.push_back(&state);
     }
   }
   while (!workList.empty()) {
     BlockState *state = workList.pop_back_val();
     for (SILBasicBlock *pred : state->block->getPredecessorBlocks()) {
       BlockState *predState = block2State[pred];
-      if (!predState->exitReachable) {
-        predState->exitReachable = true;
+      if (predState->exitReachability < state->exitReachability) {
+        // As there are 3 states, each block can be put into the workList 2
+        // times maximum.
+        predState->exitReachability = state->exitReachability;
         workList.push_back(predState);
       }
     }
@@ -806,7 +811,7 @@ void MemoryLifetimeVerifier::checkFunction(MemoryDataflow &dataFlow) {
   const Bits &nonTrivialLocations = locations.getNonTrivialLocations();
   Bits bits(locations.getNumLocations());
   for (BlockState &st : dataFlow) {
-    if (!st.reachableFromEntry || !st.exitReachable)
+    if (!st.reachableFromEntry || !st.exitReachable())
       continue;
 
     // Check all instructions in the block.

lib/SILOptimizer/Transforms/DestroyHoisting.cpp

@@ -208,9 +208,16 @@ void DestroyHoisting::expandStores(MemoryDataflow &dataFlow) {
 // Initialize the dataflow for moving destroys up the control flow.
 void DestroyHoisting::initDataflow(MemoryDataflow &dataFlow) {
   for (BlockState &st : dataFlow) {
-    st.entrySet.set();
     st.genSet.reset();
     st.killSet.reset();
+    if (st.isInInfiniteLoop()) {
+      // Ignore blocks which are in an infinite loop and prevent any destroy
+      // hoisting across such block borders.
+      st.entrySet.reset();
+      st.exitSet.reset();
+      continue;
+    }
+    st.entrySet.set();
     if (isa<UnreachableInst>(st.block->getTerminator())) {
       if (canIgnoreUnreachableBlock(st.block, dataFlow)) {
         st.exitSet.set();
@@ -284,7 +291,7 @@ bool DestroyHoisting::canIgnoreUnreachableBlock(SILBasicBlock *block,
   SILBasicBlock *singlePred = block->getSinglePredecessorBlock();
   if (!singlePred)
     return false;
-  if (!dataFlow.getState(singlePred)->exitReachable)
+  if (!dataFlow.getState(singlePred)->exitReachable())
     return false;
 
   // Check if none of the locations are touched in the unreachable-block.
@@ -374,6 +381,10 @@ void DestroyHoisting::moveDestroys(MemoryDataflow &dataFlow) {
     if (isa<UnreachableInst>(block->getTerminator()) && state.exitSet.any())
       continue;
 
+    // Ignore blocks which are in an infinite loop.
+    if (state.isInInfiniteLoop())
+      continue;
+
     // Do the inner-block processing.
     activeDestroys = state.exitSet;
     moveDestroysInBlock(block, activeDestroys, toRemove);

test/SILOptimizer/destroy_hoisting.sil

@@ -248,3 +248,33 @@ bb1:
   %r = tuple ()
   return %r : $()
 }
+
+// CHECK-LABEL: sil [ossa] @test_simple_infinite_loop
+// CHECK-NOT:     destroy_addr
+// CHECK:       } // end sil function 'test_simple_infinite_loop'
+sil [ossa] @test_simple_infinite_loop : $@convention(thin) (@in_guaranteed S) -> () {
+bb0(%0 : $*S):
+  br bb1
+bb1:
+  br bb1
+}
+
+// CHECK-LABEL: sil [ossa] @test_infinite_loop
+// CHECK-NOT:     destroy_addr
+// CHECK:       bb3:
+// CHECK-NEXT:    destroy_addr %1
+// CHECK-NOT:     destroy_addr
+// CHECK:       } // end sil function 'test_infinite_loop'
+sil [ossa] @test_infinite_loop : $@convention(thin) (@in_guaranteed S, @in S) -> () {
+bb0(%0 : $*S, %1 : $*S):
+  cond_br undef, bb1, bb2
+bb1:
+  br bb1
+bb2:
+  br bb3
+bb3:
+  destroy_addr %1 : $*S
+  %r = tuple ()
+  return %r : $()
+}
+