Merge pull request swiftlang#30731 from eeckstein/fix-destroy-hoisting

DestroyHoisting: correctly detect if debug_value_addr instructions need to be deleted.

Author: eeckstein

lib/SILOptimizer/Transforms/DestroyHoisting.cpp

@@ -101,6 +101,9 @@ class DestroyHoisting {
 
   void moveDestroys(MemoryDataflow &dataFlow);
 
+  bool locationOverlaps(const MemoryLocations::Location *loc,
+                        const Bits &destroys);
+
   void moveDestroysInBlock(SILBasicBlock *block, Bits &activeDestroys,
                            SmallVectorImpl<SILInstruction *> &toRemove);
 
@@ -415,6 +418,19 @@ void DestroyHoisting::moveDestroys(MemoryDataflow &dataFlow) {
   }
 }
 
+bool DestroyHoisting::locationOverlaps(const MemoryLocations::Location *loc,
+                                       const Bits &destroys) {
+  // We cannot just check if 'loc->subLocations' has any common bits with
+  // 'destroys', because subLocations don't include the "self" bit if all sub
+  // locations cover the whole location.
+  for (int subIdx = loc->subLocations.find_first(); subIdx >= 0;
+       subIdx = loc->subLocations.find_next(subIdx)) {
+    if (destroys.anyCommon(locations.getLocation(subIdx)->selfAndParents))
+      return true;
+  }
+  return false;
+}
+
 void DestroyHoisting::moveDestroysInBlock(
                                 SILBasicBlock *block, Bits &activeDestroys,
                                 SmallVectorImpl<SILInstruction *> &toRemove) {
@@ -435,11 +451,9 @@ void DestroyHoisting::moveDestroysInBlock(
       // debug_value_addr does not count as real use of a location. If we are
       // moving a destroy_addr above a debug_value_addr, just delete that
       // debug_value_addr.
-      if (auto *dvaLoc = locations.getLocation(DVA->getOperand())) {
-        if (activeDestroys.anyCommon(dvaLoc->subLocations) ||
-            activeDestroys.anyCommon(dvaLoc->selfAndParents))
-          toRemove.push_back(DVA);
-      }
+      auto *dvaLoc = locations.getLocation(DVA->getOperand());
+      if (dvaLoc && locationOverlaps(dvaLoc, activeDestroys))
+        toRemove.push_back(DVA);
     } else if (I.mayHaveSideEffects()) {
       // Delete all destroy_addr and debug_value_addr which are scheduled for
       // removal.

test/SILOptimizer/destroy_hoisting.sil

@@ -28,6 +28,16 @@ struct Mixed {
   var i: Int
 }
 
+public struct S2 {
+  let s: S
+}
+
+
+public enum E {
+  case A
+  case B
+}
+
 sil @unknown : $@convention(thin) () -> ()
 sil @use_S : $@convention(thin) (@in_guaranteed S) -> ()
 
@@ -277,3 +287,24 @@ bb3:
   return %r : $()
 }
 
+// CHECK-LABEL: sil [ossa] @test_debug_value
+// CHECK:       bb0({{.*}}):
+// CHECK-NEXT:    struct_element_addr
+// CHECK-NEXT:    destroy_addr
+// CHECK-NEXT:    br bb1
+// CHECK:       } // end sil function 'test_debug_value'
+sil [ossa] @test_debug_value : $@convention(method) (@inout S2, @owned X, @owned S, @inout E) -> () {
+bb0(%0 : $*S2, %1 : @owned $X, %2 : @owned $S, %3 : $*E):
+  debug_value_addr %0 : $*S2, var, name "self", argno 1
+  br bb1
+
+bb1:
+  %60 = struct_element_addr %0 : $*S2, #S2.s
+  destroy_addr %60 : $*S
+  store %2 to [init] %60 : $*S
+  %71 = struct_element_addr %60 : $*S, #S.x
+  store %1 to [assign] %71 : $*X
+
+  %75 = tuple ()
+  return %75 : $()
+}