Skip to content

Commit f1182a7

Browse files
committed
[no-implicit-copy] Remove auto +1 param signature change called by noimplicit copy in favor of following normal convention.
I also added a bunch of tests for both the trivial/non-trivial case as well as some docs to SIL.rst.
1 parent 9b62558 commit f1182a7

21 files changed

+3172
-207
lines changed

docs/SIL.rst

Lines changed: 199 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2570,6 +2570,198 @@ Consider the following legal SIL where we leak ``%0`` in blocks prefixed with
25702570
return %1 : $Klass
25712571
}
25722572

2573+
Move Only Wrapped Types
2574+
-----------------------
2575+
2576+
Semantics
2577+
~~~~~~~~~
2578+
2579+
Today SIL supports values of move only wrapped type. Given a copyable type
2580+
``$T``, the type ``$@moveOnly T`` is the move only wrapped variant of the
2581+
type. This type is always non-trivial even if ``$T`` itself is trivial (see
2582+
discussion below). All move only wrapped types obey the invariant that they can
2583+
be copied in Raw SIL but cannot be copied in Canonical SIL and later SIL
2584+
stages. This ensures that once we are in Canonical SIL such values are "move
2585+
only values" that are guaranteed to never be copied. This is enforced by:
2586+
2587+
* Having SILGen emit copies as it normally does.
2588+
2589+
* Use OSSA canonicalization to eliminate copies that aren't needed semantically
2590+
due to consuming uses of the value. This is implemented by the pass guaranteed
2591+
pass "MoveOnlyChecker". Emit errors on any of the consuming uses that we found
2592+
in said pass.
2593+
2594+
Assuming that no errors are emitted, we can then conclude before we reach
2595+
canonical SIL that the value was never copied and thus is a "move only value"
2596+
even though the actual underlying wrapped type is copyable. As an example of
2597+
this, consider the following Swift::
2598+
2599+
func doSomething(@_noImplicitCopy _ x: Klass) -> () { // expected-error {{'x' has guaranteed ownership but was consumed}}
2600+
x.doSomething()
2601+
let x2 = x // expected-note {{consuming use}}
2602+
x2.doSomething()
2603+
}
2604+
2605+
which codegens to the following SIL::
2606+
2607+
sil hidden [ossa] @doSomething : $@convention(thin) (@guaranteed Klass) -> () {
2608+
bb0(%0 : @noImplicitCopy $Klass):
2609+
%1 = copyable_to_moveonlywrapper [guaranteed] %0 : $@moveOnly Klass
2610+
%2 = copy_value %1 : $@moveOnly Klass
2611+
%3 = mark_must_check [no_copy] %2 : $@moveOnly Klass
2612+
debug_value %3 : $@moveOnly Klass, let, name "x", argno 1
2613+
%4 = begin_borrow %3 : $@moveOnly Klass
2614+
%5 = function_ref @$s4test5KlassC11doSomethingyyF : $@convention(method) (@guaranteed Klass) -> ()
2615+
%6 = moveonlywrapper_to_copyable [guaranteed] %4 : $@moveOnly Klass
2616+
%7 = apply %5(%6) : $@convention(method) (@guaranteed Klass) -> ()
2617+
end_borrow %4 : $@moveOnly Klass
2618+
%9 = begin_borrow %3 : $@moveOnly Klass
2619+
%10 = copy_value %9 : $@moveOnly Klass
2620+
%11 = moveonlywrapper_to_copyable [owned] %10 : $@moveOnly Klass
2621+
%12 = begin_borrow [lexical] %11 : $Klass
2622+
debug_value %12 : $Klass, let, name "x2"
2623+
end_borrow %9 : $@moveOnly Klass
2624+
%15 = function_ref @$s4test5KlassC11doSomethingyyF : $@convention(method) (@guaranteed Klass) -> ()
2625+
%16 = apply %15(%12) : $@convention(method) (@guaranteed Klass) -> ()
2626+
end_borrow %12 : $Klass
2627+
destroy_value %11 : $Klass
2628+
destroy_value %3 : $@moveOnly Klass
2629+
%20 = tuple ()
2630+
return %20 : $()
2631+
} // end sil function '$s4test11doSomethingyyAA5KlassCF'
2632+
2633+
When the move only checker runs upon this SIL, it will see that the
2634+
``moveonlywrapped_to_copyable [owned]`` is a transitive consuming use of ``%2``
2635+
(ignoring copies) and that we have a non-consuming use later. So it will emit an
2636+
error that we have a guaranteed parameter that is being consumed. If we remove
2637+
the assignment to ``x2``, then after the move checker runs successfully we would
2638+
get the following SIL::
2639+
2640+
sil hidden [ossa] @doSomething : $@convention(thin) (@guaranteed Klass) -> () {
2641+
bb0(%0 : @noImplicitCopy $Klass):
2642+
%1 = copyable_to_moveonlywrapper [guaranteed] %0 : $Klass
2643+
debug_value %1 : $@moveOnly Klass, let, name "x", argno 1
2644+
%4 = begin_borrow %1 : $@moveOnly Klass
2645+
%5 = function_ref @$s4test5KlassC11doSomethingyyF : $@convention(method) (@guaranteed Klass) -> ()
2646+
%6 = moveonlywrapper_to_copyable [guaranteed] %4 : $@moveOnly Klass
2647+
%7 = apply %5(%6) : $@convention(method) (@guaranteed Klass) -> ()
2648+
end_borrow %4 : $@moveOnly Klass
2649+
%20 = tuple ()
2650+
return %20 : $()
2651+
} // end sil function '$s4test11doSomethingyyAA5KlassCF'
2652+
2653+
yielding SIL without any copies just as we wanted.
2654+
2655+
At the ABI level, ``@moveOnly`` does not exist and thus never shows up in a
2656+
SILFunctionType's parameters. Instead, we represent it only in the body of
2657+
functions and on a function's internal SILArgument representation. This is since
2658+
move only wrapped is intended to be a local power tool for controlling lifetimes
2659+
rather than a viral type level annotation that would constrain the type system.
2660+
2661+
As mentioned above trivial move only wrapped types are actually
2662+
non-trivial. This is because in SIL ownership is tied directly to
2663+
non-trivialness so unless we did that we could not track ownership
2664+
accurately. This is loss of triviality is not an issue for most of the pipeline
2665+
since we eliminate all move only wrapper types for trivial types during the
2666+
guaranteed optimizations after we have run various ownership checkers but before
2667+
we have run diagnostics for trivial types (e.x.: DiagnosticConstantPropagation).
2668+
2669+
As an example in practice, consider the following Swift::
2670+
2671+
func doSomethingWithInt(@_noImplicitCopy _ x: Int) -> Int {
2672+
x + x
2673+
}
2674+
2675+
Today this codegens to the following Swift::
2676+
2677+
sil hidden [ossa] @doSomethingWithInt : $@convention(thin) (Int) -> Int {
2678+
bb0(%0 : @noImplicitCopy $Int):
2679+
%1 = copyable_to_moveonlywrapper [owned] %0 : $Int
2680+
%2 = move_value [lexical] %1 : $@moveOnly Int
2681+
%3 = mark_must_check [no_implicit_copy] %2 : $@moveOnly Int
2682+
%5 = begin_borrow %3 : $@moveOnly Int
2683+
%6 = begin_borrow %3 : $@moveOnly Int
2684+
%7 = function_ref @addIntegers : $@convention(method) (Int, Int Int.Type) -> Int
2685+
%8 = moveonlywrapper_to_copyable [guaranteed] %5 : $@moveOnly Int
2686+
%9 = moveonlywrapper_to_copyable [guaranteed] %6 : $@moveOnly Int
2687+
%10 = apply %7(%8, %9) : $@convention(method) (Int, Int) -> Int
2688+
end_borrow %6 : $@moveOnly Int
2689+
end_borrow %5 : $@moveOnly Int
2690+
destroy_value %3 : $@moveOnly Int
2691+
return %10 : $Int
2692+
}
2693+
2694+
once the checker has run, this becomes::
2695+
2696+
sil hidden [ossa] @doSomethingWithInt : $@convention(thin) (Int) -> Int {
2697+
bb0(%0 : @noImplicitCopy $Int):
2698+
%1 = copyable_to_moveonlywrapper [owned] %0 : $Int
2699+
%2 = move_value [lexical] %1 : $@moveOnly Int
2700+
%5 = begin_borrow %2 : $@moveOnly Int
2701+
%6 = begin_borrow %2 : $@moveOnly Int
2702+
%7 = function_ref @addIntegers : $@convention(method) (Int, Int Int.Type) -> Int
2703+
%8 = moveonlywrapper_to_copyable [guaranteed] %5 : $@moveOnly Int
2704+
%9 = moveonlywrapper_to_copyable [guaranteed] %6 : $@moveOnly Int
2705+
%10 = apply %7(%8, %9) : $@convention(method) (Int, Int) -> Int
2706+
end_borrow %6 : $@moveOnly Int
2707+
end_borrow %5 : $@moveOnly Int
2708+
return %10 : $Int
2709+
}
2710+
2711+
and once we have run the move only wrapped type lowerer, we get the following
2712+
SIL::
2713+
2714+
sil hidden [ossa] @doSomethingWithInt : $@convention(thin) (Int) -> Int {
2715+
bb0(%0 : @noImplicitCopy $Int):
2716+
%1 = function_ref @addIntegers : $@convention(method) (Int, Int) -> Int
2717+
%2 = apply %1(%0, %0) : $@convention(method) (Int, Int) -> Int
2718+
return %2 : $Int
2719+
}
2720+
2721+
exactly what we wanted in the end.
2722+
2723+
If we are given an owned argument or a let binding, we use a similar
2724+
approach. Consider the following Swift::
2725+
2726+
func doSomethingWithKlass(_ x: Klass) -> Klass {
2727+
@_noImplicitCopy let value = x
2728+
let value2 = value
2729+
return value
2730+
}
2731+
2732+
A hypothetical SILGen for this code is as follows::
2733+
2734+
sil hidden [ossa] @$s4test20doSomethingWithKlassyAA0E0CADF : $@convention(thin) (@guaranteed Klass) -> @owned Klass {
2735+
bb0(%0 : @guaranteed $Klass):
2736+
debug_value %0 : $Klass, let, name "x", argno 1
2737+
%3 = begin_borrow [lexical] %0 : $Klass
2738+
%4 = copy_value %3 : $Klass
2739+
%5 = copyable_to_moveonlywrapper [owned] %4 : $Klass
2740+
%6 = mark_must_check [no_implicit_copy] %5 : $@moveOnly Klass
2741+
debug_value %6 : $@moveOnly Klass, let, name "value"
2742+
%8 = begin_borrow %6 : $@moveOnly Klass
2743+
%9 = copy_value %8 : $@moveOnly Klass
2744+
%10 = moveonlywrapper_to_copyable [owned] %9 : $@moveOnly Klass
2745+
%11 = begin_borrow [lexical] %10 : $Klass
2746+
debug_value %11 : $Klass, let, name "value2"
2747+
end_borrow %8 : $@moveOnly Klass
2748+
%14 = begin_borrow %6 : $@moveOnly Klass
2749+
%15 = copy_value %14 : $@moveOnly Klass
2750+
end_borrow %14 : $@moveOnly Klass
2751+
end_borrow %11 : $Klass
2752+
destroy_value %10 : $Klass
2753+
destroy_value %6 : $@moveOnly Klass
2754+
end_borrow %3 : $Klass
2755+
%22 = moveonlywrapper_to_copyable [owned] %15 : $@moveOnly Klass
2756+
return %22 : $Klass
2757+
} // end sil function '$s4test20doSomethingWithKlassyAA0E0CADF'
2758+
2759+
Notice above how the ``moveonlywrapper_to_copyable [owned]`` is used to escape
2760+
the no implicit copy value. In fact, if one runs the following SILGen through
2761+
sil-opt, one will see that we actually have an ownership violation due to the
2762+
two uses of "value", one for initializing value2 and the other for the return
2763+
value.
2764+
25732765
Runtime Failure
25742766
---------------
25752767

