diff --git a/include/swift/SILOptimizer/Analysis/VisitBarrierAccessScopes.h b/include/swift/SILOptimizer/Analysis/VisitBarrierAccessScopes.h index aa30d4d788205..0929128480ef7 100644 --- a/include/swift/SILOptimizer/Analysis/VisitBarrierAccessScopes.h +++ b/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(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); })) { diff --git a/lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp b/lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp index d9345d4ba0727..da5909e5dcdb2 100644 --- a/lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp +++ b/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,14 +501,11 @@ 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: <> 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(instruction); auto *analysis = test.template getAnalysis(); SmallVector modifiedCopyValueInsts; @@ -516,20 +513,16 @@ static FunctionTest ShrinkBorrowScopeTest( 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 diff --git a/test/SILOptimizer/hoist_destroy_addr.sil b/test/SILOptimizer/hoist_destroy_addr.sil index ec19968d56068..09e644cb6205e 100644 --- a/test/SILOptimizer/hoist_destroy_addr.sil +++ b/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 : {{.*}} { diff --git a/test/SILOptimizer/shrink_borrow_scope.unit.sil b/test/SILOptimizer/shrink_borrow_scope.unit.sil index 4372269cb6400..f3544ada8cdbb 100644 --- a/test/SILOptimizer/shrink_borrow_scope.unit.sil +++ b/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 : $() +} diff --git a/validation-test/SILOptimizer/gh77693.swift b/validation-test/SILOptimizer/gh77693.swift new file mode 100644 index 0000000000000..c333279f2a527 --- /dev/null +++ b/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 } + } +}