Skip to content

MoveOnlyAddressChecker: Treat opaque accesses consistently in CopiedLoadBorrowEliminationVisitor. #71903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/SILGen/SILGenDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1726,8 +1726,7 @@ void SILGenFunction::emitStmtCondition(StmtCondition Cond, JumpDest FalseDest,
// Begin a new binding scope, which is popped when the next innermost debug
// scope ends. The cleanup location loc isn't the perfect source location
// but it's close enough.
B.getSILGenFunction().enterDebugScope(loc,
/*isBindingScope=*/true);
B.getSILGenFunction().enterDebugScope(loc, /*isBindingScope=*/true);
InitializationPtr initialization =
emitPatternBindingInitialization(elt.getPattern(), FalseDest);

Expand Down
28 changes: 18 additions & 10 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1573,6 +1573,19 @@ struct CopiedLoadBorrowEliminationState {
}
};

static bool shouldVisitAsEndPointUse(Operand *op) {
// If an access is static and marked as "no nested conflict", we use that
// in switch codegen to mark an opaque sub-access that move-only checking
// should not look through.
if (auto ba = dyn_cast<BeginAccessInst>(op->getUser())) {
if (ba->getEnforcement() == SILAccessEnforcement::Static
&& ba->hasNoNestedConflict()) {
return true;
}
}
return false;
}

/// An early transform that we run to convert any load_borrow that are copied
/// directly or that have any subelement that is copied to a load [copy]. This
/// lets the rest of the optimization handle these as appropriate.
Expand All @@ -1583,6 +1596,10 @@ struct CopiedLoadBorrowEliminationVisitor
CopiedLoadBorrowEliminationVisitor(CopiedLoadBorrowEliminationState &state)
: state(state) {}

bool visitTransitiveUseAsEndPointUse(Operand *op) {
return shouldVisitAsEndPointUse(op);
}

bool visitUse(Operand *op) {
LLVM_DEBUG(llvm::dbgs() << "CopiedLBElim visiting ";
llvm::dbgs() << " User: " << *op->getUser());
Expand Down Expand Up @@ -1978,16 +1995,7 @@ struct GatherUsesVisitor : public TransitiveAddressWalker<GatherUsesVisitor> {
} // end anonymous namespace

bool GatherUsesVisitor::visitTransitiveUseAsEndPointUse(Operand *op) {
// If an access is static and marked as "no nested conflict", we use that
// in switch codegen to mark an opaque sub-access that move-only checking
// should not look through.
if (auto ba = dyn_cast<BeginAccessInst>(op->getUser())) {
if (ba->getEnforcement() == SILAccessEnforcement::Static
&& ba->hasNoNestedConflict()) {
return true;
}
}
return false;
return shouldVisitAsEndPointUse(op);
}

// Filter out recognized uses that do not write to memory.
Expand Down
27 changes: 27 additions & 0 deletions test/SILOptimizer/moveonly_borrowing_switch_load_borrow.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// RUN: %target-swift-frontend -enable-experimental-feature BorrowingSwitch -enable-experimental-feature NoncopyableGenerics -emit-sil -verify %s

struct Box<Wrapped: ~Copyable>: ~Copyable {
init(_ element: consuming Wrapped) { }
}

struct Tree<Element>: ~Copyable {
struct Node: ~Copyable { }

enum Branch: ~Copyable {
case empty
case more(Box<Node>)
}
var root: Branch = .empty
}

extension Tree {
mutating func insert(_ element: consuming Element) {
root =
switch root {
case .empty:
.more(Box(Node()))
case .more:
fatalError()
}
}
}