@@ -7456,14 +7648,20 @@ mark_must_check
74567648
'[' sil-optimizer-analysis-marker ']'
74577649

74587650
sil-optimizer-analysis-marker ::= 'no_implicit_copy'
7651+
::= 'no_copy'
74597652

74607653
A canary value inserted by a SIL generating frontend to signal to the move
74617654
checker to check a specific value. Valid only in Raw SIL. The relevant checkers
74627655
should remove the `mark_must_check`_ instruction after successfully running the
74637656
relevant diagnostic. The idea here is that instead of needing to introduce
7464-
multiple "flaging" instructions for the optimizer, we can just reuse this one
7657+
multiple "flagging" instructions for the optimizer, we can just reuse this one
74657658
instruction by varying the kind.
74667659

7660+
If the sil optimizer analysis marker is ``no_implicit_copy`` then the move
7661+
checker is told to check that the result of this instruction is consumed at most
7662+
once. If the marker is ``no_copy``, then the move checker will validate that the
7663+
result of this instruction is never consumed.
7664+
74677665
No Implicit Copy and No Escape Value Instructions
74687666
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
74697667

include/swift/AST/DiagnosticsSIL.def

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -725,8 +725,10 @@ ERROR(noimplicitcopy_used_on_generic_or_existential, none,
725725
"binding or a nominal type containing such typed things", ())
726726

