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

DCE may enqueue a value for lifetime completion and later on erase the
instruction that defines that value.  When erasing an instruction, erase
each of its results from the collection of values to complete.

rdar://141560546

Author: nate-chandler

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;
   }
 
@@ -733,6 +734,12 @@ bool DCE::removeDead() {
           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;
@@ -787,6 +794,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))
@@ -799,7 +812,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);
   }
 
@@ -856,9 +871,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
@@ -1412,3 +1412,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
+}