Skip to content

Commit 4a56efe

Browse files
authored
Merge pull request swiftlang#39956 from gottesmm/pr-c8f8cf9d6d3aa0800cc5fae25efd75c625e57d3c
[moveOnly] Add a new experimental move only checker that checks noImplicitCopy variables.
2 parents 0f30978 + e5d0f38 commit 4a56efe

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)