727727
// move only checker diagnostics
728-
ERROR(sil_moveonlychecker_value_consumed_more_than_once, none,
728+
ERROR(sil_moveonlychecker_owned_value_consumed_more_than_once, none,
729729
"'%0' consumed more than once", (StringRef))
730+
ERROR(sil_moveonlychecker_guaranteed_value_consumed, none,
731+
"'%0' has guaranteed ownership but was consumed", (StringRef))
730732
NOTE(sil_moveonlychecker_consuming_use_here, none,
731733
"consuming use", ())
732734
ERROR(sil_moveonlychecker_not_understand_mark_move, none,

include/swift/SIL/SILBuilder.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,6 +1300,8 @@ class SILBuilder {
13001300
CopyableToMoveOnlyWrapperValueInst *
13011301
createGuaranteedCopyableToMoveOnlyWrapperValue(SILLocation loc,
13021302
SILValue src) {
1303+
assert(!src->getType().isTrivial(*F) &&
1304+
"trivial types can only use the owned version of this API");
13031305
return insert(new (getModule()) CopyableToMoveOnlyWrapperValueInst(
13041306
getSILDebugLocation(loc), src,
13051307
CopyableToMoveOnlyWrapperValueInst::Guaranteed));

include/swift/SIL/SILInstruction.h

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7570,7 +7570,16 @@ class MarkMustCheckInst
75707570
public:
75717571
enum class CheckKind : unsigned {
75727572
Invalid = 0,
7573+
7574+
// A signal to the move only checker to perform no implicit copy checking on
7575+
// the result of this instruction. This implies that the result can be
7576+
// consumed at most once.
75737577
NoImplicitCopy,
7578+
7579+
// A signal to the move only checker ot perform no copy checking. This
7580+
// forces the result of this instruction owned value to never be consumed
7581+
// (still allowing for non-consuming uses of course).
7582+
NoCopy,
75747583
};
75757584

75767585
private:
@@ -7589,7 +7598,15 @@ class MarkMustCheckInst
75897598
public:
75907599
CheckKind getCheckKind() const { return kind; }
75917600

7592-
bool isNoImplicitCopy() const { return kind == CheckKind::NoImplicitCopy; }
7601+
bool hasMoveCheckerKind() const {
7602+
switch (kind) {
7603+
case CheckKind::Invalid:
7604+
return false;
7605+
case CheckKind::NoImplicitCopy:
7606+
case CheckKind::NoCopy:
7607+
return true;
7608+
}
7609+
}
75937610
};
75947611

75957612
/// Convert from a non-trivial copyable type to an `@moveOnly` wrapper type.
@@ -7631,7 +7648,8 @@ class CopyableToMoveOnlyWrapperValueInst
76317648
: UnaryInstructionBase(
76327649
DebugLoc, operand, operand->getType().addingMoveOnlyWrapper(),
76337650
kind == InitialKind::Guaranteed ? OwnershipKind::Guaranteed
7634-
: OwnershipKind::Owned) {}
7651+
: OwnershipKind::Owned),
7652+
initialKind(kind) {}
76357653

76367654
public:
76377655
InitialKind getInitialKind() const { return initialKind; }

lib/SIL/IR/SILPrinter.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1910,6 +1910,9 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
19101910
case CheckKind::NoImplicitCopy:
19111911
*this << "[no_implicit_copy] ";
19121912
break;
1913+
case CheckKind::NoCopy:
1914+
*this << "[no_copy] ";
1915+
break;
19131916
}
19141917
*this << getIDAndType(I->getOperand());
19151918
}

