[DCE] Don't complete lifetimes of erased values.

lib/SILOptimizer/Transforms/DeadCodeElimination.cpp

@@ -12,19 +12,20 @@
 
 #define DEBUG_TYPE "sil-dce"
 #include "swift/Basic/Assertions.h"
+#include "swift/Basic/BlotSetVector.h"
 #include "swift/SIL/BasicBlockBits.h"
 #include "swift/SIL/DebugUtils.h"
 #include "swift/SIL/MemAccessUtils.h"
 #include "swift/SIL/NodeBits.h"
+#include "swift/SIL/OSSALifetimeCompletion.h"
 #include "swift/SIL/OwnershipUtils.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBasicBlock.h"
 #include "swift/SIL/SILBuilder.h"
 #include "swift/SIL/SILFunction.h"
 #include "swift/SIL/SILUndef.h"
-#include "swift/SIL/OSSALifetimeCompletion.h"
-#include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
 #include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h"
+#include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 #include "swift/SILOptimizer/Utils/BasicBlockOptUtils.h"
@@ -94,7 +95,7 @@ static bool seemsUseful(SILInstruction *I) {
   if (isa<DebugValueInst>(I))
     return isa<SILFunctionArgument>(I->getOperand(0))
       || isa<SILUndef>(I->getOperand(0));
-  
+
 
   // Don't delete allocation instructions in DCE.
   if (isa<AllocRefInst>(I) || isa<AllocRefDynamicInst>(I)) {
@@ -131,7 +132,7 @@ class DCE {
   DominanceInfo *DT;
   DeadEndBlocks *deadEndBlocks;
   llvm::DenseMap<SILBasicBlock *, ControllingInfo> ControllingInfoMap;
-  llvm::SmallVector<SILValue> valuesToComplete;
+  SmallBlotSetVector<SILValue, 8> valuesToComplete;
 
   // Maps instructions which produce a failing condition (like overflow
   // builtins) to the actual cond_fail instructions which handle the failure.
@@ -212,7 +213,7 @@ class DCE {
     markLive();
     return removeDead();
   }
-  
+
   bool mustInvalidateCalls() const { return CallsChanged; }
   bool mustInvalidateBranches() const { return BranchesChanged; }
 };
@@ -637,7 +638,7 @@ void DCE::endLifetimeOfLiveValue(Operand *op, SILInstruction *insertPt) {
   // If DCE is going to delete the block in which we have to insert a
   // compensating lifetime end, let complete lifetimes utility handle it.
   if (!LiveBlocks.contains(insertPt->getParent())) {
-    valuesToComplete.push_back(lookThroughBorrowedFromDef(value));
+    valuesToComplete.insert(lookThroughBorrowedFromDef(value));
     return;
   }
 
@@ -731,9 +732,23 @@ bool DCE::removeDead() {
           endLifetimeOfLiveValue(predOp, insertPt);
         }
       }
-      erasePhiArgument(&BB, i, /*cleanupDeadPhiOps=*/true,
-                       InstModCallbacks().onCreateNewInst(
-                           [&](auto *inst) { markInstructionLive(inst); }));
+      erasePhiArgument(
+          &BB, i, /*cleanupDeadPhiOps=*/true,
+          InstModCallbacks()
+              .onCreateNewInst([&](auto *inst) { markInstructionLive(inst); })
+              .onDelete([&](auto *inst) {
+                for (auto result : inst->getResults()) {
+                  result = lookThroughBorrowedFromDef(result);
+                  if (valuesToComplete.count(result)) {
+                    valuesToComplete.erase(result);
+                  }
+                }
+                inst->replaceAllUsesOfAllResultsWithUndef();
+                if (isa<ApplyInst>(inst))
+                  CallsChanged = true;
+                ++NumDeletedInsts;
+                inst->eraseFromParent();
+              }));
       Changed = true;
       BranchesChanged = true;
     }
@@ -747,7 +762,8 @@ bool DCE::removeDead() {
       // We want to replace dead terminators with unconditional branches to
       // the nearest post-dominator that has useful instructions.
       if (auto *termInst = dyn_cast<TermInst>(Inst)) {
-        SILBasicBlock *postDom = nearestUsefulPostDominator(Inst->getParent());
+        SILBasicBlock *postDom =
+            nearestUsefulPostDominator(termInst->getParent());
         if (!postDom)
           continue;
 
@@ -757,12 +773,13 @@ bool DCE::removeDead() {
           }
         }
         LLVM_DEBUG(llvm::dbgs() << "Replacing branch: ");
-        LLVM_DEBUG(Inst->dump());
+        LLVM_DEBUG(termInst->dump());
         LLVM_DEBUG(llvm::dbgs()
                    << "with jump to: BB" << postDom->getDebugID() << "\n");
 
-        replaceBranchWithJump(Inst, postDom);
-        Inst->eraseFromParent();
+        replaceBranchWithJump(termInst, postDom);
+        ++NumDeletedInsts;
+        termInst->eraseFromParent();
         BranchesChanged = true;
         Changed = true;
         continue;
@@ -780,6 +797,12 @@ bool DCE::removeDead() {
           }
         }
       }
+      for (auto result : Inst->getResults()) {
+        result = lookThroughBorrowedFromDef(result);
+        if (valuesToComplete.count(result)) {
+          valuesToComplete.erase(result);
+        }
+      }
       Inst->replaceAllUsesOfAllResultsWithUndef();
 
       if (isa<ApplyInst>(Inst))
