Merge pull request swiftlang#8810 from atrick/access

Author: swift-ci

lib/SILOptimizer/Mandatory/AccessEnforcementSelection.cpp

@@ -87,7 +87,12 @@ class SelectEnforcement {
   void propagateEscapesFrom(SILBasicBlock *bb);
 
   bool hasPotentiallyEscapedAt(SILInstruction *inst);
-  bool hasPotentiallyEscapedAtAnyReachableBlock(BeginAccessInst *access);
+
+  typedef llvm::SmallSetVector<SILBasicBlock*, 8> BlockSetVector;
+  void findBlocksAccessedAcross(EndAccessInst *endAccess,
+                                BlockSetVector &blocksAccessedAcross);
+  bool hasPotentiallyEscapedAtAnyReachableBlock(
+    BeginAccessInst *access, BlockSetVector &blocksAccessedAcross);
 
   void updateAccesses();
   void updateAccess(BeginAccessInst *access);
@@ -216,22 +221,53 @@ bool SelectEnforcement::hasPotentiallyEscapedAt(SILInstruction *point) {
   return false;
 }
 
-bool SelectEnforcement::hasPotentiallyEscapedAtAnyReachableBlock(
-                                                      BeginAccessInst *access) {
+/// Add all blocks to `Worklist` between the given `endAccess` and its
+/// `begin_access` in which the access is active at the end of the block.
+void SelectEnforcement::findBlocksAccessedAcross(
+  EndAccessInst *endAccess, BlockSetVector &blocksAccessedAcross) {
+
   // Fast path: we're not tracking any escapes.  (But the box should
   // probably have been promoted to the stack in this case.)
   if (StateMap.empty())
-    return false;
+    return;
+
+  SILBasicBlock *beginBB = endAccess->getBeginAccess()->getParent();
+  if (endAccess->getParent() == beginBB)
+    return;
+
+  assert(Worklist.empty());
+  Worklist.push_back(endAccess->getParent());
+  while (!Worklist.empty()) {
+    SILBasicBlock *bb = Worklist.pop_back_val();
+    for (auto *predBB : bb->getPredecessorBlocks()) {
+      if (!blocksAccessedAcross.insert(predBB)) continue;
+      if (predBB == beginBB) continue;
+      Worklist.push_back(predBB);
+    }
+  }
+}
 
-  auto bb = access->getParent();
+bool SelectEnforcement::hasPotentiallyEscapedAtAnyReachableBlock(
+  BeginAccessInst *access, BlockSetVector &blocksAccessedAcross) {
 
   assert(Worklist.empty());
   SmallPtrSet<SILBasicBlock*, 8> visited;
-  visited.insert(bb);
-  Worklist.push_back(bb);
+
+  // Don't follow any paths that lead to an end_access.
+  for (auto endAccess : access->getEndAccesses())
+    visited.insert(endAccess->getParent());
+
+  /// Initialize the worklist with all blocks that exit the access path.
+  for (SILBasicBlock *bb : blocksAccessedAcross) {
+    for (SILBasicBlock *succBB : bb->getSuccessorBlocks()) {
+      if (blocksAccessedAcross.count(succBB)) continue;
+      if (visited.insert(succBB).second)
+        Worklist.push_back(succBB);
+    }
+  }
 
   while (!Worklist.empty()) {
-    bb = Worklist.pop_back_val();
+    SILBasicBlock *bb = Worklist.pop_back_val();
     assert(visited.count(bb));
 
     // If we're tracking information for this block, there's an escape.
@@ -259,27 +295,23 @@ void SelectEnforcement::updateAccess(BeginAccessInst *access) {
   assert(access->getEnforcement() == SILAccessEnforcement::Unknown);
 
   // Check whether the variable escaped before any of the end_accesses.
-  bool anyDynamic = false;
-  bool hasEndAccess = false;
+  BlockSetVector blocksAccessedAcross;
   for (auto endAccess : access->getEndAccesses()) {
-    hasEndAccess = true;
-    if (hasPotentiallyEscapedAt(endAccess)) {
-      anyDynamic = true;
-      break;
-    }
+    if (hasPotentiallyEscapedAt(endAccess))
+      return setDynamicEnforcement(access);
+
+    // Add all blocks to blocksAccessedAcross between begin_access and this
+    // end_access.
+    findBlocksAccessedAcross(endAccess, blocksAccessedAcross);
   }
+  assert(blocksAccessedAcross.empty()
+         || blocksAccessedAcross.count(access->getParent()));
 
-  // If so, make the access dynamic.
-  if (anyDynamic)
+  // For every path through this access that doesn't reach an end_access, check
+  // if any block reachable from that path can see an escaped value.
+  if (hasPotentiallyEscapedAtAnyReachableBlock(access, blocksAccessedAcross)) {
     return setDynamicEnforcement(access);
-
-  // Otherwise, if there are no end_access instructions,
-  // check the terminators of every reachable block.
-  if (!hasEndAccess) {
-    if (hasPotentiallyEscapedAtAnyReachableBlock(access))
-      return setDynamicEnforcement(access);
   }
-
   // Otherwise, use static enforcement.
   setStaticEnforcement(access);
 }

test/SILOptimizer/access_marker_selection.sil

@@ -0,0 +1,40 @@
+// RUN: %target-sil-opt -enable-sil-verify-all %s -access-enforcement-selection -enforce-exclusivity=checked | %FileCheck %s
+
+sil_stage raw
+
+import Builtin
+import Swift
+
+sil @closure : $@convention(thin) (@owned { var Builtin.Int64 }) -> () {
+bb0(%0 : ${var Builtin.Int64}):
+  %empty = tuple ()
+  return %empty : $()
+}
+
+// An access that escapes on an unreachable path must be dynamic.
+//
+// CHECK-LABEL: sil @partialUnreachable : $@convention(thin) () -> () {
+// CHECK: %[[ACCESS:.*]] = begin_access [modify] [dynamic] %{{.*}} : $*Builtin.Int64
+// CHECK: bb1:
+// CHECK: end_access %[[ACCESS]] : $*Builtin.Int64
+// CHECK: return
+// CHECK: bb2:
+// CHECK: partial_apply
+// CHECK: unreachable
+sil @partialUnreachable : $@convention(thin) () -> () {
+bb0:
+  %box = alloc_box ${ var Builtin.Int64 }, var, name "x"
+  %addr = project_box %box : ${ var Builtin.Int64 }, 0
+  %write = begin_access [modify] [unknown] %addr : $*Builtin.Int64
+  cond_br undef, bb1, bb2
+
+bb1:
+  end_access %write : $*Builtin.Int64
+  %empty = tuple ()
+  return %empty : $()
+
+bb2:
+  %f = function_ref @closure : $@convention(thin) (@owned { var Builtin.Int64 }) -> ()
+  %closure = partial_apply %f(%box) : $@convention(thin) (@owned { var Builtin.Int64 }) -> ()
+  unreachable
+}