In CopyForwarding, remove debug instructions on dead temporaries

Author: meg-gupta

lib/SILOptimizer/Transforms/CopyForwarding.cpp

@@ -612,8 +612,10 @@ class CopyForwarding {
 
 protected:
   bool propagateCopy(CopyAddrInst *CopyInst, bool hoistingDestroy);
-  CopyAddrInst *findCopyIntoDeadTemp(CopyAddrInst *destCopy);
-  bool forwardDeadTempCopy(CopyAddrInst *srcCopy, CopyAddrInst *destCopy);
+  CopyAddrInst *findCopyIntoDeadTemp(
+      CopyAddrInst *destCopy,
+      SmallVectorImpl<DebugValueAddrInst *> &debugInstsToDelete);
+  bool forwardDeadTempCopy(CopyAddrInst *destCopy);
   bool forwardPropagateCopy();
   bool backwardPropagateCopy();
   bool hoistDestroy(SILInstruction *DestroyPoint, SILLocation DestroyLoc);
@@ -681,12 +683,10 @@ propagateCopy(CopyAddrInst *CopyInst, bool hoistingDestroy) {
   // Handle copy-of-copy without analyzing uses.
   // Assumes that CurrentCopy->getSrc() is dead after CurrentCopy.
   assert(CurrentCopy->isTakeOfSrc() || hoistingDestroy);
-  if (auto *srcCopy = findCopyIntoDeadTemp(CurrentCopy)) {
-    if (forwardDeadTempCopy(srcCopy, CurrentCopy)) {
-      HasChanged = true;
-      ++NumDeadTemp;
-      return true;
-    }
+  if (forwardDeadTempCopy(CurrentCopy)) {
+    HasChanged = true;
+    ++NumDeadTemp;
+    return true;
   }
 
   if (forwardPropagateCopy()) {
@@ -737,7 +737,9 @@ propagateCopy(CopyAddrInst *CopyInst, bool hoistingDestroy) {
 /// Unlike the forward and backward propagation that finds all use points, this
 /// handles copies of address projections. By conservatively checking all
 /// intervening instructions, it avoids the need to analyze projection paths.
-CopyAddrInst *CopyForwarding::findCopyIntoDeadTemp(CopyAddrInst *destCopy) {
+CopyAddrInst *CopyForwarding::findCopyIntoDeadTemp(
+    CopyAddrInst *destCopy,
+    SmallVectorImpl<DebugValueAddrInst *> &debugInstsToDelete) {
   auto tmpVal = destCopy->getSrc();
   assert(tmpVal == CurrentDef);
   assert(isIdentifiedSourceValue(tmpVal));
@@ -750,8 +752,20 @@ CopyAddrInst *CopyForwarding::findCopyIntoDeadTemp(CopyAddrInst *destCopy) {
       if (srcCopy->getDest() == tmpVal)
         return srcCopy;
     }
+    // 'SrcUserInsts' consists of all users of the 'temp'
     if (SrcUserInsts.count(UserInst))
       return nullptr;
+
+    // Collect all debug_value_addr instructions between temp to dest copy and
+    // src to temp copy. On success, these debug_value_addr instructions should
+    // be deleted.
+    if (auto *debugUser = dyn_cast<DebugValueAddrInst>(UserInst)) {
+      // 'SrcDebugValueInsts' consists of all the debug users of 'temp'
+      if (SrcDebugValueInsts.count(debugUser))
+        debugInstsToDelete.push_back(debugUser);
+      continue;
+    }
+
     if (UserInst->mayWriteToMemory())
       return nullptr;
   }
@@ -780,12 +794,16 @@ CopyAddrInst *CopyForwarding::findCopyIntoDeadTemp(CopyAddrInst *destCopy) {
 /// - %temp is uninitialized following `srcCopy` and subsequent instruction
 ///   attempts to destroy this uninitialized value.
 bool CopyForwarding::
-forwardDeadTempCopy(CopyAddrInst *srcCopy, CopyAddrInst *destCopy) {
+forwardDeadTempCopy(CopyAddrInst *destCopy) {
+  SmallVector<DebugValueAddrInst*, 2> debugInstsToDelete;
+  auto *srcCopy = findCopyIntoDeadTemp(CurrentCopy, debugInstsToDelete);
+  if (!srcCopy)
+    return false;
+
   LLVM_DEBUG(llvm::dbgs() << "  Temp Copy:" << *srcCopy
                           << "         to " << *destCopy);
 
   assert(srcCopy->getDest() == destCopy->getSrc());
-  
   // This pattern can be trivially folded without affecting %temp destroys:
   // copy_addr [...] %src, [init] %temp
   // copy_addr [take] %temp, [...] %dest
@@ -798,6 +816,11 @@ forwardDeadTempCopy(CopyAddrInst *srcCopy, CopyAddrInst *destCopy) {
       .createDestroyAddr(srcCopy->getLoc(), srcCopy->getDest());
   }
 
+  // Delete all dead debug_value_addr instructions
+  for (auto *deadDebugUser : debugInstsToDelete) {
+    deadDebugUser->eraseFromParent();
+  }
+
   // Either `destCopy` is a take, or the caller is hoisting a destroy:
   // copy_addr %temp, %dest
   // ...

test/SILOptimizer/copyforward.sil

@@ -613,6 +613,7 @@ public struct S<T> {
 // CHECK-LABEL: deadtemp
 // CHECK: %[[G:.*]] = struct_element_addr %0 : $*S<T>, #S.g
 // CHECK-NOT: copy_addr
+// CHECK-NOT: debug_value_addr
 // CHECK: %[[F:.*]] = struct_element_addr %0 : $*S<T>, #S.f
 // CHECK: copy_addr %[[G]] to %[[F]] : $*T
 // CHECK: return
@@ -621,6 +622,7 @@ bb0(%0 : $*S<T>):
   %1 = struct_element_addr %0 : $*S<T>, #S.g
   %2 = alloc_stack $T
   copy_addr %1 to [initialization] %2 : $*T
+  debug_value_addr %2 : $*T
   %4 = struct_element_addr %0 : $*S<T>, #S.f
   copy_addr [take] %2 to %4 : $*T
   dealloc_stack %2 : $*T