Skip to content

Commit 3250382

Browse files
committed
insert hops after end_access when possible
1 parent 7957f02 commit 3250382

File tree

1 file changed

+53
-11
lines changed

1 file changed

+53
-11
lines changed

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,13 @@
1717
#include "swift/AST/Expr.h"
1818
#include "swift/AST/Stmt.h"
1919
#include "swift/ClangImporter/ClangModule.h"
20-
#include "swift/SIL/BasicBlockData.h"
2120
#include "swift/SIL/BasicBlockBits.h"
22-
#include "swift/SIL/SILValue.h"
21+
#include "swift/SIL/BasicBlockData.h"
2322
#include "swift/SIL/InstructionUtils.h"
23+
#include "swift/SIL/MemAccessUtils.h"
2424
#include "swift/SIL/SILArgument.h"
2525
#include "swift/SIL/SILBuilder.h"
26+
#include "swift/SIL/SILValue.h"
2627
#include "swift/SILOptimizer/PassManager/Passes.h"
2728
#include "swift/SILOptimizer/PassManager/Transforms.h"
2829
#include "swift/SILOptimizer/Utils/CFGOptUtils.h"
@@ -458,7 +459,7 @@ namespace {
458459
void injectActorHops();
459460

460461
/// Given an initializing block and the live-in availability of TheMemory,
461-
/// this function injects a `hop_to_executor` instruction after the
462+
/// this function injects a `hop_to_executor` instruction soon after the
462463
/// first non-load use of TheMemory that fully-initializes it.
463464
/// An "initializing block" is one that definitely contains a such a
464465
/// non-load use, e.g., because its live-in set is *not* all-Yes, but its
@@ -801,7 +802,8 @@ static void injectHopToExecutorAfter(SILLocation loc,
801802
SILBasicBlock::iterator insertPt,
802803
SILValue actor, bool needsBorrow = true) {
803804

804-
LLVM_DEBUG(llvm::dbgs() << "hop-injector: inserting hop after " << *insertPt);
805+
LLVM_DEBUG(llvm::dbgs() << "hop-injector: requested insertion after "
806+
<< *insertPt);
805807

806808
// While insertAfter can handle terminators, it cannot handle ones that lead
807809
// to a block with multiple predecessors. I don't expect that a terminator
@@ -810,15 +812,55 @@ static void injectHopToExecutorAfter(SILLocation loc,
810812
// initialized.
811813
assert(!isa<TermInst>(*insertPt) && "unexpected hop-inject after terminator");
812814

813-
SILBuilderWithScope::insertAfter(&*insertPt, [&](SILBuilder &b) {
814-
if (needsBorrow)
815-
actor = b.createBeginBorrow(loc, actor);
815+
auto injectAfter = [&](SILInstruction *insertPt) -> void {
816+
LLVM_DEBUG(llvm::dbgs() << "hop-injector: injecting after " << *insertPt);
817+
SILBuilderWithScope::insertAfter(insertPt, [&](SILBuilder &b) {
818+
if (needsBorrow)
819+
actor = b.createBeginBorrow(loc, actor);
820+
821+
b.createHopToExecutor(loc, actor, /*mandatory=*/false);
822+
823+
if (needsBorrow)
824+
b.createEndBorrow(loc, actor);
825+
});
826+
};
827+
828+
//////
829+
// NOTE: We prefer to inject a hop outside of any access regions, so that
830+
// the dynamic access-set is empty. This is a best-effort to avoid injecting
831+
// it inside of a region, but does not account for overlapping accesses, etc.
832+
// But, I cannot think of a way to create an overlapping access with a stored
833+
// property when it is first initialized, because it's not valid to pass those
834+
// inout or capture them in a closure. - kavon
816835

817-
b.createHopToExecutor(loc, actor, /*mandatory=*/false);
836+
SILInstruction *cur = &*insertPt;
837+
BeginAccessInst *access = nullptr;
838+
839+
// Finds begin_access instructions that need hops placed after its end_access.
840+
auto getBeginAccess = [](SILValue v) -> BeginAccessInst * {
841+
return dyn_cast<BeginAccessInst>(getAccessScope(v));
842+
};
843+
844+
// If this insertion-point is after a store-like instruction, look for a
845+
// begin_access corresponding to the destination.
846+
if (auto *store = dyn_cast<StoreInst>(cur)) {
847+
access = getBeginAccess(store->getDest());
848+
} else if (auto *assign = dyn_cast<AssignInst>(cur)) {
849+
access = getBeginAccess(assign->getDest());
850+
}
851+
852+
// If we found a begin_access, then we need to inject the hop after
853+
// all of the corresponding end_accesses.
854+
if (access) {
855+
for (auto *endAccess : access->getEndAccesses())
856+
injectAfter(endAccess);
857+
858+
return;
859+
}
818860

819-
if (needsBorrow)
820-
b.createEndBorrow(loc, actor);
821-
});
861+
//////
862+
// Otherwise, we just put the hop after the original insertion point.
863+
return injectAfter(cur);
822864
}
823865

824866
void LifetimeChecker::injectActorHopForBlock(

0 commit comments

Comments
 (0)