TempRValueOpt: small fixes/improvements for extending access scopes

Don't move an end_access over a (non-aliasing) end_access. This would destroy the proper nesting of accesses.
Also, add some comments, asserts and tests.

Author: eeckstein

lib/SILOptimizer/Transforms/TempRValueElimination.cpp

@@ -367,19 +367,27 @@ bool TempRValueOptPass::extendAccessScopes(
 
   for (SILInstruction &inst : make_range(begin, end)) {
     if (auto *endAccess = dyn_cast<EndAccessInst>(&inst)) {
+      // To keep things simple, we can just move a single end_access. Also, we
+      // cannot move an end_access over a (non-aliasing) end_access.
+      if (endAccessToMove)
+        return false;
       // Is this the end of an access scope of the copy-source?
       if (!aa->isNoAlias(copySrc, endAccess->getSource())) {
-        // To keep things simple, we can just move a single end_access.
-        if (endAccessToMove)
-          return false;
+        assert(endAccess->getBeginAccess()->getAccessKind() ==
+                 SILAccessKind::Read &&
+               "a may-write end_access should not be in the copysrc lifetime");
         endAccessToMove = endAccess;
       }
     } else if (endAccessToMove) {
       // We cannot move an end_access over a begin_access. This would destroy
       // the proper nesting of accesses.
-      if (isa<BeginAccessInst>(&inst))
+      if (isa<BeginAccessInst>(&inst) || isa<BeginUnpairedAccessInst>(inst))
         return false;
       // Don't extend a read-access scope over a (potential) write.
+      // Note that inst can be a function call containing other access scopes.
+      // But doing the mayWriteToMemory check, we know that the function can
+      // only contain read accesses (to the same memory location). So it's fine
+      // to move endAccessToMove even over such a function call.
       if (aa->mayWriteToMemory(&inst, endAccessToMove->getSource()))
         return false;
     }

test/SILOptimizer/temp_rvalue_opt_ossa.sil

@@ -879,6 +879,50 @@ bb0:
   return %a : $String
 }
 
+// CHECK-LABEL: sil [ossa] @dontExtendModifyAccess
+// CHECK:      begin_access
+// CHECK-NEXT: alloc_stack
+// CHECK-NEXT: copy_addr
+// CHECK-NEXT: end_access
+// CHECK-NEXT: load
+// CHECK-LABEL: } // end sil function 'dontExtendModifyAccess'
+sil [ossa] @dontExtendModifyAccess : $@convention(thin) () -> @owned String {
+bb0:
+  %1 = global_addr @globalString : $*String
+  %3 = begin_access [modify] [dynamic] %1 : $*String
+  %4 = alloc_stack $String
+  copy_addr %3 to [initialization] %4 : $*String
+  end_access %3 : $*String
+  %6 = load [copy] %4 : $*String
+  destroy_addr %4 : $*String
+  dealloc_stack %4 : $*String
+  return %6 : $String
+}
+
+// CHECK-LABEL: sil [ossa] @dontExtendAccessScopeOverEndAccess
+// CHECK:      begin_access [read] [dynamic] %0 : $*Int
+// CHECK-NEXT: begin_access [read] [dynamic] %{{[0-9]+}} : $*String
+// CHECK-NEXT: alloc_stack
+// CHECK-NEXT: copy_addr 
+// CHECK-NEXT: end_access %{{[0-9]+}} : $*String
+// CHECK-NEXT: end_access %{{[0-9]+}} : $*Int
+// CHECK-NEXT: load
+// CHECK-LABEL: } // end sil function 'dontExtendAccessScopeOverEndAccess'
+sil [ossa] @dontExtendAccessScopeOverEndAccess : $@convention(thin) (@in_guaranteed Int) -> @owned String {
+bb0(%0 : $*Int):
+  %1 = global_addr @globalString : $*String
+  %2 = begin_access [read] [dynamic] %0 : $*Int
+  %3 = begin_access [read] [dynamic] %1 : $*String
+  %4 = alloc_stack $String
+  copy_addr %3 to [initialization] %4 : $*String
+  end_access %3 : $*String
+  end_access %2 : $*Int
+  %6 = load [copy] %4 : $*String
+  destroy_addr %4 : $*String
+  dealloc_stack %4 : $*String
+  return %6 : $String
+}
+
 // CHECK-LABEL: sil [ossa] @dontExtendAccessScopeOverBeginAccess : $@convention(thin) (@in Klass) -> () {
 // CHECK: bb0(%0 : $*Klass):
 // CHECK:   [[STACK:%.*]] = alloc_stack $Klass