Skip to content

Commit 0a16cc3

Browse files
committed
[move-only] Implement an initial version of the move only address checker.
Some notes: 1. I added support for both loadable/address only types. 2. These tests are based off of porting the move only object tests for inout, vars, mutating self, etc. 3. I did not include already written tests for address only types in this specific merge since I need to change us to borrow move only var like types. Without that, we get a lot of spurious error msgs and the burden of writing that is not worth it. So instead in a forthcoming commit where I fix that issue in SILGen, I will commit the corresponding address only tests for this work. 4. I did not include support for trivial types in this. I am going to do object/address for that at the same time.
1 parent 0583457 commit 0a16cc3

File tree

13 files changed

+4067
-19
lines changed

13 files changed

+4067
-19
lines changed

docs/SIL.rst

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2593,9 +2593,9 @@ only values" that are guaranteed to never be copied. This is enforced by:
25932593
* Having SILGen emit copies as it normally does.
25942594

25952595
* Use OSSA canonicalization to eliminate copies that aren't needed semantically
2596-
due to consuming uses of the value. This is implemented by the pass guaranteed
2597-
pass "MoveOnlyChecker". Emit errors on any of the consuming uses that we found
2598-
in said pass.
2596+
due to consuming uses of the value. This is implemented by the guaranteed
2597+
passes "MoveOnlyObjectChecker" and "MoveOnlyAddressChecker". We will emit
2598+
errors on any of the consuming uses that we found in said pass.
25992599

26002600
Assuming that no errors are emitted, we can then conclude before we reach
26012601
canonical SIL that the value was never copied and thus is a "move only value"
@@ -2667,7 +2667,7 @@ rather than a viral type level annotation that would constrain the type system.
26672667
As mentioned above trivial move only wrapped types are actually
26682668
non-trivial. This is because in SIL ownership is tied directly to
26692669
non-trivialness so unless we did that we could not track ownership
2670-
accurately. This is loss of triviality is not an issue for most of the pipeline
2670+
accurately. This loss of triviality is not an issue for most of the pipeline
26712671
since we eliminate all move only wrapper types for trivial types during the
26722672
guaranteed optimizations after we have run various ownership checkers but before
26732673
we have run diagnostics for trivial types (e.x.: DiagnosticConstantPropagation).

