Skip to content

Commit e5d0f38

Browse files
committed
[moveOnly] Add a new experimental move only checker that checks noImplicitCopy variables.
NOTE: This is only available when the flag -enable-experimental-move-only. There are no effects when the flag is disabled. The way that this works is that it takes advantage of the following changes to SILGen emission: * When SILGen initializes a let with NoImplicitCopyAttribute, SILGen now emits a begin_borrow [lexical] + copy + move_only. This is a pattern that we can check and know that we are processing a move only value. When performing move checking, we check move_only as a move only value and that it isn't consumed multiple times. * The first point works well for emitting all diagnostics except for initializing an additional let var. To work around that I changed let initialization to always bind to an owned value to a move of that owned value. There is no semantic difference since that value is going to be consumed by the binding operation anyways so we effectively just move the cleanup from the original value we wanted to bind to the move. We still then actually borrow the new let value with a begin_borrow [lexical] for the new let value. This ensures that an initialization of a let value appears to be a consuming use to the move only value checker while ensuring that the value has a proper begin_borrow [lexical]. Some notes on functionality: 1. This attribute can only be applied to local 'let'. 2. "print" due to how we call it today with a vararg array is treated as a consuming use (unfortunately). 3. I have not added the builtin copy operator yet, but I recently added a _move skeleton attribute so one can end the lifetimes of these values early. 4. This supports all types that are not address only types (similar to _move). To support full on address only types we need opaque values. rdar://83957088
1 parent 1b3fa26 commit e5d0f38

File tree

11 files changed

+738
-31
lines changed

11 files changed

