Skip to content

Commit c4a503f

Browse files
gottesmmgottesmm
authored
Merge pull request swiftlang#63578 from gottesmm/pr-e2a6558f3a5
[move-only] Prepare for handling ref_element_addr/global_addr/escaping closure captured params
2 parents 912a4a8 + b33535c commit c4a503f

27 files changed

+226
-197
lines changed

‎docs/SIL.rst

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2660,7 +2660,7 @@ which codegens to the following SIL::
26602660
bb0(%0 : @noImplicitCopy $Klass):
26612661
%1 = copyable_to_moveonlywrapper [guaranteed] %0 : $@moveOnly Klass
26622662
%2 = copy_value %1 : $@moveOnly Klass
2663-
%3 = mark_must_check [no_copy] %2 : $@moveOnly Klass
2663+
%3 = mark_must_check [no_consume_or_assign] %2 : $@moveOnly Klass
26642664
debug_value %3 : $@moveOnly Klass, let, name "x", argno 1
26652665
%4 = begin_borrow %3 : $@moveOnly Klass
26662666
%5 = function_ref @$s4test5KlassC11doSomethingyyF : $@convention(method) (@guaranteed Klass) -> ()
@@ -2730,7 +2730,7 @@ Today this codegens to the following Swift::
27302730
bb0(%0 : @noImplicitCopy $Int):
27312731
%1 = copyable_to_moveonlywrapper [owned] %0 : $Int
27322732
%2 = move_value [lexical] %1 : $@moveOnly Int
2733-
%3 = mark_must_check [no_implicit_copy] %2 : $@moveOnly Int
2733+
%3 = mark_must_check [consumable_and_assignable] %2 : $@moveOnly Int
27342734
%5 = begin_borrow %3 : $@moveOnly Int
27352735
%6 = begin_borrow %3 : $@moveOnly Int
27362736
%7 = function_ref @addIntegers : $@convention(method) (Int, Int Int.Type) -> Int
@@ -2789,7 +2789,7 @@ A hypothetical SILGen for this code is as follows::
27892789
%3 = begin_borrow [lexical] %0 : $Klass
27902790
%4 = copy_value %3 : $Klass
27912791
%5 = copyable_to_moveonlywrapper [owned] %4 : $Klass
2792-
%6 = mark_must_check [no_implicit_copy] %5 : $@moveOnly Klass
2792+
%6 = mark_must_check [consumable_and_assignable] %5 : $@moveOnly Klass
27932793
debug_value %6 : $@moveOnly Klass, let, name "value"
27942794
%8 = begin_borrow %6 : $@moveOnly Klass
27952795
%9 = copy_value %8 : $@moveOnly Klass
@@ -7823,8 +7823,8 @@ mark_must_check
78237823
sil-instruction ::= 'mark_must_check'
78247824
'[' sil-optimizer-analysis-marker ']'
78257825

7826-
sil-optimizer-analysis-marker ::= 'no_implicit_copy'
7827-
::= 'no_copy'
7826+
sil-optimizer-analysis-marker ::= 'consumable_and_assignable'
7827+
::= 'no_consume_or_assign'
78287828

78297829
A canary value inserted by a SIL generating frontend to signal to the move
78307830
checker to check a specific value. Valid only in Raw SIL. The relevant checkers
@@ -7833,10 +7833,10 @@ relevant diagnostic. The idea here is that instead of needing to introduce
78337833
multiple "flagging" instructions for the optimizer, we can just reuse this one
78347834
instruction by varying the kind.
78357835

7836-
If the sil optimizer analysis marker is ``no_implicit_copy`` then the move
7836+
If the sil optimizer analysis marker is ``consumable_and_assignable`` then the move
78377837
checker is told to check that the result of this instruction is consumed at most
7838-
once. If the marker is ``no_copy``, then the move checker will validate that the
7839-
result of this instruction is never consumed.
7838+
once. If the marker is ``no_consume_or_assign``, then the move checker will
7839+
validate that the result of this instruction is never consumed or assigned over.
78407840

78417841
No Implicit Copy and No Escape Value Instructions
78427842
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

