Skip to content

Commit 6051885

Browse files
authored
Merge pull request #65780 from meg-gupta/paonstackfix
Fix insertion of dealloc_stack for partial_apply [on_stack] while going to non-OSSA
2 parents f1d2740 + b19e10c commit 6051885

File tree

2 files changed

+51
-10
lines changed

2 files changed

+51
-10
lines changed

lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -485,19 +485,11 @@ bool OwnershipModelEliminatorVisitor::visitDestroyValueInst(
485485
DestroyValueInst *dvi) {
486486
// Nonescaping closures are represented ultimately as trivial pointers to
487487
// their context, but we use ownership to do borrow checking of their captures
488-
// in OSSA. Now that we're eliminating ownership, fold away destroys, unless
489-
// we're destroying the original partial_apply, in which case this is where
490-
// we dealloc_stack the context.
488+
// in OSSA. Now that we're eliminating ownership, fold away destroys.
491489
auto operand = dvi->getOperand();
492490
auto operandTy = operand->getType();
493491
if (auto operandFnTy = operandTy.getAs<SILFunctionType>()){
494492
if (operandFnTy->isTrivialNoEscape()) {
495-
if (auto origPA = dvi->getNonescapingClosureAllocation()) {
496-
withBuilder<void>(dvi, [&](SILBuilder &b, SILLocation loc) {
497-
b.createDeallocStack(loc, origPA);
498-
});
499-
}
500-
501493
eraseInstruction(dvi);
502494
return true;
503495
}
@@ -627,9 +619,41 @@ static bool stripOwnership(SILFunction &func) {
627619
if (func.isExternalDeclaration())
628620
return false;
629621

622+
llvm::DenseMap<PartialApplyInst *, SmallVector<SILInstruction *>>
623+
lifetimeEnds;
624+
625+
// Nonescaping closures are represented ultimately as trivial pointers to
626+
// their context, but we use ownership to do borrow checking of their captures
627+
// in OSSA. Now that we're eliminating ownership, we need to dealloc_stack the
628+
// context at its lifetime ends.
629+
// partial_apply's lifetime ends has to be gathered before we begin to leave
630+
// OSSA, but no dealloc_stack can be emitted until after we leave OSSA.
631+
for (auto &block : func) {
632+
for (auto &ii : block) {
633+
auto *pai = dyn_cast<PartialApplyInst>(&ii);
634+
if (!pai || !pai->isOnStack()) {
635+
continue;
636+
}
637+
pai->visitOnStackLifetimeEnds([&](Operand *op) {
638+
lifetimeEnds[pai].push_back(op->getUser());
639+
return true;
640+
});
641+
}
642+
}
643+
630644
// Set F to have unqualified ownership.
631645
func.setOwnershipEliminated();
632646

647+
// Now that we are in non-ossa, create dealloc_stack at partial_apply's
648+
// lifetime ends
649+
for (auto &it : lifetimeEnds) {
650+
auto *pai = it.first;
651+
for (auto *lifetimeEnd : it.second) {
652+
SILBuilderWithScope(lifetimeEnd->getNextInstruction())
653+
.createDeallocStack(lifetimeEnd->getLoc(), pai);
654+
}
655+
}
656+
633657
bool madeChange = false;
634658
SmallVector<SILInstruction *, 32> createdInsts;
635659
OwnershipModelEliminatorVisitor visitor(func);

test/SILOptimizer/ownership_model_eliminator.sil

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,4 +393,21 @@ bb0(%0 : $*C):
393393
dealloc_stack %1 : $*C
394394
%9999 = tuple()
395395
return %9999 : $()
396-
}
396+
}
397+
398+
sil @closure1: $@convention(thin) (@guaranteed C, @inout_aliasable C) -> ()
399+
sil @closure2 : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> ()) -> ()
400+
401+
// Ensure no assertion about missing dealloc_stack for partial_apply [on_stack] after OME
402+
sil [ossa] @test_partial_apply_on_stack: $@convention(thin) (@guaranteed C, @inout C) -> () {
403+
bb0(%0 : @guaranteed $C, %1 : $*C):
404+
%f1 = function_ref @closure1: $@convention(thin) (@guaranteed C, @inout_aliasable C) -> ()
405+
%pa1 = partial_apply [callee_guaranteed] [on_stack] %f1(%0, %1) : $@convention(thin) (@guaranteed C, @inout_aliasable C) -> ()
406+
%md = mark_dependence %pa1 : $@noescape @callee_guaranteed () -> () on %1 : $*C
407+
%f2 = function_ref @closure2 : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> ()) -> ()
408+
%pa2 = partial_apply [callee_guaranteed] %f2(%md) : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> ()) -> ()
409+
destroy_value %pa2 : $@callee_guaranteed () -> ()
410+
%t = tuple ()
411+
return %t : $()
412+
}
413+

0 commit comments

Comments
 (0)