Skip to content

Commit a6f65f0

Browse files
atrickatrick
committed
[move-only] Fix drop_deinit OSSA lowering
drop_deinit ultimately only affects the semantics of its destroy_value. Avoid generating releases for destroys in which the deinit has been dropped. Instead, individually release the members.
1 parent cc0c44c commit a6f65f0

File tree

4 files changed

+122
-119
lines changed

4 files changed

+122
-119
lines changed

‎lib/SILGen/SILGenDestructor.cpp

Lines changed: 45 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ void SILGenFunction::emitDeallocatingMoveOnlyDestructor(DestructorDecl *dd) {
262262
// Clean up our members, consuming our +1 self value as we do it.
263263
emitMoveOnlyMemberDestruction(selfValue,
264264
dd->getDeclContext()->getSelfNominalTypeDecl(),
265-
cleanupLoc, nullptr);
265+
cleanupLoc);
266266

267267
// Return.
268268
B.createReturn(loc, emitEmptyTuple(loc));
@@ -497,113 +497,59 @@ void SILGenFunction::emitClassMemberDestruction(ManagedValue selfValue,
497497

498498
void SILGenFunction::emitMoveOnlyMemberDestruction(SILValue selfValue,
499499
NominalTypeDecl *nom,
500-
CleanupLocation cleanupLoc,
501-
SILBasicBlock *finishBB) {
502-
// drop_deinit must be used to invalidate any user-defined struct/enum deinit
503-
// before the individual members can be destroyed.
500+
CleanupLocation cleanupLoc) {
501+
// drop_deinit invalidates any user-defined struct/enum deinit
502+
// before the individual members are destroyed.
504503
selfValue = B.createDropDeinit(cleanupLoc, selfValue);
505-
if (selfValue->getType().isAddress()) {
506-
if (auto *structDecl = dyn_cast<StructDecl>(nom)) {
507-
for (VarDecl *vd : nom->getStoredProperties()) {
508-
const TypeLowering &ti = getTypeLowering(vd->getType());
509-
if (ti.isTrivial())
510-
continue;
511-
512-
SILValue addr = B.createStructElementAddr(
513-
cleanupLoc, selfValue, vd, ti.getLoweredType().getAddressType());
514-
addr = B.createBeginAccess(cleanupLoc, addr, SILAccessKind::Deinit,
515-
SILAccessEnforcement::Static,
516-
false /*noNestedConflict*/,
517-
false /*fromBuiltin*/);
518-
B.createDestroyAddr(cleanupLoc, addr);
519-
B.createEndAccess(cleanupLoc, addr, false /*is aborting*/);
520-
}
521-
} else {
522-
auto *origBlock = B.getInsertionBB();
523-
auto *enumDecl = cast<EnumDecl>(nom);
524-
SmallVector<std::pair<EnumElementDecl *, SILBasicBlock *>, 8>
525-
caseCleanups;
526-
auto *contBlock = createBasicBlock();
527-
528-
for (auto *enumElt : enumDecl->getAllElements()) {
529-
auto *enumBlock = createBasicBlock();
530-
SILBuilder builder(enumBlock, enumBlock->begin());
531-
532-
if (enumElt->hasAssociatedValues()) {
533-
auto *take = builder.createUncheckedTakeEnumDataAddr(
534-
cleanupLoc, selfValue, enumElt);
535-
builder.createDestroyAddr(cleanupLoc, take);
536-
}
537-
538-
// Branch to the continue trampoline block.
539-
builder.createBranch(cleanupLoc, contBlock);
540-
caseCleanups.emplace_back(enumElt, enumBlock);
541-
542-
// Set the insertion point to after this enum block so we insert the
543-
// next new block after this block.
544-
B.setInsertionPoint(enumBlock);
545-
}
504+
if (selfValue->getType().isObject()) {
505+
// A destroy value that uses the result of a drop_deinit implicitly performs
506+
// memberwise destruction.
507+
B.emitDestroyValueOperation(cleanupLoc, selfValue);
508+
return;
509+
}
510+
if (auto *structDecl = dyn_cast<StructDecl>(nom)) {
511+
for (VarDecl *vd : nom->getStoredProperties()) {
512+
const TypeLowering &ti = getTypeLowering(vd->getType());
513+
if (ti.isTrivial())
514+
continue;
546515

547-
B.setInsertionPoint(origBlock);
548-
B.createSwitchEnumAddr(cleanupLoc, selfValue, nullptr, caseCleanups);
549-
B.setInsertionPoint(contBlock);
516+
SILValue addr = B.createStructElementAddr(
517+
cleanupLoc, selfValue, vd, ti.getLoweredType().getAddressType());
518+
addr = B.createBeginAccess(
519+
cleanupLoc, addr, SILAccessKind::Deinit, SILAccessEnforcement::Static,
520+
false /*noNestedConflict*/, false /*fromBuiltin*/);
521+
B.createDestroyAddr(cleanupLoc, addr);
522+
B.createEndAccess(cleanupLoc, addr, false /*is aborting*/);
550523
}
551524
} else {
552-
if (auto *sd = dyn_cast<StructDecl>(nom)) {
553-
if (llvm::any_of(sd->getStoredProperties(),
554-
[&](VarDecl *vd) {
555-
auto &lowering = getTypeLowering(vd->getType());
556-
return !lowering.isTrivial();
557-
})) {
558-
auto *d = B.createDestructureStruct(cleanupLoc, selfValue,
559-
OwnershipKind::Owned);
560-
for (auto result : d->getResults()) {
561-
B.emitDestroyValueOperation(cleanupLoc, result);
562-
}
563-
} else {
564-
// If we only contain trivial uses, we don't have anything to cleanup,
565-
// so just insert an end_lifetime.
566-
B.createEndLifetime(cleanupLoc, selfValue);
525+
auto *origBlock = B.getInsertionBB();
526+
auto *enumDecl = cast<EnumDecl>(nom);
527+
SmallVector<std::pair<EnumElementDecl *, SILBasicBlock *>, 8> caseCleanups;
528+
auto *contBlock = createBasicBlock();
529+
530+
for (auto *enumElt : enumDecl->getAllElements()) {
531+
auto *enumBlock = createBasicBlock();
532+
SILBuilder builder(enumBlock, enumBlock->begin());
533+
534+
if (enumElt->hasAssociatedValues()) {
535+
auto *take = builder.createUncheckedTakeEnumDataAddr(
536+
cleanupLoc, selfValue, enumElt);
537+
builder.createDestroyAddr(cleanupLoc, take);
567538
}
568-
} else {
569-
auto *origBlock = B.getInsertionBB();
570-
auto *enumDecl = dyn_cast<EnumDecl>(nom);
571-
SmallVector<std::pair<EnumElementDecl *, SILBasicBlock *>, 8>
572-
caseCleanups;
573-
auto *contBlock = createBasicBlock();
574-
575-
for (auto *enumElt : enumDecl->getAllElements()) {
576-
auto *enumBlock = createBasicBlock();
577-
SILBuilder builder(enumBlock, enumBlock->begin());
578-
579-
if (enumElt->hasAssociatedValues()) {
580-
auto caseType = selfValue->getType().getEnumElementType(
581-
enumElt, enumBlock->getParent());
582-
auto *phiArg =
583-
enumBlock->createPhiArgument(caseType, OwnershipKind::Owned);
584-
builder.emitDestroyValueOperation(cleanupLoc, phiArg);
585-
}
586539

587-
// Branch to the continue trampoline block.
588-
builder.createBranch(cleanupLoc, contBlock);
589-
caseCleanups.emplace_back(enumElt, enumBlock);
540+
// Branch to the continue trampoline block.
541+
builder.createBranch(cleanupLoc, contBlock);
542+
caseCleanups.emplace_back(enumElt, enumBlock);
590543

591-
// Set the insertion point to after this enum block so we insert the
592-
// next new block after this block.
593-
B.setInsertionPoint(enumBlock);
594-
}
595-
596-
B.setInsertionPoint(origBlock);
597-
B.createSwitchEnum(cleanupLoc, selfValue, nullptr, caseCleanups);
598-
B.setInsertionPoint(contBlock);
544+
// Set the insertion point to after this enum block so we insert the
545+
// next new block after this block.
546+
B.setInsertionPoint(enumBlock);
599547
}
600-
}
601548

602-
if (finishBB)
603-
B.createBranch(cleanupLoc, finishBB);
604-
605-
if (finishBB)
606-
B.emitBlock(finishBB);
549+
B.setInsertionPoint(origBlock);
550+
B.createSwitchEnumAddr(cleanupLoc, selfValue, nullptr, caseCleanups);
551+
B.setInsertionPoint(contBlock);
552+
}
607553
}
608554

609555
void SILGenFunction::emitObjCDestructor(SILDeclRef dtor) {

‎lib/SILGen/SILGenFunction.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -815,12 +815,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
815815
///
816816
/// \param selfValue The 'self' value.
817817
/// \param nd The nominal declaration whose members are being destroyed.
818-
/// \param finishBB If set, used as the basic block after members have been
819-
/// destroyed, and we're ready to perform final cleanups
820-
/// before returning.
821818
void emitMoveOnlyMemberDestruction(SILValue selfValue, NominalTypeDecl *nd,
822-
CleanupLocation cleanupLoc,
823-
SILBasicBlock *finishBB);
819+
CleanupLocation cleanupLoc);
824820

825821
/// Generates code to destroy linearly recursive data structures, without
826822
/// building up the call stack.

‎lib/SILGen/SILGenStmt.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -795,8 +795,7 @@ void StmtEmitter::visitDiscardStmt(DiscardStmt *S) {
795795
}
796796
}
797797

798-
SGF.emitMoveOnlyMemberDestruction(selfValue.forward(SGF), nominal, loc,
799-
nullptr);
798+
SGF.emitMoveOnlyMemberDestruction(selfValue.forward(SGF), nominal, loc);
800799
}
801800

802801
void StmtEmitter::visitYieldStmt(YieldStmt *S) {

‎lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,17 @@ struct OwnershipModelEliminatorVisitor
135135
bool visitCopyValueInst(CopyValueInst *cvi);
136136
bool visitExplicitCopyValueInst(ExplicitCopyValueInst *cvi);
137137
bool visitExplicitCopyAddrInst(ExplicitCopyAddrInst *cai);
138+
139+
void splitDestroy(DestroyValueInst *destroy);
138140
bool visitDestroyValueInst(DestroyValueInst *dvi);
139141
bool visitLoadBorrowInst(LoadBorrowInst *lbi);
140142
bool visitMoveValueInst(MoveValueInst *mvi) {
141143
eraseInstructionAndRAUW(mvi, mvi->getOperand());
142144
return true;
143145
}
144146
bool visitDropDeinitInst(DropDeinitInst *ddi) {
145-
eraseInstructionAndRAUW(ddi, ddi->getOperand());
146-
return true;
147+
instructionsToSimplify.insert(ddi);
148+
return false;
147149
}
148150
bool visitBeginBorrowInst(BeginBorrowInst *bbi) {
149151
eraseInstructionAndRAUW(bbi, bbi->getOperand());
@@ -481,6 +483,57 @@ bool OwnershipModelEliminatorVisitor::visitPartialApplyInst(
481483
return false;
482484
}
483485

486+
// Destroy all nontrivial members of the struct or enum destroyed by \p destroy
487+
// ignoring any user-defined deinit.
488+
//
489+
// See also splitDestructure().
490+
void OwnershipModelEliminatorVisitor::splitDestroy(DestroyValueInst *destroy) {
491+
SILModule &module = destroy->getModule();
492+
SILFunction *function = destroy->getFunction();
493+
auto loc = destroy->getLoc();
494+
auto operand = destroy->getOperand();
495+
auto operandTy = operand->getType();
496+
NominalTypeDecl *nominalDecl = operandTy.getNominalOrBoundGenericNominal();
497+
498+
if (auto *sd = dyn_cast<StructDecl>(nominalDecl)) {
499+
withBuilder<void>(destroy, [&](SILBuilder &builder, SILLocation loc) {
500+
llvm::SmallVector<Projection, 8> projections;
501+
Projection::getFirstLevelProjections(
502+
operandTy, module, TypeExpansionContext(*function), projections);
503+
for (Projection &projection : projections) {
504+
auto *projectedValue =
505+
projection.createObjectProjection(builder, loc, operand).get();
506+
builder.emitDestroyValueOperation(loc, projectedValue);
507+
}
508+
});
509+
return;
510+
}
511+
512+
// "Destructure" an enum.
513+
auto *enumDecl = dyn_cast<EnumDecl>(nominalDecl);
514+
SmallVector<std::pair<EnumElementDecl *, SILBasicBlock *>, 8> caseCleanups;
515+
auto *destroyBlock = destroy->getParent();
516+
auto *contBlock = destroyBlock->split(std::next(destroy->getIterator()));
517+
518+
for (auto *enumElt : enumDecl->getAllElements()) {
519+
auto *enumBlock = function->createBasicBlockBefore(contBlock);
520+
SILBuilder builder(enumBlock, enumBlock->begin());
521+
if (enumElt->hasAssociatedValues()) {
522+
auto caseType = operandTy.getEnumElementType(enumElt, function);
523+
auto *phiArg =
524+
enumBlock->createPhiArgument(caseType, OwnershipKind::Owned);
525+
SILBuilderWithScope(enumBlock, builderCtx, destroy->getDebugScope())
526+
.emitDestroyValueOperation(loc, phiArg);
527+
}
528+
// Branch to the continue block.
529+
builder.createBranch(loc, contBlock);
530+
caseCleanups.emplace_back(enumElt, enumBlock);
531+
}
532+
SILBuilderWithScope switchBuilder(destroyBlock, builderCtx,
533+
destroy->getDebugScope());
534+
switchBuilder.createSwitchEnum(loc, operand, nullptr, caseCleanups);
535+
}
536+
484537
bool OwnershipModelEliminatorVisitor::visitDestroyValueInst(
485538
DestroyValueInst *dvi) {
486539
// Nonescaping closures are represented ultimately as trivial pointers to
@@ -494,12 +547,15 @@ bool OwnershipModelEliminatorVisitor::visitDestroyValueInst(
494547
return true;
495548
}
496549
}
497-
498-
// A destroy_value of an address-only type cannot be replaced.
499-
//
500-
// TODO: When LowerAddresses runs before this, we can remove this case.
501-
if (operandTy.isAddressOnly(*dvi->getFunction()))
502-
return false;
550+
551+
// A drop_deinit eliminates any user-defined deinit. Its destroy does not
552+
// lower to a release. If any members require deinitialization, they must be
553+
// destructured and individually destroyed.
554+
if (isa<DropDeinitInst>(lookThroughOwnershipInsts(operand))) {
555+
splitDestroy(dvi);
556+
eraseInstruction(dvi);
557+
return true;
558+
}
503559

504560
// Now that we have set the unqualified ownership flag,
505561
// emitDestroyValueOperation will insert the appropriate instruction.
@@ -556,6 +612,7 @@ bool OwnershipModelEliminatorVisitor::visitSwitchEnumInst(
556612
return true;
557613
}
558614

615+
// See also splitDestroy().
559616
void OwnershipModelEliminatorVisitor::splitDestructure(
560617
SILInstruction *destructureInst, SILValue destructureOperand) {
561618
assert((isa<DestructureStructInst>(destructureInst) ||
@@ -664,12 +721,11 @@ static bool stripOwnership(SILFunction &func) {
664721
arg->setOwnershipKind(OwnershipKind::None);
665722
}
666723

667-
for (auto ii = block.begin(), ie = block.end(); ii != ie;) {
668-
// Since we are going to be potentially removing instructions, we need
669-
// to make sure to increment our iterator before we perform any
670-
// visits.
724+
// This loop may erase instructions and split basic blocks.
725+
for (auto ii = block.begin(); ii != block.end(); ++ii) {
671726
SILInstruction *inst = &*ii;
672-
++ii;
727+
if (inst->isDeleted())
728+
continue;
673729

674730
madeChange |= visitor.visit(inst);
675731
}
@@ -689,6 +745,12 @@ static bool stripOwnership(SILFunction &func) {
689745
auto value = visitor.instructionsToSimplify.pop_back_val();
690746
if (!value.has_value())
691747
continue;
748+
749+
if (auto dropDeinit = dyn_cast<DropDeinitInst>(*value)) {
750+
visitor.eraseInstructionAndRAUW(dropDeinit, dropDeinit->getOperand());
751+
madeChange = true;
752+
continue;
753+
}
692754
auto callbacks =
693755
InstModCallbacks().onDelete([&](SILInstruction *instToErase) {
694756
visitor.eraseInstruction(instToErase);

0 commit comments

Comments
 (0)