+738
-31
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,5 +690,16 @@ NOTE(capturepromotion_variable_defined_here,none,
690690
ERROR(move_operator_used_on_generic_or_existential_value, none,
691691
"move() used on a generic or existential value", ())
692692

693+
// noimplicitcopy on generic or existential binding
694+
ERROR(noimplicitcopy_used_on_generic_or_existential, none,
695+
"@_noImplicitCopy can not be used on a generic or existential typed "
696+
"binding or a nominal type containing such typed things", ())
697+
698+
// move only checker diagnostics
699+
ERROR(sil_moveonlychecker_value_consumed_more_than_once, none,
700+
"'%0' consumed more than once", (StringRef))
701+
NOTE(sil_moveonlychecker_consuming_use_here, none,
702+
"consuming use", ())
703+
693704
#define UNDEFINE_DIAGNOSTIC_MACROS
694705
#include "DefineDiagnosticMacros.h"

include/swift/SIL/DebugUtils.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,18 @@ inline SILInstruction *getSingleNonDebugUser(SILValue V) {
174174
return I->getUser();
175175
}
176176

177+
/// If \p value has a single debug user, return the operand associated with that
178+
/// use. Otherwise, returns nullptr.
179+
inline Operand *getSingleDebugUse(SILValue value) {
180+
auto range = getDebugUses(value);
181+
auto ii = range.begin(), ie = range.end();
182+
if (ii == ie)
183+
return nullptr;
184+
if (std::next(ii) != ie)
185+
return nullptr;
186+
return *ii;
187+
}
188+
177189
/// Erases the instruction \p I from it's parent block and deletes it, including
178190
/// all debug instructions which use \p I.
179191
/// Precondition: The instruction may only have debug instructions as uses.

include/swift/SILOptimizer/PassManager/Passes.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,8 @@ PASS(BugReducerTester, "bug-reducer-tester",
416416
"sil-bug-reducer Tool Testing by Asserting on a Sentinel Function")
417417
PASS(AssemblyVisionRemarkGenerator, "assembly-vision-remark-generator",
418418
"Emit assembly vision remarks that provide source level guidance of where runtime calls ended up")
419+
PASS(MoveOnlyChecker, "sil-move-only-checker",
420+
"Pass that enforces move only invariants on raw SIL")
419421
PASS(PruneVTables, "prune-vtables",
420422
"Mark class methods that do not require vtable dispatch")
421423
PASS_RANGE(AllPasses, AADumper, PruneVTables)

include/swift/SILOptimizer/Utils/CanonicalOSSALifetime.h

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,13 @@ extern llvm::Statistic NumCopiesGenerated;
114114
/// Insert a copy on this operand. Trace and update stats.
115115
void copyLiveUse(Operand *use, InstModCallbacks &instModCallbacks);
116116

117+
/// Diagnose that the given value is a move only type for which \p use causes a
118+
/// need to copy the move only value.
119+
///
120+
/// copy on this operand. Trace and update stats.
121+
void diagnoseRequiredCopyOfMoveOnly(Operand *use,
122+
InstModCallbacks &instModCallbacks);
123+
117124
/// Information about consumes on the extended-lifetime boundary. Consuming uses
118125
/// within the lifetime are not included--they will consume a copy after
119126
/// rewriting. For borrowed def values, the consumes do not include the end of
@@ -244,6 +251,15 @@ class CanonicalizeOSSALifetime {
244251
/// If true, then new destroy_value instructions will be poison.
245252
bool poisonRefsMode;
246253

254+
/// If true and we are processing a value of move_only type, emit a diagnostic
255+
/// when-ever we need to insert a copy_value.
256+
std::function<void(Operand *)> moveOnlyCopyValueNotification;
257+
258+
/// If true and we are processing a value of move_only type, pass back to the
259+
/// caller any consuming uses that are going to be used as part of the final
260+
/// lifetime boundary in case we need to emit diagnostics.
261+
std::function<void(Operand *)> moveOnlyFinalConsumingUse;
262+
247263
NonLocalAccessBlockAnalysis *accessBlockAnalysis;
248264
// Lazily initialize accessBlocks only when
249265
// extendLivenessThroughOverlappingAccess is invoked.
@@ -296,10 +312,27 @@ class CanonicalizeOSSALifetime {
296312
CanonicalOSSAConsumeInfo consumes;
297313

298314
public:
299-
CanonicalizeOSSALifetime(bool pruneDebugMode, bool poisonRefsMode,
300-
NonLocalAccessBlockAnalysis *accessBlockAnalysis,
301-
DominanceInfo *domTree, InstructionDeleter &deleter)
315+
void maybeNotifyMoveOnlyCopy(Operand *use) {
316+
if (!moveOnlyCopyValueNotification)
317+
return;
318+
moveOnlyCopyValueNotification(use);
319+
}
320+
321+
void maybeNotifyFinalConsumingUse(Operand *use) {
322+
if (!moveOnlyFinalConsumingUse)
323+
return;
324+
moveOnlyFinalConsumingUse(use);
325+
}
326+
327+
CanonicalizeOSSALifetime(
328+
bool pruneDebugMode, bool poisonRefsMode,
329+
NonLocalAccessBlockAnalysis *accessBlockAnalysis, DominanceInfo *domTree,
330+
InstructionDeleter &deleter,
331+
std::function<void(Operand *)> moveOnlyCopyValueNotification = nullptr,
332+
std::function<void(Operand *)> moveOnlyFinalConsumingUse = nullptr)
302333
: pruneDebugMode(pruneDebugMode), poisonRefsMode(poisonRefsMode),
334+
moveOnlyCopyValueNotification(moveOnlyCopyValueNotification),
335+
moveOnlyFinalConsumingUse(moveOnlyFinalConsumingUse),
303336
accessBlockAnalysis(accessBlockAnalysis), domTree(domTree),
304337
deleter(deleter) {}
305338

lib/SILGen/SILGenDecl.cpp

Lines changed: 87 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,18 @@
1818
#include "Scope.h"
1919
#include "SwitchEnumBuilder.h"
2020
#include "swift/AST/ASTMangler.h"
21+
#include "swift/AST/DiagnosticsSIL.h"
2122
#include "swift/AST/GenericEnvironment.h"
2223
#include "swift/AST/Module.h"
2324
#include "swift/AST/NameLookup.h"
24-
#include "swift/AST/ProtocolConformance.h"
2525
#include "swift/AST/PropertyWrappers.h"
26+
#include "swift/AST/ProtocolConformance.h"
2627
#include "swift/Basic/ProfileCounter.h"
2728
#include "swift/SIL/FormalLinkage.h"
2829
#include "swift/SIL/PrettyStackTrace.h"
2930
#include "swift/SIL/SILArgument.h"
3031
#include "swift/SIL/SILDebuggerClient.h"
32+
#include "swift/SIL/SILInstruction.h"
3133
#include "swift/SIL/SILType.h"
3234
#include "swift/SIL/TypeLowering.h"
3335
#include "llvm/ADT/SmallString.h"
@@ -36,6 +38,13 @@
3638
using namespace swift;
3739
using namespace Lowering;
3840

41+
// Utility for emitting diagnostics.
42+
template <typename... T, typename... U>
43+
static void diagnose(ASTContext &Context, SourceLoc loc, Diag<T...> diag,
44+
U &&...args) {
45+
Context.Diags.diagnose(loc, diag, std::forward<U>(args)...);
46+
}
47+
3948
void Initialization::_anchor() {}
4049
void SILDebuggerClient::anchor() {}
4150

@@ -254,10 +263,6 @@ class DestroyLocalVariable : public Cleanup {
254263

255264
void emit(SILGenFunction &SGF, CleanupLocation l,
256265
ForUnwind_t forUnwind) override {
257-
SILValue val = SGF.VarLocs[Var].value;
258-
if (SGF.getASTContext().SILOpts.EnableExperimentalLexicalLifetimes &&
259-
val->getOwnershipKind() != OwnershipKind::None)
260-
SGF.B.createEndBorrow(l, val);
261266
SGF.destroyLocalVariable(l, Var);
262267
}
263268

@@ -456,7 +461,16 @@ class LetValueInitialization : public Initialization {
456461
needsTemporaryBuffer =
457462
lowering.isAddressOnly() && SGF.silConv.useLoweredAddresses();
458463
}
459-
464+
465+
// Make sure that we have a non-address only type when binding a
466+
// @_noImplicitCopy let.
467+
if (SGF.getASTContext().LangOpts.EnableExperimentalMoveOnly &&
468+
lowering.isAddressOnly() &&
469+
vd->getAttrs().hasAttribute<NoImplicitCopyAttr>()) {
470+
auto d = diag::noimplicitcopy_used_on_generic_or_existential;
471+
diagnose(SGF.getASTContext(), vd->getLoc(), d);
472+
}
473+
460474
if (needsTemporaryBuffer) {
461475
address = SGF.emitTemporaryAllocation(vd, lowering.getLoweredType());
462476
if (isUninitialized)
@@ -517,7 +531,7 @@ class LetValueInitialization : public Initialization {
517531
SplitCleanups);
518532
}
519533

520-
void bindValue(SILValue value, SILGenFunction &SGF) {
534+
void bindValue(SILValue value, SILGenFunction &SGF, bool wasPlusOne) {
521535
assert(!SGF.VarLocs.count(vd) && "Already emitted this vardecl?");
522536
// If we're binding an address to this let value, then we can use it as an
523537
// address later. This happens when binding an address only parameter to
@@ -527,9 +541,33 @@ class LetValueInitialization : public Initialization {
527541
SILLocation PrologueLoc(vd);
528542

529543
if (SGF.getASTContext().SILOpts.EnableExperimentalLexicalLifetimes &&
530-
value->getOwnershipKind() != OwnershipKind::None)
531-
value = SILValue(
532-
SGF.B.createBeginBorrow(PrologueLoc, value, /*isLexical*/ true));
544+
value->getOwnershipKind() != OwnershipKind::None) {
545+
if (!SGF.getASTContext().LangOpts.EnableExperimentalMoveOnly) {
546+
value = SILValue(
547+
SGF.B.createBeginBorrow(PrologueLoc, value, /*isLexical*/ true));
548+
} else {
549+
// If we have an owned value that had a cleanup, then create a
550+
// move_value that acts as a consuming use of the value. The reason why
551+
// we want this is even if we are only performing a borrow for our
552+
// lexical lifetime, we want to ensure that our defs see this
553+
// initialization as consuming this value.
554+
if (value->getOwnershipKind() == OwnershipKind::Owned) {
555+
assert(wasPlusOne);
556+
value = SILValue(SGF.B.createMoveValue(PrologueLoc, value));
557+
}
558+
559+
if (vd->getAttrs().hasAttribute<NoImplicitCopyAttr>()) {
560+
value = SILValue(SGF.B.createBeginBorrow(PrologueLoc, value,
561+
/*isLexical*/ true));
562+
value = SGF.B.createCopyValue(PrologueLoc, value);
563+
value = SGF.B.createMoveValue(PrologueLoc, value);
564+
} else {
565+
value = SILValue(
566+
SGF.B.createBeginBorrow(PrologueLoc, value, /*isLexical*/ true));
567+
}
568+
}
569+
}
570+
533571
SGF.VarLocs[vd] = SILGenFunction::VarLoc::get(value);
534572

535573
// Emit a debug_value[_addr] instruction to record the start of this value's
@@ -540,26 +578,27 @@ class LetValueInitialization : public Initialization {
540578
SILDebugVariable DbgVar(vd->isLet(), /*ArgNo=*/0);
541579
SGF.B.emitDebugDescription(PrologueLoc, value, DbgVar);
542580
}
543-
581+
544582
void copyOrInitValueInto(SILGenFunction &SGF, SILLocation loc,
545583
ManagedValue value, bool isInit) override {
546584
// If this let value has an address, we can handle it just like a single
547585
// buffer value.
548-
if (hasAddress())
586+
if (hasAddress()) {
549587
return SingleBufferInitialization::
550588
copyOrInitValueIntoSingleBuffer(SGF, loc, value, isInit, address);
551-
589+
}
590+
552591
// Otherwise, we bind the value.
553592
if (isInit) {
554593
// Disable the rvalue expression cleanup, since the let value
555594
// initialization has a cleanup that lives for the entire scope of the
556595
// let declaration.
557-
bindValue(value.forward(SGF), SGF);
596+
bindValue(value.forward(SGF), SGF, value.isPlusOne(SGF));
558597
} else {
559598
// Disable the expression cleanup of the copy, since the let value
560599
// initialization has a cleanup that lives for the entire scope of the
561600
// let declaration.
562-
bindValue(value.copyUnmanaged(SGF, loc).forward(SGF), SGF);
601+
bindValue(value.copyUnmanaged(SGF, loc).forward(SGF), SGF, true);
563602
}
564603
}
565604

@@ -1715,16 +1754,38 @@ void SILGenFunction::destroyLocalVariable(SILLocation silLoc, VarDecl *vd) {
17151754
// For 'let' bindings, we emit a release_value or destroy_addr, depending on
17161755
// whether we have an address or not.
17171756
SILValue Val = loc.value;
1718-
if (!Val->getType().isAddress()) {
1719-
SILValue valueToBeDestroyed;
1720-
if (getASTContext().SILOpts.EnableExperimentalLexicalLifetimes &&
1721-
Val->getOwnershipKind() != OwnershipKind::None) {
1722-
auto *inst = cast<BeginBorrowInst>(Val.getDefiningInstruction());
1723-
valueToBeDestroyed = inst->getOperand();
1724-
} else {
1725-
valueToBeDestroyed = Val;
1726-
}
1727-
B.emitDestroyValueOperation(silLoc, valueToBeDestroyed);
1728-
} else
1757+
1758+
if (Val->getType().isAddress()) {
17291759
B.createDestroyAddr(silLoc, Val);
1760+
return;
1761+
}
1762+
1763+
if (!getASTContext().SILOpts.EnableExperimentalLexicalLifetimes ||
1764+
Val->getOwnershipKind() == OwnershipKind::None) {
1765+
B.emitDestroyValueOperation(silLoc, Val);
1766+
return;
1767+
}
1768+
1769+
if (auto *bbi = dyn_cast<BeginBorrowInst>(Val.getDefiningInstruction())) {
1770+
B.createEndBorrow(silLoc, bbi);
1771+
B.emitDestroyValueOperation(silLoc, bbi->getOperand());
1772+
return;
1773+
}
1774+
1775+
if (getASTContext().LangOpts.EnableExperimentalMoveOnly) {
1776+
if (auto *mvi = dyn_cast<MoveValueInst>(Val.getDefiningInstruction())) {
1777+
if (auto *cvi = dyn_cast<CopyValueInst>(mvi->getOperand())) {
1778+
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
1779+
if (bbi->isLexical()) {
1780+
B.emitDestroyValueOperation(silLoc, mvi);
1781+
B.createEndBorrow(silLoc, bbi);
1782+
B.emitDestroyValueOperation(silLoc, bbi->getOperand());
1783+
return;
1784+
}
1785+
}
1786+
}
1787+
}
1788+
}
1789+
1790+
llvm_unreachable("unhandled case");
17301791
}

lib/SILOptimizer/Mandatory/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ target_sources(swiftSILOptimizer PRIVATE
1717
IRGenPrepare.cpp
1818
LowerHopToActor.cpp
1919
MandatoryInlining.cpp
20+
MoveOnlyChecker.cpp
2021
NestedSemanticFunctionCheck.cpp
2122
OptimizeHopToExecutor.cpp
2223
PredictableMemOpt.cpp

0 commit comments

Comments
 (0)