include/swift/AST/DiagnosticsSIL.def

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -727,13 +727,25 @@ ERROR(noimplicitcopy_used_on_generic_or_existential, none,
727727
// move only checker diagnostics
728728
ERROR(sil_moveonlychecker_owned_value_consumed_more_than_once, none,
729729
"'%0' consumed more than once", (StringRef))
730+
ERROR(sil_moveonlychecker_value_used_after_consume, none,
731+
"'%0' used after consume. Lifetime extension of variable requires a copy", (StringRef))
730732
ERROR(sil_moveonlychecker_guaranteed_value_consumed, none,
731733
"'%0' has guaranteed ownership but was consumed", (StringRef))
734+
ERROR(sil_moveonlychecker_inout_not_reinitialized_before_end_of_function, none,
735+
"'%0' consumed but not reinitialized before end of function", (StringRef))
736+
ERROR(sil_moveonlychecker_value_consumed_in_a_loop, none,
737+
"'%0' consumed by a use in a loop", (StringRef))
738+
732739
NOTE(sil_moveonlychecker_consuming_use_here, none,
733740
"consuming use", ())
734-
ERROR(sil_moveonlychecker_not_understand_mark_move, none,
741+
NOTE(sil_moveonlychecker_nonconsuming_use_here, none,
742+
"non-consuming use", ())
743+
ERROR(sil_moveonlychecker_not_understand_no_implicit_copy, none,
735744
"Usage of @noImplicitCopy that the move checker does not know how to "
736745
"check!", ())
746+
ERROR(sil_moveonlychecker_not_understand_moveonly, none,
747+
"Usage of a move only type that the move checker does not know how to "
748+
"check!", ())
737749

738750
// move kills copyable values checker diagnostics
739751
ERROR(sil_movekillscopyablevalue_value_consumed_more_than_once, none,

include/swift/SIL/PrunedLiveness.h

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,9 +556,141 @@ struct PrunedLivenessBoundary {
556556
ArrayRef<SILBasicBlock *> postDomBlocks);
557557
};
558558

559+
//===----------------------------------------------------------------------===//
560+
// Field Sensitive Pruned Liveness
561+
//===----------------------------------------------------------------------===//
562+
559563
/// Given a type T and a descendent field F in T's type tree, then the
560564
/// sub-element number of F is the first leaf element of the type tree in its
561565
/// linearized representation.
566+
///
567+
/// Linearized Representation of Structs/Tuples
568+
/// -------------------------------------------
569+
///
570+
/// For structs/tuples, the linearized representation is just an array with one
571+
/// element for each leaf element. Thus if we have a struct of the following
572+
/// sort:
573+
///
574+
/// ```
575+
/// struct Pair {
576+
/// var lhs: Int
577+
/// var rhs: Int
578+
/// }
579+
///
580+
/// struct MyStruct {
581+
/// var firstField: Int
582+
/// var pairField: Pair
583+
/// var tupleField: (Int, Int)
584+
/// }
585+
/// ```
586+
///
587+
/// the linearized representation of MyStruct's type tree leaves would be:
588+
///
589+
/// ```
590+
/// [firstField, pairField.lhs, pairField.rhs, tupleField.0, tupleField.1]
591+
/// ```
592+
///
593+
/// So if one had an uninitialized myStruct and initialized pairField, one would
594+
/// get the following bit set of liveness:
595+
///
596+
/// ```
597+
/// [0, 1, 1, 0, 0]
598+
/// ```
599+
///
600+
/// Linearized Representation of Enums
601+
/// ----------------------------------
602+
///
603+
/// Since enums are sum types, an enum can require different numbers of bits in
604+
/// its linearized representation depending on the payload of the case that the
605+
/// enum is initialized to. To work around this problem in our representation,
606+
/// we always store enough bits for the max sized payload of all cases of the
607+
/// enum and add an additional last bit for the discriminator. Any extra bits
608+
/// that may be needed (e.x.: we are processing a enum case with a smaller
609+
/// payload) are always assumed to be set to the same value that the
610+
/// discriminator bit is set to. This representation allows us to track liveness
611+
/// trading off the ability to determine information about the actual case that
612+
/// we are tracking. Since we just care about liveness, this is a trade off that
613+
/// we are willing to make since our goal (computing liveness) is still solved.
614+
///
615+
/// With the above paragraph in mind, an example of the bit layout of an enum
616+
/// looks as follows:
617+
///
618+
/// ```
619+
/// [ PAYLOAD BITS | EXTRA_TOP_LEVEL_BITS | DISCRIMINATOR_BIT ]
620+
/// ```
621+
///
622+
/// Notice how placing the discriminator bit last ensures that separately the
623+
/// payload and the extra top level bits/discriminator bit are both contiguous
624+
/// in the representation. This ensures that we can test both that the payload
625+
/// is live and separately that the discriminator/extra top level bits are live
626+
/// with a single contiguous range of bits. This is important since field
627+
/// sensitive liveness can only compute liveness for contiguous ranges of bits.
628+
///
629+
/// Lets look at some examples, starting with E:
630+
///
631+
/// ```
632+
/// enum E {
633+
/// case firstCase
634+
/// case secondCase(Int)
635+
/// case thirdCase(Pair)
636+
/// }
637+
/// ```
638+
///
639+
/// The linearized representation of E would be three slots since the payload
640+
/// with the largest linearized representation is Pair:
641+
///
642+
/// ----- |E| --------
643+
/// / \
644+
/// / \
645+
/// v v
646+
/// | Pair | | Discriminator |
647+
/// / \
648+
/// / \
649+
/// / \
650+
/// v v
651+
/// | LHS | | RHS |
652+
///
653+
/// This in term would mean the following potential bit representations given
654+
/// various cases/states of deinitialization of the payload.
655+
///
656+
/// ```
657+
/// firstCase inited: [1, 1, 1]
658+
/// firstCase deinited: [0, 0, 0]
659+
///
660+
/// secondCase inited: [1, 1, 1]
661+
/// secondCase payload deinited: [0, 1, 1]
662+
/// secondCase deinited: [0, 0, 0]
663+
///
664+
/// thirdCase inited: [1, 1, 1]
665+
/// thirdCase payload deinited: [0, 0, 1]
666+
/// thirdCase deinited: [0, 0, 0]
667+
/// ```
668+
///
669+
/// Now lets consider an enum without any payload cases. Given such an enum:
670+
///
671+
/// ```
672+
/// enum E2 {
673+
/// case firstCase
674+
/// case secondCase
675+
/// case thirdCase
676+
/// }
677+
/// ```
678+
///
679+
/// we would only use a single bit in our linearized representation, just for
680+
/// the discriminator value.
681+
///
682+
/// Enums and Partial Initialization
683+
/// --------------------------------
684+
///
685+
/// One property of our representation of structs and tuples is that a code
686+
/// generator can reinitialize a struct/tuple completely just by re-initializing
687+
/// each of its sub-types individually. This is not possible for enums in our
688+
/// representation since if one just took the leaf nodes for the payload, one
689+
/// would not update the bit for the enum case itself and any additional spare
690+
/// bits. Luckily for us, this is actually impossible to do in SIL since it is
691+
/// impossible to dynamically change the payload of an enum without destroying
692+
/// the original enum and its payload since that would be a verifier caught
693+
/// leak.
562694
struct SubElementNumber {
563695
unsigned number;
564696

include/swift/SILOptimizer/PassManager/Passes.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,8 @@ PASS(AssemblyVisionRemarkGenerator, "assembly-vision-remark-generator",
454454
"Emit assembly vision remarks that provide source level guidance of where runtime calls ended up")
455455
PASS(MoveOnlyObjectChecker, "sil-move-only-object-checker",
456456
"Pass that enforces move only invariants on raw SIL for objects")
457+
PASS(MoveOnlyAddressChecker, "sil-move-only-address-checker",
458+
"Pass that enforces move only invariants on raw SIL for addresses")
457459
PASS(MoveKillsCopyableValuesChecker, "sil-move-kills-copyable-values-checker",
458460
"Pass that checks that any copyable (non-move only) value that is passed "
459461
"to _move do not have any uses later than the _move")

lib/SIL/Utils/PrunedLiveness.cpp

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#include "swift/SIL/PrunedLiveness.h"
14+
#include "swift/AST/TypeExpansionContext.h"
1415
#include "swift/Basic/Defer.h"
1516
#include "swift/SIL/BasicBlockDatastructures.h"
1617
#include "swift/SIL/BasicBlockUtils.h"
1718
#include "swift/SIL/OwnershipUtils.h"
19+
#include "swift/SIL/SILInstruction.h"
1820

1921
using namespace swift;
2022

@@ -406,8 +408,26 @@ TypeSubElementCount::TypeSubElementCount(SILType type, SILModule &mod,
406408
return;
407409
}
408410

409-
// If this isn't a tuple or struct, it is a single element. This was our
410-
// default value, so we can just return.
411+
// If we have an enum, we add one for tracking if the base enum is set and use
412+
// the remaining bits for the max sized payload. This ensures that if we have
413+
// a smaller sized payload, we still get all of the bits set, allowing for a
414+
// homogeneous representation.
415+
if (auto *enumDecl = type.getEnumOrBoundGenericEnum()) {
416+
unsigned numElements = 0;
417+
for (auto *eltDecl : enumDecl->getAllElements()) {
418+
if (!eltDecl->hasAssociatedValues())
419+
continue;
420+
numElements = std::max(
421+
numElements,
422+
unsigned(TypeSubElementCount(
423+
type.getEnumElementType(eltDecl, mod, context), mod, context)));
424+
}
425+
number = numElements + 1;
426+
return;
427+
}
428+
429+
// If this isn't a tuple, struct, or enum, it is a single element. This was
430+
// our default value, so we can just return.
411431
}
412432

413433
Optional<SubElementNumber>
@@ -463,8 +483,30 @@ SubElementNumber::compute(SILValue projectionDerivedFromRoot,
463483
continue;
464484
}
465485

466-
// This fails when we visit unchecked_take_enum_data_addr. We should just
467-
// add support for enums.
486+
// In the case of enums, we note that our representation is:
487+
//
488+
// ---------|Enum| ---
489+
// / \
490+
// / \
491+
// v v
492+
// |Bits for Max Sized Payload| |Discrim Bit|
493+
//
494+
// So our payload is always going to start at the current field number since
495+
// we are the left most child of our parent enum. So we just need to look
496+
// through to our parent enum.
497+
if (auto *enumData = dyn_cast<UncheckedTakeEnumDataAddrInst>(
498+
projectionDerivedFromRoot)) {
499+
projectionDerivedFromRoot = enumData->getOperand();
500+
continue;
501+
}
502+
503+
// Init enum data addr is treated like unchecked take enum data addr.
504+
if (auto *initData =
505+
dyn_cast<InitEnumDataAddrInst>(projectionDerivedFromRoot)) {
506+
projectionDerivedFromRoot = initData->getOperand();
507+
continue;
508+
}
509+
468510
#ifndef NDEBUG
469511
if (!isa<InitExistentialAddrInst>(projectionDerivedFromRoot)) {
470512
llvm::errs() << "Unknown access path instruction!\n";

lib/SILGen/SILGenBuilder.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -946,7 +946,8 @@ ManagedValue SILGenBuilder::createGuaranteedCopyableToMoveOnlyWrapperValue(
946946
ManagedValue
947947
SILGenBuilder::createMarkMustCheckInst(SILLocation loc, ManagedValue value,
948948
MarkMustCheckInst::CheckKind kind) {
949-
assert(value.isPlusOne(SGF) && "Argument must be at +1!");
949+
assert((value.isPlusOne(SGF) || value.isLValue()) &&
950+
"Argument must be at +1 or be an inout!");
950951
CleanupCloner cloner(*this, value);
951952
auto *mdi = SILBuilder::createMarkMustCheckInst(
952953
loc, value.forward(getSILGenFunction()), kind);

lib/SILGen/SILGenPattern.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2834,7 +2834,7 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
28342834
auto subject = ([&]() -> ConsumableManagedValue {
28352835
// If we have a move only value, ensure plus one and convert it. Switches
28362836
// always consume move only values.
2837-
if (subjectMV.getType().isMoveOnly()) {
2837+
if (subjectMV.getType().isMoveOnly() && subjectMV.getType().isObject()) {
28382838
if (subjectMV.getType().isMoveOnlyWrapped()) {
28392839
subjectMV = B.createOwnedMoveOnlyWrapperToCopyableValue(
28402840
S, subjectMV.ensurePlusOne(*this, S));

lib/SILGen/SILGenProlog.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "swift/AST/ParameterList.h"
2323
#include "swift/AST/PropertyWrappers.h"
2424
#include "swift/SIL/SILArgument.h"
25+
#include "swift/SIL/SILArgumentConvention.h"
2526
#include "swift/SIL/SILInstruction.h"
2627

2728
using namespace swift;
@@ -108,8 +109,14 @@ class EmitBBArguments : public CanTypeVisitor<EmitBBArguments,
108109
}
109110
}
110111
}
111-
if (isInOut)
112+
if (isInOut) {
113+
// If we are inout and are move only, insert a note to the move checker to
114+
// check ownership.
115+
if (mv.getType().isMoveOnly() && !mv.getType().isMoveOnlyWrapped())
116+
mv = SGF.B.createMarkMustCheckInst(
117+
loc, mv, MarkMustCheckInst::CheckKind::NoImplicitCopy);
112118
return mv;
119+
}
113120

114121
// This can happen if the value is resilient in the calling convention
115122
// but not resilient locally.
@@ -561,6 +568,10 @@ static void emitCaptureArguments(SILGenFunction &SGF,
561568
ty = ty.getAddressType();
562569
}
563570
SILValue arg = SGF.F.begin()->createFunctionArgument(ty, VD);
571+
if (isInOut && (ty.isMoveOnly() && !ty.isMoveOnlyWrapped())) {
572+
arg = SGF.B.createMarkMustCheckInst(
573+
Loc, arg, MarkMustCheckInst::CheckKind::NoImplicitCopy);
574+
}
564575
SGF.VarLocs[VD] = SILGenFunction::VarLoc::get(arg);
565576
SILDebugVariable DbgVar(VD->isLet(), ArgNo);
566577
if (ty.isAddress()) {

lib/SILOptimizer/Mandatory/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ target_sources(swiftSILOptimizer PRIVATE
2525
MoveKillsCopyableAddressesChecker.cpp
2626
MoveKillsCopyableValuesChecker.cpp
2727
MoveOnlyObjectChecker.cpp
28+
MoveOnlyAddressChecker.cpp
2829
NestedSemanticFunctionCheck.cpp
2930
OptimizeHopToExecutor.cpp
3031
PerformanceDiagnostics.cpp

0 commit comments

Comments
 (0)