SIL: handle mark_dependence in the MemoryLifetimeVerifier

The check is not perfect, because it only checks that the base operand is alive _at_ the mark_dependence.
Ideally it should check that the base operand is alive during the whole lifetime of the value operand.

Author: eeckstein

lib/SIL/Utils/MemoryLocations.cpp

@@ -89,6 +89,9 @@ static SILValue getBaseValue(SILValue addr) {
       case ValueKind::BeginAccessInst:
         addr = cast<BeginAccessInst>(addr)->getOperand();
         break;
+      case ValueKind::MarkDependenceInst:
+        addr = cast<MarkDependenceInst>(addr)->getValue();
+        break;
       default:
         return addr;
     }
@@ -334,6 +337,14 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx
         if (!cast<LoadBorrowInst>(user)->getUsersOfType<BranchInst>().empty())
           return false;
         break;
+      case SILInstructionKind::MarkDependenceInst: {
+        auto *mdi = cast<MarkDependenceInst>(user);
+        if (use == &mdi->getAllOperands()[MarkDependenceInst::Value]) {
+          if (!analyzeLocationUsesRecursively(mdi, locIdx, collectedVals, subLocationMap))
+            return false;
+        }
+        break;
+      }
       case SILInstructionKind::DebugValueInst:
         if (cast<DebugValueInst>(user)->hasAddrVal())
           break;

lib/SIL/Verifier/MemoryLifetimeVerifier.cpp

@@ -306,6 +306,10 @@ void MemoryLifetimeVerifier::requireBitsSetForArgument(const Bits &bits, Operand
 }
 
 bool MemoryLifetimeVerifier::applyMayRead(Operand *argOp, SILValue addr) {
+  // Conservatively assume that a partial_apply does _not_ read an argument.
+  if (isa<PartialApplyInst>(argOp->getUser()))
+    return false;
+  
   FullApplySite as(argOp->getUser());
   CalleeList callees;
   if (calleeCache) {
@@ -881,6 +885,18 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) {
         locations.clearBits(bits, opVal);
         break;
       }
+      case SILInstructionKind::MarkDependenceInst: {
+        auto *mdi = cast<MarkDependenceInst>(&I);
+        if (mdi->getBase()->getType().isAddress() &&
+            // In case the mark_dependence is used for a closure it might be that the base
+            // is "self" in an initializer and "self" is not fully initialized, yet.
+            !mdi->getType().isFunction()) {
+          requireBitsSet(bits, mdi->getBase(), &I);
+        }
+        // TODO: check that the base operand is alive during the whole lifetime
+        // of the value operand.
+        break;
+      }
       default:
         break;
     }

test/SIL/memory_lifetime.sil

@@ -510,6 +510,18 @@ bb0(%0 :  $*T):
   return %res : $()
 }
 
+sil [ossa] @test_non_escaping_closure_with_mark_dependence : $@convention(thin) () -> () {
+bb0:
+  %0 = alloc_stack $T
+  %func = function_ref @closure : $@convention(thin) (@in_guaranteed T) -> ()
+  %pa = partial_apply [callee_guaranteed] [on_stack] %func(%0) : $@convention(thin) (@in_guaranteed T) -> ()
+  %pa1 = mark_dependence %pa on %0
+  destroy_value %pa1 : $@noescape @callee_guaranteed () -> () 
+  dealloc_stack %0
+  %res = tuple ()
+  return %res : $()
+}
+
 sil [ossa] @test_store_borrow : $@convention(thin) (@guaranteed T) -> () {
 bb0(%0 :  @guaranteed $T):
   %s = alloc_stack $T
@@ -806,3 +818,35 @@ bb3:
   return %r : $()
 }
 
