DeadCodeElimination: don't remove empty access scopes

Empty access scopes can be a result of e.g. redundant-load-elimination.
It's still important to keep those access scopes to detect access violations.
Even if the load is physically not done anymore, in case of a conflicting access a propagated load is still wrong and must be detected.

rdar://164571252

Author: eeckstein

lib/SILOptimizer/Transforms/DeadCodeElimination.cpp

@@ -54,7 +54,7 @@ static bool seemsUseful(SILInstruction *I) {
   // Even though begin_access/destroy_value/copy_value/end_lifetime have
   // side-effects, they can be DCE'ed if they do not have useful
   // dependencies/reverse dependencies
-  if (isa<BeginAccessInst>(I) || isa<CopyValueInst>(I) ||
+  if (isa<CopyValueInst>(I) ||
       isa<DestroyValueInst>(I) || isa<EndLifetimeInst>(I) ||
       isa<EndBorrowInst>(I))
     return false;
@@ -361,12 +361,6 @@ void DCE::markLive() {
         }
         break;
       }
-      case SILInstructionKind::EndAccessInst: {
-        // An end_access is live only if it's begin_access is also live.
-        auto *beginAccess = cast<EndAccessInst>(&I)->getBeginAccess();
-        addReverseDependency(beginAccess, &I);
-        break;
-      }
       case SILInstructionKind::DestroyValueInst: {
         auto phi = PhiValue(I.getOperand(0));
         // Disable DCE of phis which are lexical or may have a pointer escape.

test/SILOptimizer/dead_code_elimination.sil

@@ -253,11 +253,10 @@ bb3:
   br bb1
 }
 
-// Check that DCE eliminates dead access instructions.
+// Dead accesses must not be removed
 // CHECK-LABEL: sil @dead_access
-// CHECK: bb0
-// CHECK-NEXT: tuple
-// CHECK-NEXT: return
+// CHECK:         begin_access
+// CHECK:         begin_access
 // CHECK-LABEL: end sil function 'dead_access'
 sil @dead_access : $@convention(thin) (@in Container) -> () {
 bb0(%0 : $*Container):

test/SILOptimizer/dead_code_elimination_ossa.sil

@@ -290,11 +290,10 @@ bb3:
   br bb1
 }
 
-// Check that DCE eliminates dead access instructions.
+// Dead accesses must not be removed
 // CHECK-LABEL: sil [ossa] @dead_access :
-// CHECK: bb0
-// CHECK-NEXT: tuple
-// CHECK-NEXT: return
+// CHECK:         begin_access
+// CHECK:         begin_access
 // CHECK-LABEL: } // end sil function 'dead_access'
 sil [ossa] @dead_access : $@convention(thin) (@in Container) -> () {
 bb0(%0 : $*Container):

test/SILOptimizer/exclusivity_violation.swift

@@ -0,0 +1,67 @@
+// RUN: %target-run-simple-swift(-Onone)
+// RUN: %target-run-simple-swift(-O)
+
+// REQUIRES: executable_test
+
+import StdlibUnittest
+
+
+var tests = TestSuite("exclusivity checking")
+
+struct NC: ~Copyable {
+  var i: Int = 1
+
+  mutating func add(_ other: borrowing Self) {
+    i += other.i
+    i += other.i
+    print(self.i, other.i)
+  }
+}
+
+class C1 {
+  var nc = NC()
+
+  func foo() {
+    nc.add(nc)
+  }
+}
+
+struct S {
+  var i: Int = 1
+
+  mutating func add(_ c: C2) {
+    let other = c.getS()
+    i += other.i
+    i += other.i
+    print(self.i, other.i)
+  }
+}
+
+final class C2 {
+  var s = S()
+
+  @inline(never)
+  func getS() -> S { s }
+
+  func foo() {
+    s.add(self)
+  }
+}
+
+tests.test("non-copyable type")
+  .crashOutputMatches("Simultaneous accesses")
+  .code {
+    expectCrashLater()
+
+    C1().foo()
+  }
+
+tests.test("copyable type")
+  .crashOutputMatches("Simultaneous accesses")
+  .code {
+    expectCrashLater()
+
+    C2().foo()
+  }
+
+runAllTests()