Skip to content

Commit 1faeb50

Browse files
nate-chandlernate-chandler
authored
Merge pull request #73121 from nate-chandler/rdar126715654
[NoncopyablePartialConsumption] Promote to feature.
2 parents 5fce3f9 + 3e2d1d0 commit 1faeb50

File tree

57 files changed

+320
-1267
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+320
-1267
lines changed

‎include/swift/Basic/Features.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ LANGUAGE_FEATURE(BuiltinStoreRaw, 0, "Builtin.storeRaw")
176176
LANGUAGE_FEATURE(BuiltinCreateTask, 0, "Builtin.createTask and Builtin.createDiscardingTask")
177177
SUPPRESSIBLE_LANGUAGE_FEATURE(AssociatedTypeImplements, 0, "@_implements on associated types")
178178
LANGUAGE_FEATURE(BuiltinAddressOfRawLayout, 0, "Builtin.addressOfRawLayout")
179+
LANGUAGE_FEATURE(MoveOnlyPartialConsumption, 429, "Partial consumption of noncopyable values")
179180

180181
// Swift 6
181182
UPCOMING_FEATURE(ConciseMagicFile, 274, 6)
@@ -191,7 +192,6 @@ UPCOMING_FEATURE(InferSendableFromCaptures, 418, 6)
191192
UPCOMING_FEATURE(ImplicitOpenExistentials, 352, 6)
192193
UPCOMING_FEATURE(RegionBasedIsolation, 414, 6)
193194
UPCOMING_FEATURE(DynamicActorIsolation, 423, 6)
194-
UPCOMING_FEATURE(MoveOnlyPartialConsumption, 429, 6)
195195

196196
// Swift 7
197197
UPCOMING_FEATURE(ExistentialAny, 335, 7)

