Merge pull request #77908 from nate-chandler/rdar139840307

[BarrierAccessScopes] Handle end_access instructions' barrierness introduced during run.

Author: nate-chandler

include/swift/SILOptimizer/Analysis/VisitBarrierAccessScopes.h

@@ -156,7 +156,7 @@ class VisitBarrierAccessScopes {
       // whose successors are in the region) blocks in the region only the top
       // of which was already visited.  Either way, the instructions between the
       // local gen and its kill have not yet been visited.  Visit them now.
-      auto foundLocalKill = visitBlockFromGenUntilBegin(instruction);
+      auto foundLocalKill = visitBlockFromGenThroughKill(instruction);
       assert(foundLocalKill && "local gen without local kill?!");
       (void)foundLocalKill;
     }
@@ -165,13 +165,14 @@ class VisitBarrierAccessScopes {
 private:
   /// Entry points for visiting: they visit increasingly large portions of a
   /// block.
-  /// - visitBlockFromGenUntilBegin: Instructions and phi until a kill.
+  /// - visitBlockFromGenThroughKill: Instructions and phi through the first
+  ///                                 kill.
   /// - visitBlockFromGen: Instructions, phi, and begin.
   /// - visitBlock: End, instructions, phi, and begin.
 
   /// Visit instructions and phis starting from the specified gen until a kill
   /// is found.
-  bool visitBlockFromGenUntilBegin(SILInstruction *from) {
+  bool visitBlockFromGenThroughKill(SILInstruction *from) {
     assert(effects.effectForInstruction(from) == Effects::Effect::Gen());
     for (auto *instruction = from; instruction;
          instruction = instruction->getPreviousInstruction()) {
@@ -198,9 +199,21 @@ class VisitBarrierAccessScopes {
         [&](SILBasicBlock *successor) { return visited.contains(successor); }));
 
     visited.insert(block);
-    bool foundLocalKill = visitBlockFromGenUntilBegin(from);
-    assert(!foundLocalKill && "found local kill for non-local gen?!");
-    (void)foundLocalKill;
+    for (auto *instruction = from; instruction;
+         instruction = instruction->getPreviousInstruction()) {
+      if (visitInstruction(instruction)) {
+        // New kills are incrementally added as access scopes are determined to
+        // be barriers.  For this reason, gens may newly be discovered to be
+        // local.  This can only happen when the kill which makes the gen local
+        // ends an access scope (i.e. is an end_access).
+        assert(isa<EndAccessInst>(instruction) &&
+               "found preexisting local kill for initially-non-local gen?!");
+        // Even so, the remainder of the block must still be visited.
+      }
+    }
+    if (block->hasPhi()) {
+      visitPhi(block);
+    }
     visitBlockBegin(block);
   }
 
@@ -259,8 +272,9 @@ class VisitBarrierAccessScopes {
       }
     }
     // If any of this block's predecessors haven't already been visited, it
-    // means that they aren't in the region and consequently this block is a
-    // barrier block.
+    // means EITHER that those predecessors aren't in the region OR that a
+    // barrier was encountered when visiting some (iterative) successor of that
+    // predecessor.  Either way, this block is a barrier block as a result.
     if (llvm::any_of(block->getSuccessorBlocks(), [&](SILBasicBlock *block) {
           return !visited.contains(block);
         })) {

lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp

@@ -296,8 +296,8 @@ Dataflow::Effect Dataflow::effectForPhi(SILBasicBlock *block) {
 }
 
 /// Finds end_access instructions which are barriers to hoisting because the
-/// access scopes they contain barriers to hoisting.  Hoisting end_borrows into
-/// such access scopes could introduce exclusivity violations.
+/// access scopes they end contain barriers to hoisting.  Hoisting end_borrows
+/// into such access scopes could introduce exclusivity violations.
 ///
 /// Implements BarrierAccessScopeFinder::Visitor
 class BarrierAccessScopeFinder final {
@@ -501,35 +501,28 @@ bool swift::shrinkBorrowScope(
 namespace swift::test {
 // Arguments:
 // - BeginBorrowInst - the introducer for the scope to shrink
-// - bool - the expected return value of shrinkBorrowScope
-// - variadic list of values consisting of the copies expected to be rewritten
 // Dumps:
 // - DELETED:  <<instruction deleted>>
 static FunctionTest ShrinkBorrowScopeTest(
-    "shrink-borrow-scope", [](auto &function, auto &arguments, auto &test) {
+    "shrink_borrow_scope", [](auto &function, auto &arguments, auto &test) {
       auto instruction = arguments.takeValue();
-      auto expected = arguments.takeBool();
       auto *bbi = cast<BeginBorrowInst>(instruction);
       auto *analysis = test.template getAnalysis<BasicCalleeAnalysis>();
       SmallVector<CopyValueInst *, 4> modifiedCopyValueInsts;
       InstructionDeleter deleter(
           InstModCallbacks().onDelete([&](auto *instruction) {
             llvm::outs() << "DELETED:\n";
             instruction->print(llvm::outs());
+            instruction->eraseFromParent();
+
           }));
       auto shrunk =
           shrinkBorrowScope(*bbi, deleter, analysis, modifiedCopyValueInsts);
-      unsigned index = 0;
+      auto *shrunkString = shrunk ? "shrunk" : "did not shrink";
+      llvm::outs() << "Result: " << shrunkString << "\n";
+      llvm::outs() << "Rewrote the following copies:\n";
       for (auto *cvi : modifiedCopyValueInsts) {
-        auto expectedCopy = arguments.takeValue();
-        llvm::outs() << "rewritten copy " << index << ":\n";
-        llvm::outs() << "expected:\n";
-        expectedCopy->print(llvm::outs());
-        llvm::outs() << "got:\n";
         cvi->print(llvm::outs());
-        assert(cvi == expectedCopy);
-        ++index;
       }
-      assert(expected == shrunk && "didn't shrink expectedly!?");
     });
 } // namespace swift::test

test/SILOptimizer/hoist_destroy_addr.sil

@@ -1174,7 +1174,7 @@ entry(%addr : $*X):
   return %retval : $()
 }
 
-// Even though the apply of %empty is not a deinit barrier (c.f.
+// Even though the apply of %empty is not a deinit barrier (cf.
 // hoist_over_apply_of_non_barrier_fn), verify that the destroy_addr is not
 // hoisted, because MoS is move-only.
 // CHECK-LABEL: sil [ossa] @dont_move_destroy_addr_of_moveonly_struct : {{.*}} {

test/SILOptimizer/shrink_borrow_scope.unit.sil

@@ -5,16 +5,21 @@ import Swift
 
 class C {}
 
+struct X {}
+
 sil [ossa] @get_owned_c : $@convention(thin) () -> (@owned C)
 sil [ossa] @callee_guaranteed: $@convention(thin) (@guaranteed C) -> ()
+sil [ossa] @barrier : $@convention(thin) () -> ()
+
 
 // CHECK-LABEL: begin running test 1 of {{[^,]+}} on nohoist_over_rewritten_copy
 // CHECK-NOT: DELETED:
 // CHECK-NOT: end_borrow
+// CHECK: Result: did not shrink
 // CHECK-LABEL: end running test 1 of {{[^,]+}} on nohoist_over_rewritten_copy
 sil [ossa] @nohoist_over_rewritten_copy : $@convention(thin) () -> (@owned C, @owned C) {
 entry:
-  specify_test "shrink-borrow-scope @trace true @trace[1]"
+  specify_test "shrink_borrow_scope %lifetime"
   %get_owned_c = function_ref @get_owned_c : $@convention(thin) () -> (@owned C)
   %instance = apply %get_owned_c() : $@convention(thin) () -> (@owned C)
   %lifetime = begin_borrow [lexical] %instance : $C
@@ -28,3 +33,37 @@ entry:
   return %retval : $(C, C)
 }
 
+// CHECK-LABEL: begin running test {{.*}} on nohoist_over_access_scope_barrier_1
+// CHECK:       Result: did not shrink
+// CHECK-LABEL: end running test {{.*}} on nohoist_over_access_scope_barrier_1
+sil [ossa] @nohoist_over_access_scope_barrier_1 : $@convention(thin) (@guaranteed C, @inout X) -> () {
+entry(%c : @guaranteed $C, %addr : $*X):
+  %borrow = begin_borrow %c : $C
+  specify_test "shrink_borrow_scope %borrow"
+  %access = begin_access [modify] [dynamic] %addr : $*X
+  cond_br undef, kill_introducer, newly_local_gen
+
+kill_introducer:
+  cond_br undef, left, right
+
+left:
+  %barrier = function_ref @barrier : $@convention(thin) () -> ()
+  apply %barrier() : $@convention(thin) () -> ()
+  end_access %access : $*X
+  end_borrow %borrow : $C
+  br exit
+
+right:
+  end_access %access : $*X
+  end_borrow %borrow : $C
+  br exit
+
+newly_local_gen:
+  end_access %access : $*X
+  end_borrow %borrow : $C
+  br exit
+
+exit:
+  %retval = tuple ()
+  return %retval : $()
+}

validation-test/SILOptimizer/gh77693.swift

@@ -0,0 +1,17 @@
+// RUN: %target-swift-frontend -c -O %s
+
+protocol P {}
+enum E {
+  case c(any P)
+  var d: [String:String] { [:] }
+}
+
+final class C {
+  var o: [String:String]?
+}
+
+func f(_ e: E?) {
+  if let e {
+    C().o?.merge(e.d) { c, _ in c }
+  }
+}