TempRValueOpt: fix the handling of begin_access

Consider the related end_access instructions as uses to correctly mark the end of the lifetime of the temporary.
This fixes a miscompile in case there is a modification of the copy-source between an begin_access and end_access.

Author: eeckstein

lib/SILOptimizer/Transforms/TempRValueElimination.cpp

@@ -153,9 +153,24 @@ collectLoads(Operand *addressUse, CopyAddrInst *originalCopy,
     LLVM_DEBUG(llvm::dbgs()
                << "  Temp use may write/destroy its source" << *user);
     return false;
-  case SILInstructionKind::BeginAccessInst:
-    return cast<BeginAccessInst>(user)->getAccessKind() == SILAccessKind::Read;
-
+  case SILInstructionKind::BeginAccessInst: {
+    auto *beginAccess = cast<BeginAccessInst>(user);
+    if (beginAccess->getAccessKind() != SILAccessKind::Read)
+      return false;
+    // We don't have to recursively call collectLoads for the beginAccess
+    // result, because a SILAccessKind::Read already guarantees that there are
+    // no writes to the beginAccess result address (or any projection from it).
+    // But we have to register the end-accesses as loads to correctly mark the
+    // end-of-lifetime of the temporary object.
+    for (Operand *accessUse : beginAccess->getUses()) {
+      if (auto *endAccess = dyn_cast<EndAccessInst>(accessUse->getUser())) {
+        if (endAccess->getParent() != block)
+          return false;
+        loadInsts.insert(endAccess);
+      }
+    }
+    return true;
+  }
   case SILInstructionKind::MarkDependenceInst: {
     auto mdi = cast<MarkDependenceInst>(user);
     // If the user is the base operand of the MarkDependenceInst we can return

test/SILOptimizer/temp_rvalue_opt_ossa.sil

@@ -759,6 +759,27 @@ bb0(%0 : $*Klass):
   return %9999 : $()
 }
 
+
+// CHECK-LABEL: sil [ossa] @dont_optimize_with_modify_inside_access
+// CHECK:   [[STK:%[0-9]+]] = alloc_stack $Klass
+// CHECK:   copy_addr %0 to [initialization] [[STK]]
+// CHECK:   begin_access [read] [static] [[STK]]
+// CHECK-LABEL: } // end sil function 'dont_optimize_with_modify_inside_access'
+sil [ossa] @dont_optimize_with_modify_inside_access : $@convention(thin) (@inout Klass, @owned Klass) -> () {
+bb0(%0 : $*Klass, %1 : @owned $Klass):
+  %stack = alloc_stack $Klass
+  copy_addr %0 to [initialization] %stack : $*Klass
+  %f = function_ref @inguaranteed_user_without_result : $@convention(thin) (@in_guaranteed Klass) -> ()
+  %access = begin_access [read] [static] %stack : $*Klass
+  store %1 to [assign] %0 : $*Klass // This store prevents the optimization
+  %call = apply %f(%access) : $@convention(thin) (@in_guaranteed Klass) -> ()
+  end_access %access : $*Klass
+  destroy_addr %stack : $*Klass
+  dealloc_stack %stack : $*Klass
+  %9999 = tuple()
+  return %9999 : $()
+}
+
 /////////////////
 // Store Tests //
 /////////////////