‎include/swift/AST/DiagnosticsSIL.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ NOTE(sil_moveonlychecker_nonconsuming_use_here, none,
767767
NOTE(sil_movekillscopyablevalue_value_cyclic_consumed_in_loop_here, none,
768768
"consuming in loop use here", ())
769769

770-
ERROR(sil_moveonlychecker_not_understand_no_implicit_copy, none,
770+
ERROR(sil_moveonlychecker_not_understand_consumable_and_assignable, none,
771771
"Usage of @noImplicitCopy that the move checker does not know how to "
772772
"check!", ())
773773
ERROR(sil_moveonlychecker_not_understand_moveonly, none,

‎include/swift/SIL/SILInstruction.h

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8208,15 +8208,26 @@ class MarkMustCheckInst
82088208
enum class CheckKind : unsigned {
82098209
Invalid = 0,
82108210

8211-
// A signal to the move only checker to perform no implicit copy checking on
8212-
// the result of this instruction. This implies that the result can be
8213-
// consumed at most once.
8214-
NoImplicitCopy,
8215-
8216-
// A signal to the move only checker ot perform no copy checking. This
8217-
// forces the result of this instruction owned value to never be consumed
8218-
// (still allowing for non-consuming uses of course).
8219-
NoCopy,
8211+
/// A signal to the move only checker to perform checking that allows for
8212+
/// this value to be consumed along its boundary (in the case of let/var
8213+
/// semantics) and also written over in the case of var semantics. NOTE: Of
8214+
/// course this still implies the value cannot be copied and can be consumed
8215+
/// only once along all program paths.
8216+
ConsumableAndAssignable,
8217+
8218+
/// A signal to the move only checker to perform no consume or assign
8219+
/// checking. This forces the result of this instruction owned value to never
8220+
/// be consumed (for let/var semantics) or assigned over (for var
8221+
/// semantics). Of course, we still allow for non-consuming uses.
8222+
NoConsumeOrAssign,
8223+
8224+
/// A signal to the move checker that the given value cannot be consumed,
8225+
/// but is allowed to be assigned over. This is used for situations like
8226+
/// global_addr/ref_element_addr/closure escape where we do not want to
8227+
/// allow for the user to take the value (leaving the memory in an
8228+
/// uninitialized state), but we are ok with the user assigning a new value,
8229+
/// completely assigning over the value at once.
8230+
AssignableButNotConsumable,
82208231
};
82218232

82228233
private:
@@ -8239,8 +8250,9 @@ class MarkMustCheckInst
82398250
switch (kind) {
82408251
case CheckKind::Invalid:
82418252
return false;
8242-
case CheckKind::NoImplicitCopy:
8243-
case CheckKind::NoCopy:
8253+
case CheckKind::ConsumableAndAssignable:
8254+
case CheckKind::NoConsumeOrAssign:
8255+
case CheckKind::AssignableButNotConsumable:
82448256
return true;
82458257
}
82468258
}

‎lib/SIL/IR/SILPrinter.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1992,11 +1992,14 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
19921992
switch (I->getCheckKind()) {
19931993
case CheckKind::Invalid:
19941994
llvm_unreachable("Invalid?!");
1995-
case CheckKind::NoImplicitCopy:
1996-
*this << "[no_implicit_copy] ";
1995+
case CheckKind::ConsumableAndAssignable:
1996+
*this << "[consumable_and_assignable] ";
19971997
break;
1998-
case CheckKind::NoCopy:
1999-
*this << "[no_copy] ";
1998+
case CheckKind::NoConsumeOrAssign:
1999+
*this << "[no_consume_or_assign] ";
2000+
break;
2001+
case CheckKind::AssignableButNotConsumable:
2002+
*this << "[assignable_but_not_consumable] ";
20002003
break;
20012004
}
20022005
*this << getIDAndType(I->getOperand());

‎lib/SIL/Parser/ParseSIL.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3661,10 +3661,13 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
36613661
}
36623662

36633663
using CheckKind = MarkMustCheckInst::CheckKind;
3664-
CheckKind CKind = llvm::StringSwitch<CheckKind>(AttrName)
3665-
.Case("no_implicit_copy", CheckKind::NoImplicitCopy)
3666-
.Case("no_copy", CheckKind::NoCopy)
3667-
.Default(CheckKind::Invalid);
3664+
CheckKind CKind =
3665+
llvm::StringSwitch<CheckKind>(AttrName)
3666+
.Case("consumable_and_assignable",
3667+
CheckKind::ConsumableAndAssignable)
3668+
.Case("no_consume_or_assign", CheckKind::NoConsumeOrAssign)
3669+
.Case("assignable_but_not_consumable", CheckKind::AssignableButNotConsumable)
3670+
.Default(CheckKind::Invalid);
36683671

36693672
if (CKind == CheckKind::Invalid) {
36703673
auto diag = diag::sil_markmustcheck_invalid_attribute;

‎lib/SILGen/SILGenConstructor.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,8 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
839839
if (selfArg.getType().isMoveOnly()) {
840840
assert(selfArg.getOwnershipKind() == OwnershipKind::Owned);
841841
selfArg = B.createMarkMustCheckInst(
842-
selfDecl, selfArg, MarkMustCheckInst::CheckKind::NoImplicitCopy);
842+
selfDecl, selfArg,
843+
MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
843844
}
844845
VarLocs[selfDecl] = VarLoc::get(selfArg.getValue());
845846
}

‎lib/SILGen/SILGenDecl.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ class LocalVariableInitialization : public SingleBufferInitialization {
380380
if (Addr->getType().isMoveOnly()) {
381381
// TODO: Handle no implicit copy here.
382382
Addr = SGF.B.createMarkMustCheckInst(
383-
decl, Addr, MarkMustCheckInst::CheckKind::NoImplicitCopy);
383+
decl, Addr, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
384384
}
385385

386386
// Push a cleanup to destroy the local variable. This has to be
@@ -580,7 +580,8 @@ class LetValueInitialization : public Initialization {
580580
if (value->getType().isPureMoveOnly()) {
581581
value = SGF.B.createMoveValue(PrologueLoc, value, /*isLexical*/ true);
582582
return SGF.B.createMarkMustCheckInst(
583-
PrologueLoc, value, MarkMustCheckInst::CheckKind::NoImplicitCopy);
583+
PrologueLoc, value,
584+
MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
584585
}
585586

586587
// Otherwise, if we don't have a no implicit copy trivial type, just
@@ -594,7 +595,8 @@ class LetValueInitialization : public Initialization {
594595
SGF.B.createOwnedCopyableToMoveOnlyWrapperValue(PrologueLoc, value);
595596
value = SGF.B.createMoveValue(PrologueLoc, value, /*isLexical*/ true);
596597
return SGF.B.createMarkMustCheckInst(
597-
PrologueLoc, value, MarkMustCheckInst::CheckKind::NoImplicitCopy);
598+
PrologueLoc, value,
599+
MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
598600
}
599601

600602
// Then if we don't have move only, just perform a lexical borrow if the
@@ -636,7 +638,8 @@ class LetValueInitialization : public Initialization {
636638
!value->getType().isMoveOnlyWrapped()) {
637639
value = SGF.B.createMoveValue(PrologueLoc, value, true /*isLexical*/);
638640
return SGF.B.createMarkMustCheckInst(
639-
PrologueLoc, value, MarkMustCheckInst::CheckKind::NoImplicitCopy);
641+
PrologueLoc, value,
642+
MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
640643
}
641644

642645
// Otherwise, if we do not have a no implicit copy variable, just follow
@@ -655,7 +658,8 @@ class LetValueInitialization : public Initialization {
655658
value = SGF.B.createCopyValue(PrologueLoc, value);
656659
value = SGF.B.createOwnedCopyableToMoveOnlyWrapperValue(PrologueLoc, value);
657660
return SGF.B.createMarkMustCheckInst(
658-
PrologueLoc, value, MarkMustCheckInst::CheckKind::NoImplicitCopy);
661+
PrologueLoc, value,
662+
MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
659663
}
660664

661665
void bindValue(SILValue value, SILGenFunction &SGF, bool wasPlusOne) {

‎lib/SILGen/SILGenProlog.cpp

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ SILValue SILGenFunction::emitSelfDeclForDestructor(VarDecl *selfDecl) {
4949
// they cannot escape.
5050
if (selfValue->getOwnershipKind() == OwnershipKind::Owned) {
5151
selfValue = B.createMarkMustCheckInst(
52-
selfDecl, selfValue, MarkMustCheckInst::CheckKind::NoImplicitCopy);
52+
selfDecl, selfValue,
53+
MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
5354
}
5455
}
5556

@@ -129,7 +130,7 @@ class EmitBBArguments : public CanTypeVisitor<EmitBBArguments,
129130
// check ownership.
130131
if (mv.getType().isMoveOnly() && !mv.getType().isMoveOnlyWrapped())
131132
mv = SGF.B.createMarkMustCheckInst(
132-
loc, mv, MarkMustCheckInst::CheckKind::NoImplicitCopy);
133+
loc, mv, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
133134
return mv;
134135
}
135136

@@ -322,15 +323,15 @@ struct ArgumentInitHelper {
322323
value = SGF.B.createMoveValue(loc, argrv.forward(SGF),
323324
/*isLexical*/ true);
324325
value = SGF.B.createMarkMustCheckInst(
325-
loc, value, MarkMustCheckInst::CheckKind::NoImplicitCopy);
326+
loc, value, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
326327
SGF.emitManagedRValueWithCleanup(value);
327328
return value;
328329
}
329330

330331
assert(value->getOwnershipKind() == OwnershipKind::Guaranteed);
331332
value = SGF.B.createCopyValue(loc, value);
332333
value = SGF.B.createMarkMustCheckInst(
333-
loc, value, MarkMustCheckInst::CheckKind::NoCopy);
334+
loc, value, MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
334335
SGF.emitManagedRValueWithCleanup(value);
335336
return value;
336337
}
@@ -341,9 +342,9 @@ struct ArgumentInitHelper {
341342

342343
// If our argument was owned, we use no implicit copy. Otherwise, we
343344
// use no copy.
344-
auto kind = MarkMustCheckInst::CheckKind::NoCopy;
345+
auto kind = MarkMustCheckInst::CheckKind::NoConsumeOrAssign;
345346
if (pd->isOwned())
346-
kind = MarkMustCheckInst::CheckKind::NoImplicitCopy;
347+
kind = MarkMustCheckInst::CheckKind::ConsumableAndAssignable;
347348
value = SGF.B.createMarkMustCheckInst(loc, value, kind);
348349
SGF.emitManagedRValueWithCleanup(value);
349350
return value;
@@ -353,7 +354,7 @@ struct ArgumentInitHelper {
353354
value = SGF.B.createGuaranteedCopyableToMoveOnlyWrapperValue(loc, value);
354355
value = SGF.B.createCopyValue(loc, value);
355356
value = SGF.B.createMarkMustCheckInst(
356-
loc, value, MarkMustCheckInst::CheckKind::NoCopy);
357+
loc, value, MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
357358
SGF.emitManagedRValueWithCleanup(value);
358359
return value;
359360
}
@@ -365,7 +366,7 @@ struct ArgumentInitHelper {
365366
loc, argrv.forward(SGF));
366367
value = SGF.B.createMoveValue(loc, value, true /*is lexical*/);
367368
value = SGF.B.createMarkMustCheckInst(
368-
loc, value, MarkMustCheckInst::CheckKind::NoImplicitCopy);
369+
loc, value, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
369370
SGF.emitManagedRValueWithCleanup(value);
370371
return value;
371372
}
@@ -547,14 +548,14 @@ static void emitCaptureArguments(SILGenFunction &SGF,
547548
val = addr->getManagedAddress();
548549
}
549550

550-
// If this constant is a move only type, we need to add no_copy checking to
551+
// If this constant is a move only type, we need to add no_consume_or_assign checking to
551552
// ensure that we do not consume this captured value in the function. This
552553
// is because closures can be invoked multiple times which is inconsistent
553554
// with consuming the move only type.
554555
if (val.getType().isMoveOnly()) {
555556
val = val.ensurePlusOne(SGF, Loc);
556-
val = SGF.B.createMarkMustCheckInst(Loc, val,
557-
MarkMustCheckInst::CheckKind::NoCopy);
557+
val = SGF.B.createMarkMustCheckInst(
558+
Loc, val, MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
558559
}
559560

560561
SGF.VarLocs[VD] = SILGenFunction::VarLoc::get(val.getValue());
@@ -585,7 +586,7 @@ static void emitCaptureArguments(SILGenFunction &SGF,
585586
SILValue addr = SGF.B.createProjectBox(VD, box, 0);
586587
if (addr->getType().isMoveOnly())
587588
addr = SGF.B.createMarkMustCheckInst(
588-
VD, addr, MarkMustCheckInst::CheckKind::NoImplicitCopy);
589+
VD, addr, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
589590
SGF.VarLocs[VD] = SILGenFunction::VarLoc::get(addr, box);
590591
SILDebugVariable DbgVar(VD->isLet(), ArgNo);
591592
SGF.B.createDebugValueAddr(Loc, addr, DbgVar);
@@ -608,7 +609,7 @@ static void emitCaptureArguments(SILGenFunction &SGF,
608609
SILValue arg = SILValue(fArg);
609610
if (isInOut && (ty.isMoveOnly() && !ty.isMoveOnlyWrapped())) {
610611
arg = SGF.B.createMarkMustCheckInst(
611-
Loc, arg, MarkMustCheckInst::CheckKind::NoImplicitCopy);
612+
Loc, arg, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
612613
}
613614
SGF.VarLocs[VD] = SILGenFunction::VarLoc::get(arg);
614615
SILDebugVariable DbgVar(VD->isLet(), ArgNo);

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressChecker.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,7 +1081,8 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
10811081
assert(op->getOperandNumber() == CopyAddrInst::Src &&
10821082
"Should have dest above in memInstMust{Rei,I}nitialize");
10831083

1084-
if (markedValue->getCheckKind() == MarkMustCheckInst::CheckKind::NoCopy) {
1084+
if (markedValue->getCheckKind() ==
1085+
MarkMustCheckInst::CheckKind::NoConsumeOrAssign) {
10851086
LLVM_DEBUG(llvm::dbgs()
10861087
<< "Found mark must check [nocopy] error: " << *user);
10871088
diagnosticEmitter.emitAddressDiagnosticNoCopy(markedValue, copyAddr);
@@ -1159,7 +1160,8 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
11591160
// If we are asked to perform guaranteed checking, emit an error if we
11601161
// have /any/ consuming uses. This is a case that can always be converted
11611162
// to a load_borrow if we pass the check.
1162-
if (markedValue->getCheckKind() == MarkMustCheckInst::CheckKind::NoCopy) {
1163+
if (markedValue->getCheckKind() ==
1164+
MarkMustCheckInst::CheckKind::NoConsumeOrAssign) {
11631165
if (!moveChecker.canonicalizer.foundAnyConsumingUses()) {
11641166
LLVM_DEBUG(llvm::dbgs()
11651167
<< "Found mark must check [nocopy] error: " << *user);

‎lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,9 @@ void DiagnosticEmitter::emitCheckerDoesntUnderstandDiagnostic(
9595
// that copy propagation did not understand. Emit a we did not understand
9696
// error.
9797
if (markedValue->getType().isMoveOnlyWrapped()) {
98-
diagnose(fn->getASTContext(), markedValue,
99-
diag::sil_moveonlychecker_not_understand_no_implicit_copy);
98+
diagnose(
99+
fn->getASTContext(), markedValue,
100+
diag::sil_moveonlychecker_not_understand_consumable_and_assignable);
100101
} else {
101102
diagnose(fn->getASTContext(), markedValue,
102103
diag::sil_moveonlychecker_not_understand_moveonly);

0 commit comments

Comments
 (0)