@@ -792,7 +815,9 @@ bool DCE::removeDead() {
 
   OSSALifetimeCompletion completion(F, DT, *deadEndBlocks);
   for (auto value : valuesToComplete) {
-    completion.completeOSSALifetime(value,
+    if (!value.has_value())
+      continue;
+    completion.completeOSSALifetime(*value,
                                     OSSALifetimeCompletion::Boundary::Liveness);
   }
 
@@ -849,9 +874,9 @@ void DCE::computeLevelNumbers(PostDomTreeNode *root) {
     auto entry = workList.pop_back_val();
     PostDomTreeNode *node = entry.first;
     unsigned level = entry.second;
-    
+
     insertControllingInfo(node->getBlock(), level);
-    
+
     for (PostDomTreeNode *child : *node) {
       workList.push_back({child, level + 1});
     }

test/SILOptimizer/dead_code_elimination_nontrivial_ossa.sil

@@ -514,7 +514,7 @@ bb1(%newborrow : @guaranteed $Klass, %borrow2 : @guaranteed $Klass):
 // Here %newborrowo and %newborrowi are both dead phis.
 // First end_borrow for the incoming value of %newborrowi is added
 // It is non straight forward to find the insert pt for the end_borrow of the incoming value of %newborrowo
-// This may not be important once CanonicalizeOSSALifetime supports rewrite of multi-block borrows. 
+// This may not be important once CanonicalizeOSSALifetime supports rewrite of multi-block borrows.
 sil [ossa] @dce_nestedborrowlifetime3 : $@convention(thin) (@guaranteed Klass) -> () {
 bb0(%0 : @guaranteed $Klass):
   %borrowo = begin_borrow %0 : $Klass
@@ -752,7 +752,7 @@ exit:
 sil hidden [ossa] @add_end_borrow_after_destroy_value : $@convention(thin) (Builtin.Int1, @owned Klass, @owned Klass) -> () {
 bb0(%condition : $Builtin.Int1, %instance_1 : @owned $Klass, %instance_2 : @owned $Klass):
   %lifetime_1 = begin_borrow %instance_1 : $Klass
-  
+
   %copy_1 = copy_value %lifetime_1 : $Klass
   %stack_addr = alloc_stack $Klass
   store %copy_1 to [init] %stack_addr : $*Klass
@@ -1124,7 +1124,7 @@ sil @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
 sil [ossa] @test_forwarded_phi1 : $@convention(thin) (@owned Wrapper1) -> () {
 bb0(%0 : @owned $Wrapper1):
   %outer = begin_borrow %0 : $Wrapper1
-  br bb1 
+  br bb1
 
 bb1:
   %ex1 = struct_extract %outer : $Wrapper1, #Wrapper1.val1
@@ -1144,13 +1144,13 @@ bb3(%phi3 : @guaranteed $Klass, %phi4 : @guaranteed $Wrapper1):
 }
 
 // CHECK-LABEL: sil [ossa] @test_forwarded_phi2 :
-// CHECK: br bb3 
+// CHECK: br bb3
 // CHECK: end_borrow
 // CHECK-LABEL: } // end sil function 'test_forwarded_phi2'
 sil [ossa] @test_forwarded_phi2 : $@convention(thin) (@owned Wrapper1) -> () {
 bb0(%0 : @owned $Wrapper1):
   %outer = begin_borrow %0 : $Wrapper1
-  br bb1 
+  br bb1
 
 bb1:
   br bb2(%outer : $Wrapper1)
@@ -1202,7 +1202,7 @@ bb3(%phi2 : @guaranteed $Klass, %phi3 : @guaranteed $Wrapper1):
 sil [ossa] @test_loadborrow_dep : $@convention(thin) (@in Wrapper1) -> () {
 bb0(%0 : $*Wrapper1):
   %outer = load_borrow %0 : $*Wrapper1
-  br bb1 
+  br bb1
 
 bb1:
   %ex1 = struct_extract %outer : $Wrapper1, #Wrapper1.val1
@@ -1411,3 +1411,34 @@ bb5(%16 : @reborrow $FakeOptional<Klass>, %17 : @reborrow $FakeOptional<Klass>):
   return %23
 }
 
+struct String {
+  var guts: Builtin.AnyObject
+}
+
+sil [_semantics "string.makeUTF8"] [ossa] @makeString : $@convention(thin) (Builtin.RawPointer, Builtin.Word) -> @owned String
+
+// CHECK-LABEL: sil [ossa] @uncompletedDeadStrings
+// CHECK-NOT:     apply
+// CHECK-LABEL: } // end sil function 'uncompletedDeadStrings'
+sil [ossa] @uncompletedDeadStrings : $@convention(thin) () -> () {
+  %first_ptr = string_literal utf8 "first"
+  %first_len = integer_literal $Builtin.Word, 5
+  %makeString = function_ref @makeString : $@convention(thin) (Builtin.RawPointer, Builtin.Word) -> @owned String
+  %first = apply %makeString(%first_ptr, %first_len) : $@convention(thin) (Builtin.RawPointer, Builtin.Word) -> @owned String
+  cond_br undef, nope, yep
+
+nope:
+  destroy_value %first
+  %second_ptr = string_literal utf8 "second"
+  %second_len = integer_literal $Builtin.Word, 6
+  %second = apply %makeString(%second_ptr, %second_len) : $@convention(thin) (Builtin.RawPointer, Builtin.Word) -> @owned String
+  br this(%second)
+
+yep:
+  br this(%first)
+
+this(%string : @owned $String):
+  destroy_value %string
+  %retval = tuple ()
+  return %retval
+}