+sil [ossa] @mark_dependence : $@convention(method) (@owned T, @owned Inner) -> @owned Inner {
+bb0(%0 : @owned $T, %1 : @owned $Inner):
+  %2 = alloc_stack $T
+  store %0 to [init] %2
+  %4 = alloc_stack $Inner
+  store %1 to [init] %4
+  %6 = mark_dependence %4 on %2
+  %7 = begin_access [read] [dynamic] %6
+  %8 = load [take] %7
+  end_access %7
+  dealloc_stack %4
+  destroy_addr %2
+  dealloc_stack %2
+  return %8
+}
+
+sil [ossa] @mark_dependence_uninit_value : $@convention(thin) (@owned T, @owned Inner) -> @owned Inner {
+bb0(%0 : @owned $T, %1 : @owned $Inner):
+  %2 = alloc_stack $T
+  store %0 to [init] %2
+  %4 = alloc_stack $Inner
+  store %1 to [init] %4
+  %6 = mark_dependence %4 on %2
+  %7 = begin_access [read] [dynamic] %6
+  %8 = load [take] %7
+  end_access %7
+  dealloc_stack %4
+  destroy_addr %2
+  dealloc_stack %2
+  return %8
+}
+

test/SIL/memory_lifetime_failures.sil

@@ -792,3 +792,20 @@ bb0(%0: $*Inner):
   return %r : $()
 }
 
+// CHECK: SIL memory lifetime failure in @mark_dependence_uninit_base: memory is not initialized, but should be
+sil [ossa] @mark_dependence_uninit_base : $@convention(thin) (@owned T, @owned Inner) -> @owned Inner {
+bb0(%0 : @owned $T, %1 : @owned $Inner):
+  %2 = alloc_stack $T
+  store %0 to [init] %2
+  %4 = alloc_stack $Inner
+  store %1 to [init] %4
+  destroy_addr %2
+  %6 = mark_dependence %4 on %2
+  %7 = begin_access [read] [dynamic] %6
+  %8 = load [take] %7
+  end_access %7
+  dealloc_stack %4
+  dealloc_stack %2
+  return %8
+}
+

test/SILOptimizer/ossa_lifetime_completion.sil

@@ -216,18 +216,20 @@ bb3:
 }
 
 // Ensure no assert fires while handling lifetime end of partial_apply
-sil [ossa] @testPartialApplyStack2 : $@convention(thin) (@guaranteed C) -> () {
-bb0(%0 : @guaranteed $C):
+sil [ossa] @testPartialApplyStack2 : $@convention(thin) (@owned C) -> () {
+bb0(%0 : @owned $C):
   specify_test "ossa_lifetime_completion @instruction[1] availability"
   %2 = alloc_stack $C
   %3 = copy_value %0 : $C
+  store %0 to [init] %2
   %4 = begin_borrow %3 : $C
   %5 = function_ref @foo : $@convention(thin) (@guaranteed C) -> ()
   %6 = partial_apply [callee_guaranteed] [on_stack] %5(%4) : $@convention(thin) (@guaranteed C) -> ()
   %7 = mark_dependence %6 : $@noescape @callee_guaranteed () -> () on %2 : $*C
   destroy_value %7 : $@noescape @callee_guaranteed () -> ()
   end_borrow %4 : $C
   destroy_value %3 : $C
+  destroy_addr %2 : $*C
   dealloc_stack %2 : $*C
   %12 = tuple ()
   return %12 : $()

test/SILOptimizer/redundant_load_elim_nontrivial_ossa.sil

@@ -812,7 +812,7 @@ bb0(%0 : $*Klass, %1 : $*Klass, %2 : @owned $Klass):
 // CHECK: load
 // CHECK-NOT: load
 // CHECK-LABEL: } // end sil function 'redundant_load_mark_dependence'
-sil [ossa] @redundant_load_mark_dependence : $@convention(thin) (@inout Klass, @guaranteed Builtin.NativeObject) -> @owned (Klass, Klass) {
+sil [ossa] @redundant_load_mark_dependence : $@convention(thin) (@in Klass, @guaranteed Builtin.NativeObject) -> @owned (Klass, Klass) {
 bb0(%0 : $*Klass, %1 : @guaranteed $Builtin.NativeObject):
   %2 = mark_dependence %0 : $*Klass on %1 : $Builtin.NativeObject
   %4 = load [copy] %2 : $*Klass