lib/SIL/Parser/ParseSIL.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3398,6 +3398,7 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
33983398
using CheckKind = MarkMustCheckInst::CheckKind;
33993399
CheckKind CKind = llvm::StringSwitch<CheckKind>(AttrName)
34003400
.Case("no_implicit_copy", CheckKind::NoImplicitCopy)
3401+
.Case("no_copy", CheckKind::NoCopy)
34013402
.Default(CheckKind::Invalid);
34023403

34033404
if (CKind == CheckKind::Invalid) {

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5398,7 +5398,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
53985398

53995399
void checkCopyableToMoveOnlyWrapperValueInst(
54005400
CopyableToMoveOnlyWrapperValueInst *cvt) {
5401-
require(!cvt->getOperand()->getType().isTrivial(*cvt->getFunction()),
5401+
require(cvt->getInitialKind() ==
5402+
CopyableToMoveOnlyWrapperValueInst::Owned ||
5403+
!cvt->getOperand()->getType().isTrivial(*cvt->getFunction()),
54025404
"To convert from a trivial value to a move only wrapper value use "
54035405
"TrivialToGuaranteedMoveOnlyWrapperValueInst");
54045406
require(cvt->getOperand()->getType().isObject(),

lib/SILGen/SILGenApply.cpp

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1811,9 +1811,13 @@ static void emitRawApply(SILGenFunction &SGF,
18111811
// Gather the arguments.
18121812
for (auto i : indices(args)) {
18131813
SILValue argValue;
1814+
auto inputTy =
1815+
substFnConv.getSILType(inputParams[i], SGF.getTypeExpansionContext());
1816+
18141817
if (inputParams[i].isConsumed()) {
18151818
argValue = args[i].forward(SGF);
1816-
if (argValue->getType().isMoveOnlyWrapped()) {
1819+
if (argValue->getType().isMoveOnlyWrapped() &&
1820+
!inputTy.isMoveOnlyWrapped()) {
18171821
argValue =
18181822
SGF.B.createOwnedMoveOnlyWrapperToCopyableValue(loc, argValue);
18191823
}
@@ -1826,26 +1830,30 @@ static void emitRawApply(SILGenFunction &SGF,
18261830
// will error. At this point we just want to make sure that the emitted
18271831
// types line up.
18281832
if (arg.getType().isMoveOnlyWrapped()) {
1829-
// We need to borrow so that we can convert from $@moveOnly T -> $T. Use
1830-
// a formal access borrow to ensure that we have tight scopes like we do
1831-
// when we borrow fn.
1832-
if (!arg.isPlusZero())
1833-
arg = arg.formalAccessBorrow(SGF, loc);
1834-
arg = SGF.B.createGuaranteedMoveOnlyWrapperToCopyableValue(loc, arg);
1833+
if (!inputTy.isMoveOnlyWrapped()) {
1834+
// We need to borrow so that we can convert from $@moveOnly T -> $T.
1835+
// Use a formal access borrow to ensure that we have tight scopes like
1836+
// we do when we borrow fn.
1837+
if (!arg.isPlusZero())
1838+
arg = arg.formalAccessBorrow(SGF, loc);
1839+
1840+
arg = SGF.B.createGuaranteedMoveOnlyWrapperToCopyableValue(loc, arg);
1841+
}
18351842
}
18361843

18371844
argValue = arg.getValue();
18381845
}
18391846

18401847
#ifndef NDEBUG
1841-
auto inputTy =
1842-
substFnConv.getSILType(inputParams[i], SGF.getTypeExpansionContext());
18431848
if (argValue->getType() != inputTy) {
18441849
auto &out = llvm::errs();
18451850
out << "TYPE MISMATCH IN ARGUMENT " << i << " OF APPLY AT ";
18461851
printSILLocationDescription(out, loc, SGF.getASTContext());
18471852
out << " argument value: ";
18481853
argValue->print(out);
1854+
out << " argument type: ";
1855+
argValue->getType().print(out);
1856+
out << "\n";
18491857
out << " parameter type: ";
18501858
inputTy.print(out);
18511859
out << "\n";

0 commit comments

Comments
 (0)