‎include/swift/SIL/FieldSensitivePrunedLiveness.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,11 @@ struct SubElementOffset {
239239
/// s a struct => count(s) := sum(s.fields, { f in count(type(f)) })
240240
/// + s.hasDeinit
241241
/// e an enum => count(e) := sum(e.elements, { elt in count(type(elt)) })
242-
/// + e.hasDeinit
243242
/// + 1 // discriminator
243+
/// + e.hasDeinit
244+
///
245+
/// The deinit bit is at the end to make drop_deinit produce a value whose
246+
/// leaves are contiguous.
244247
struct TypeSubElementCount {
245248
unsigned number;
246249

‎lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp

Lines changed: 93 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -375,13 +375,50 @@ SubElementOffset::computeForValue(SILValue projectionDerivedFromRoot,
375375
// MARK: TypeTreeLeafTypeRange
376376
//===----------------------------------------------------------------------===//
377377

378+
/// Whether \p targetInst is dominated by one of the provided switch_enum_addr's
379+
/// destination blocks whose corresponding enum element has no associated values
380+
/// which need to be destroyed (i.e. either it has no associated values or they
381+
/// are trivial).
382+
static bool isDominatedByPayloadlessSwitchEnumAddrDests(
383+
SILInstruction *targetInst, ArrayRef<SwitchEnumAddrInst *> seais,
384+
DominanceInfo *domTree) {
385+
if (seais.empty())
386+
return false;
387+
auto *target = targetInst->getParent();
388+
for (auto *seai : seais) {
389+
if (!domTree->dominates(seai, targetInst)) {
390+
continue;
391+
}
392+
auto size = seai->getNumCases();
393+
auto ty = seai->getOperand()->getType();
394+
for (unsigned index = 0; index < size; ++index) {
395+
auto pair = seai->getCase(index);
396+
auto *eltDecl = pair.first;
397+
if (eltDecl->hasAssociatedValues()) {
398+
auto eltTy = ty.getEnumElementType(eltDecl, seai->getFunction());
399+
if (!eltTy.isTrivial(*seai->getFunction())) {
400+
continue;
401+
}
402+
}
403+
auto *block = pair.second;
404+
if (domTree->dominates(block, target))
405+
return true;
406+
}
407+
}
408+
return false;
409+
}
410+
378411
void TypeTreeLeafTypeRange::constructFilteredProjections(
379412
SILValue value, SILInstruction *insertPt, SmallBitVector &filterBitVector,
380413
DominanceInfo *domTree,
381414
llvm::function_ref<bool(SILValue, TypeTreeLeafTypeRange, NeedsDestroy_t)>
382415
callback) {
383416
auto *fn = insertPt->getFunction();
384417
SILType type = value->getType();
418+
auto loc =
419+
(insertPt->getLoc().getKind() != SILLocation::ArtificialUnreachableKind)
420+
? insertPt->getLoc()
421+
: RegularLocation::getAutoGeneratedLocation();
385422

386423
PRUNED_LIVENESS_LOG(llvm::dbgs() << "ConstructFilteredProjection. Bv: "
387424
<< filterBitVector << '\n');
@@ -409,8 +446,7 @@ void TypeTreeLeafTypeRange::constructFilteredProjections(
409446
continue;
410447
}
411448

412-
auto newValue =
413-
builder.createStructElementAddr(insertPt->getLoc(), value, varDecl);
449+
auto newValue = builder.createStructElementAddr(loc, value, varDecl);
414450
callback(newValue, TypeTreeLeafTypeRange(start, next), NeedsDestroy);
415451
start = next;
416452
}
@@ -429,60 +465,72 @@ void TypeTreeLeafTypeRange::constructFilteredProjections(
429465
unsigned next;
430466
};
431467
SmallVector<ElementRecord, 2> projectedElements;
432-
unsigned start = startEltOffset;
468+
unsigned runningStart = startEltOffset;
433469
for (auto *eltDecl : enumDecl->getAllElements()) {
434470
if (!eltDecl->hasAssociatedValues())
435471
continue;
436472

437-
auto nextType = type.getEnumElementType(eltDecl, fn);
438-
unsigned next = start + TypeSubElementCount(nextType, fn);
439-
if (noneSet(filterBitVector, start, next)) {
440-
start = next;
473+
auto eltTy = type.getEnumElementType(eltDecl, fn);
474+
unsigned next = runningStart + TypeSubElementCount(eltTy, fn);
475+
if (noneSet(filterBitVector, runningStart, next)) {
476+
runningStart = next;
441477
continue;
442478
}
443479

444-
projectedElements.push_back({eltDecl, start, next});
445-
start = next;
480+
projectedElements.push_back({eltDecl, runningStart, next});
481+
runningStart = next;
446482
}
483+
assert((runningStart + 1 + (type.isValueTypeWithDeinit() ? 1 : 0)) ==
484+
endEltOffset);
447485

448-
// Add a bit for the discriminator.
449-
unsigned next = start + 1;
450-
451-
if (!allSet(filterBitVector, start, next)) {
486+
if (!allSet(filterBitVector, startEltOffset, endEltOffset)) {
487+
TinyPtrVector<SwitchEnumAddrInst *> seais;
452488
for (auto record : projectedElements) {
453489
// Find a preexisting unchecked_take_enum_data_addr that dominates
454490
// insertPt.
455491
bool foundProjection = false;
456-
for (auto *user : value->getUsers()) {
457-
auto *utedai = dyn_cast<UncheckedTakeEnumDataAddrInst>(user);
458-
if (!utedai) {
459-
continue;
460-
}
461-
if (utedai->getElement() == record.element) {
462-
continue;
492+
StackList<SILValue> worklist(value->getFunction());
493+
worklist.push_back(value);
494+
while (!worklist.empty()) {
495+
auto v = worklist.pop_back_val();
496+
for (auto *user : v->getUsers()) {
497+
if (auto *ddi = dyn_cast<DropDeinitInst>(user)) {
498+
worklist.push_back(ddi);
499+
continue;
500+
}
501+
if (auto *seai = dyn_cast<SwitchEnumAddrInst>(user)) {
502+
seais.push_back(seai);
503+
}
504+
auto *utedai = dyn_cast<UncheckedTakeEnumDataAddrInst>(user);
505+
if (!utedai) {
506+
continue;
507+
}
508+
if (utedai->getElement() != record.element) {
509+
continue;
510+
}
511+
if (!domTree->dominates(utedai, insertPt)) {
512+
continue;
513+
}
514+
515+
callback(utedai, TypeTreeLeafTypeRange(record.start, record.next),
516+
NeedsDestroy);
517+
foundProjection = true;
463518
}
464-
if (!domTree->dominates(utedai, insertPt)) {
465-
continue;
466-
}
467-
468-
callback(utedai, TypeTreeLeafTypeRange(record.start, record.next),
469-
DoesNotNeedDestroy);
470-
foundProjection = true;
471519
}
520+
(void)foundProjection;
472521
assert(foundProjection ||
473-
llvm::count_if(enumDecl->getAllElements(), [](auto *elt) {
474-
return elt->hasAssociatedValues();
475-
}) == 1);
522+
llvm::count_if(
523+
enumDecl->getAllElements(),
524+
[](auto *elt) { return elt->hasAssociatedValues(); }) == 1 ||
525+
isDominatedByPayloadlessSwitchEnumAddrDests(insertPt, seais,
526+
domTree));
476527
}
477528
return;
478529
}
479530

480531
// Then just pass back our enum base value as the pointer.
481-
callback(value, TypeTreeLeafTypeRange(start, next), NeedsDestroy);
482-
483-
// Then set start to next and assert we covered the entire end elt offset.
484-
start = next;
485-
assert(start == endEltOffset);
532+
callback(value, TypeTreeLeafTypeRange(startEltOffset, endEltOffset),
533+
NeedsDestroy);
486534
return;
487535
}
488536

@@ -497,8 +545,7 @@ void TypeTreeLeafTypeRange::constructFilteredProjections(
497545
continue;
498546
}
499547

500-
auto newValue =
501-
builder.createTupleElementAddr(insertPt->getLoc(), value, index);
548+
auto newValue = builder.createTupleElementAddr(loc, value, index);
502549
callback(newValue, TypeTreeLeafTypeRange(start, next), NeedsDestroy);
503550
start = next;
504551
}
@@ -526,9 +573,12 @@ void TypeTreeLeafTypeRange::get(
526573

527574
// An `inject_enum_addr` only initializes the enum tag.
528575
if (auto inject = dyn_cast<InjectEnumAddrInst>(op->getUser())) {
529-
auto upperBound = *startEltOffset + TypeSubElementCount(projectedValue);
530-
// TODO: account for deinit component if enum has deinit.
531-
assert(!projectedValue->getType().isValueTypeWithDeinit());
576+
// Subtract the deinit bit, if any: the discriminator bit is before it:
577+
//
578+
// [ case1 bits ..., case2 bits, ..., discriminator bit, deinit bit ]
579+
auto deinitBits = projectedValue->getType().isValueTypeWithDeinit() ? 1 : 0;
580+
auto upperBound =
581+
*startEltOffset + TypeSubElementCount(projectedValue) - deinitBits;
532582
ranges.push_back({upperBound - 1, upperBound});
533583
return;
534584
}
@@ -551,10 +601,11 @@ void TypeTreeLeafTypeRange::get(
551601
}
552602
numAtoms += elementAtoms;
553603
}
554-
// TODO: account for deinit component if enum has deinit.
555-
assert(!projectedValue->getType().isValueTypeWithDeinit());
604+
// The discriminator bit is consumed.
556605
ranges.push_back(
557606
{*startEltOffset + numAtoms, *startEltOffset + numAtoms + 1});
607+
// The deinit bit is _not_ consumed. A drop_deinit is required to
608+
// consumingly switch an enum with a deinit.
558609
return;
559610
}
560611

‎lib/SILGen/SILGenDestructor.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -264,14 +264,11 @@ void SILGenFunction::emitDeallocatingMoveOnlyDestructor(DestructorDecl *dd) {
264264
dd->getDeclContext()->getSelfNominalTypeDecl(),
265265
cleanupLoc);
266266

267-
if (getASTContext().LangOpts.hasFeature(
268-
Feature::MoveOnlyPartialConsumption)) {
269-
if (auto *ddi = dyn_cast<DropDeinitInst>(selfValue)) {
270-
if (auto *mu =
271-
dyn_cast<MarkUnresolvedNonCopyableValueInst>(ddi->getOperand())) {
272-
if (auto *asi = dyn_cast<AllocStackInst>(mu->getOperand())) {
273-
B.createDeallocStack(loc, asi);
274-
}
267+
if (auto *ddi = dyn_cast<DropDeinitInst>(selfValue)) {
268+
if (auto *mu =
269+
dyn_cast<MarkUnresolvedNonCopyableValueInst>(ddi->getOperand())) {
270+
if (auto *asi = dyn_cast<AllocStackInst>(mu->getOperand())) {
271+
B.createDeallocStack(loc, asi);
275272
}
276273
}
277274
}

‎lib/SILGen/SILGenProlog.cpp

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -59,30 +59,19 @@ SILValue SILGenFunction::emitSelfDeclForDestructor(VarDecl *selfDecl) {
5959
// owned, lets mark them as needing to be no implicit copy checked so they
6060
// cannot escape.
6161
if (selfType.isMoveOnly() && !selfType.isAnyClassReferenceType()) {
62-
if (getASTContext().LangOpts.hasFeature(
63-
Feature::MoveOnlyPartialConsumption)) {
64-
SILValue addr = B.createAllocStack(selfDecl, selfValue->getType(), dv);
65-
addr = B.createMarkUnresolvedNonCopyableValueInst(
66-
selfDecl, addr,
67-
MarkUnresolvedNonCopyableValueInst::CheckKind::
68-
ConsumableAndAssignable);
69-
if (selfValue->getType().isObject()) {
70-
B.createStore(selfDecl, selfValue, addr, StoreOwnershipQualifier::Init);
71-
} else {
72-
B.createCopyAddr(selfDecl, selfValue, addr, IsTake, IsInitialization);
73-
}
74-
// drop_deinit invalidates any user-defined struct/enum deinit
75-
// before the individual members are destroyed.
76-
addr = B.createDropDeinit(selfDecl, addr);
77-
selfValue = addr;
62+
SILValue addr = B.createAllocStack(selfDecl, selfValue->getType(), dv);
63+
addr = B.createMarkUnresolvedNonCopyableValueInst(
64+
selfDecl, addr,
65+
MarkUnresolvedNonCopyableValueInst::CheckKind::ConsumableAndAssignable);
66+
if (selfValue->getType().isObject()) {
67+
B.createStore(selfDecl, selfValue, addr, StoreOwnershipQualifier::Init);
7868
} else {
79-
if (selfValue->getOwnershipKind() == OwnershipKind::Owned) {
80-
selfValue = B.createMarkUnresolvedNonCopyableValueInst(
81-
selfDecl, selfValue,
82-
MarkUnresolvedNonCopyableValueInst::CheckKind::
83-
ConsumableAndAssignable);
84-
}
69+
B.createCopyAddr(selfDecl, selfValue, addr, IsTake, IsInitialization);
8570
}
71+
// drop_deinit invalidates any user-defined struct/enum deinit
72+
// before the individual members are destroyed.
73+
addr = B.createDropDeinit(selfDecl, addr);
74+
selfValue = addr;
8675
}
8776

8877
VarLocs[selfDecl] = VarLoc::get(selfValue);

‎lib/SILOptimizer/Analysis/SimplifyInstruction.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ SILValue InstSimplifier::visitStructExtractInst(StructExtractInst *sei) {
172172

173173
SILValue
174174
InstSimplifier::visitUncheckedEnumDataInst(UncheckedEnumDataInst *uedi) {
175+
if (uedi->getOperand()->getType().isValueTypeWithDeinit())
176+
return SILValue();
175177
// (unchecked_enum_data (enum payload)) -> payload
176178
auto opt = lookThroughOwnershipInsts(uedi->getOperand());
177179
if (auto *ei = dyn_cast<EnumInst>(opt)) {

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerTester.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ class MoveOnlyAddressCheckerTesterPass : public SILFunctionTransform {
109109
borrowtodestructure::IntervalMapAllocator allocator;
110110
MoveOnlyAddressChecker checker{getFunction(), diagnosticEmitter,
111111
allocator, domTree, poa};
112-
madeChange = checker.check(moveIntroducersToProcess);
112+
madeChange = checker.completeLifetimes();
113+
madeChange |= checker.check(moveIntroducersToProcess);
113114
}
114115

115116
// If we did not emit any diagnostics, emit a diagnostic if we missed any

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@
242242
#include "swift/SIL/FieldSensitivePrunedLiveness.h"
243243
#include "swift/SIL/InstructionUtils.h"
244244
#include "swift/SIL/MemAccessUtils.h"
245+
#include "swift/SIL/OSSALifetimeCompletion.h"
245246
#include "swift/SIL/OwnershipUtils.h"
246247
#include "swift/SIL/PrunedLiveness.h"
247248
#include "swift/SIL/SILArgument.h"
@@ -1663,10 +1664,20 @@ struct CopiedLoadBorrowEliminationVisitor
16631664
}
16641665

16651666
case OperandOwnership::ForwardingConsume:
1666-
case OperandOwnership::DestroyingConsume:
1667+
case OperandOwnership::DestroyingConsume: {
1668+
if (auto *dvi = dyn_cast<DestroyValueInst>(nextUse->getUser())) {
1669+
auto value = dvi->getOperand();
1670+
auto *pai = dyn_cast_or_null<PartialApplyInst>(
1671+
value->getDefiningInstruction());
1672+
if (pai && pai->isOnStack()) {
1673+
// A destroy_value of an on_stack partial apply isn't actually a
1674+
// consuming use--it closes a borrow scope.
1675+
continue;
1676+
}
1677+
}
16671678
// We can only hit this if our load_borrow was copied.
16681679
llvm_unreachable("We should never hit this");
1669-
1680+
}
16701681
case OperandOwnership::GuaranteedForwarding: {
16711682
SmallVector<SILValue, 8> forwardedValues;
16721683
auto *fn = nextUse->getUser()->getFunction();
@@ -1791,10 +1802,11 @@ shouldEmitPartialMutationError(UseState &useState, PartialMutation::Kind kind,
17911802
}
17921803

17931804
auto feature = partialMutationFeature(kind);
1794-
if (!fn->getModule().getASTContext().LangOpts.hasFeature(feature) &&
1805+
if (feature &&
1806+
!fn->getModule().getASTContext().LangOpts.hasFeature(*feature) &&
17951807
!isa<DropDeinitInst>(user)) {
17961808
LLVM_DEBUG(llvm::dbgs()
1797-
<< " " << getFeatureName(feature) << " disabled!\n");
1809+
<< " " << getFeatureName(*feature) << " disabled!\n");
17981810
// If the types equal, just bail early.
17991811
// Emit the error.
18001812
return {PartialMutationError::featureDisabled(iterType, kind)};
@@ -3975,3 +3987,29 @@ bool MoveOnlyAddressChecker::check(
39753987
}
39763988
return pimpl.changed;
39773989
}
3990+
bool MoveOnlyAddressChecker::completeLifetimes() {
3991+
// TODO: Delete once OSSALifetimeCompletion is run as part of SILGenCleanup
3992+
bool changed = false;
3993+
3994+
// Lifetimes must be completed inside out (bottom-up in the CFG).
3995+
PostOrderFunctionInfo *postOrder = poa->get(fn);
3996+
OSSALifetimeCompletion completion(fn, domTree);
3997+
for (auto *block : postOrder->getPostOrder()) {
3998+
for (SILInstruction &inst : reverse(*block)) {
3999+
for (auto result : inst.getResults()) {
4000+
if (completion.completeOSSALifetime(result) ==
4001+
LifetimeCompletion::WasCompleted) {
4002+
changed = true;
4003+
}
4004+
}
4005+
}
4006+
for (SILArgument *arg : block->getArguments()) {
4007+
assert(!arg->isReborrow() && "reborrows not legal at this SIL stage");
4008+
if (completion.completeOSSALifetime(arg) ==
4009+
LifetimeCompletion::WasCompleted) {
4010+
changed = true;
4011+
}
4012+
}
4013+
}
4014+
return changed;
4015+
}

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ struct MoveOnlyAddressChecker {
4444
/// \p diagnosticEmitter.getDiagnosticCount().
4545
bool check(llvm::SmallSetVector<MarkUnresolvedNonCopyableValueInst *, 32>
4646
&moveIntroducersToProcess);
47+
bool completeLifetimes();
4748
};
4849

4950
} // namespace siloptimizer

0 commit comments

Comments
 (0)