Skip to content

Commit 2e9c6c7

Browse files
atrickatrick
committed
Fix visitNonEscapingLifetimeEnds to handle mark_dependence uses
Now it visits unknown uses separately rather than asserting. (cherry picked from commit d7b9149)
1 parent 1b62e78 commit 2e9c6c7

File tree

11 files changed

+112
-70
lines changed

11 files changed

+112
-70
lines changed

‎SwiftCompilerSources/Sources/Optimizer/Utilities/BorrowUtils.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,11 @@ enum BorrowingInstruction : CustomStringConvertible, Hashable {
206206
/// incoming value dominates or is consumed by an outer adjacent
207207
/// phi. See InteriorLiveness.
208208
///
209+
/// FIXME: To generate conservatively correct liveness, this should return
210+
/// .abortWalk if this is a mark_dependence and the scope-ending use is not
211+
/// the last in the function (e.g. a store rather than a destroy or return).
212+
/// The client needs to use LifetimeDependenceDefUseWalker to do better.
213+
///
209214
/// TODO: to hande reborrow-extended uses, migrate ExtendedLiveness
210215
/// to SwiftCompilerSources.
211216
///

‎include/swift/SIL/OwnershipUseVisitor.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,13 @@ bool OwnershipUseVisitor<Impl>::visitInnerBorrow(Operand *borrowingOperand) {
258258
return false;
259259

260260
return BorrowingOperand(borrowingOperand)
261-
.visitScopeEndingUses([&](Operand *borrowEnd) {
262-
return visitInnerBorrowScopeEnd(borrowEnd);
263-
});
261+
.visitScopeEndingUses(
262+
[&](Operand *borrowEnd) {
263+
return visitInnerBorrowScopeEnd(borrowEnd);
264+
},
265+
[&](Operand *unknownUse) {
266+
return asImpl().handlePointerEscape(unknownUse);
267+
});
264268
}
265269

266270
template <typename Impl>

‎include/swift/SIL/OwnershipUtils.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,12 +360,16 @@ struct BorrowingOperand {
360360
/// over a region of code instead of just for a single instruction, visit
361361
/// those uses.
362362
///
363-
/// Returns false and early exits if the visitor \p func returns false.
363+
/// Returns false and early exits if the \p visitScopeEnd or \p
364+
/// visitUnknownUse returns false.
364365
///
365366
/// For an instantaneous borrow, such as apply, this visits no uses. For
366367
/// begin_apply it visits the end_apply uses. For borrow introducers, it
367368
/// visits the end of the introduced borrow scope.
368-
bool visitScopeEndingUses(function_ref<bool(Operand *)> func) const;
369+
bool visitScopeEndingUses(function_ref<bool(Operand *)> visitScopeEnd,
370+
function_ref<bool(Operand *)> visitUnknownUse
371+
= [](Operand *){ return false; })
372+
const;
369373

370374
/// Returns true for borrows that create a local borrow scope but have no
371375
/// scope-ending uses (presumably all paths are dead-end blocks). This does
@@ -381,7 +385,10 @@ struct BorrowingOperand {
381385
/// BorrowingOperand.
382386
///
383387
/// Returns false and early exits if the visitor \p func returns false.
384-
bool visitExtendedScopeEndingUses(function_ref<bool(Operand *)> func) const;
388+
bool visitExtendedScopeEndingUses(
389+
function_ref<bool(Operand *)> func,
390+
function_ref<bool(Operand *)> visitUnknownUse
391+
= [](Operand *){ return false; }) const;
385392

386393
/// Returns true if this borrow scope operand consumes guaranteed
387394
/// values and produces a new scope afterwards.

‎include/swift/SIL/PrunedLiveness.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,9 @@ class PrunedLiveBlocks {
289289
/// scope. Reborrows within nested borrows scopes are already summarized by the
290290
/// outer borrow scope.
291291
enum class InnerBorrowKind {
292-
Contained, // any borrows are fully contained within this live range
293-
Reborrowed // at least one immediately nested borrow is reborrowed
292+
Contained, // any borrows are fully contained within this live range
293+
Reborrowed, // at least one immediately nested borrow is reborrowed
294+
Escaped // the end of the borrow scope is indeterminate
294295
};
295296

296297
inline InnerBorrowKind meet(InnerBorrowKind lhs, InnerBorrowKind rhs) {

‎include/swift/SIL/SILInstruction.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8546,8 +8546,9 @@ class MarkDependenceInst
85468546
}
85478547

85488548
/// Visit the instructions that end the lifetime of an OSSA on-stack closure.
8549-
bool visitNonEscapingLifetimeEnds(llvm::function_ref<bool (Operand*)> func)
8550-
const;
8549+
bool visitNonEscapingLifetimeEnds(
8550+
llvm::function_ref<bool (Operand*)> visitScopeEnd,
8551+
llvm::function_ref<bool (Operand*)> visitUnknownUse) const;
85518552
};
85528553

85538554
/// Promote an Objective-C block that is on the stack to the heap, or simply

‎lib/SIL/IR/SILInstruction.cpp

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1792,23 +1792,24 @@ bool SILInstruction::maySuspend() const {
17921792
return false;
17931793
}
17941794

1795-
static bool visitRecursivelyLifetimeEndingUses(
1796-
SILValue i,
1797-
bool &noUsers,
1798-
llvm::function_ref<bool(Operand *)> func)
1799-
{
1795+
static bool
1796+
visitRecursivelyLifetimeEndingUses(
1797+
SILValue i, bool &noUsers,
1798+
llvm::function_ref<bool(Operand *)> visitScopeEnd,
1799+
llvm::function_ref<bool(Operand *)> visitUnknownUse) {
1800+
18001801
for (Operand *use : i->getConsumingUses()) {
18011802
noUsers = false;
18021803
if (isa<DestroyValueInst>(use->getUser())) {
1803-
if (!func(use)) {
1804+
if (!visitScopeEnd(use)) {
18041805
return false;
18051806
}
18061807
continue;
18071808
}
18081809
if (auto *ret = dyn_cast<ReturnInst>(use->getUser())) {
18091810
auto fnTy = ret->getFunction()->getLoweredFunctionType();
18101811
assert(!fnTy->getLifetimeDependenceInfo().empty());
1811-
if (!func(use)) {
1812+
if (!visitScopeEnd(use)) {
18121813
return false;
18131814
}
18141815
continue;
@@ -1826,16 +1827,11 @@ static bool visitRecursivelyLifetimeEndingUses(
18261827
// the structural requirements of on-stack partial_apply uses.
18271828
auto *user = use->getUser();
18281829
if (user->getNumResults() == 0) {
1829-
llvm::errs() << "partial_apply [on_stack] use:\n";
1830-
user->printInContext(llvm::errs());
1831-
if (isa<BranchInst>(user)) {
1832-
llvm::report_fatal_error("partial_apply [on_stack] cannot be cloned");
1833-
}
1834-
llvm::report_fatal_error("partial_apply [on_stack] must be directly "
1835-
"forwarded to a destroy_value");
1830+
return visitUnknownUse(use);
18361831
}
18371832
for (auto res : use->getUser()->getResults()) {
1838-
if (!visitRecursivelyLifetimeEndingUses(res, noUsers, func)) {
1833+
if (!visitRecursivelyLifetimeEndingUses(res, noUsers, visitScopeEnd,
1834+
visitUnknownUse)) {
18391835
return false;
18401836
}
18411837
}
@@ -1851,19 +1847,36 @@ PartialApplyInst::visitOnStackLifetimeEnds(
18511847
&& "only meaningful for OSSA stack closures");
18521848
bool noUsers = true;
18531849

1854-
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func)) {
1850+
auto visitUnknownUse = [](Operand *unknownUse){
1851+
llvm::errs() << "partial_apply [on_stack] use:\n";
1852+
auto *user = unknownUse->getUser();
1853+
user->printInContext(llvm::errs());
1854+
if (isa<BranchInst>(user)) {
1855+
llvm::report_fatal_error("partial_apply [on_stack] cannot be cloned");
1856+
}
1857+
llvm::report_fatal_error("partial_apply [on_stack] must be directly "
1858+
"forwarded to a destroy_value");
1859+
return false;
1860+
};
1861+
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func,
1862+
visitUnknownUse)) {
18551863
return false;
18561864
}
18571865
return !noUsers;
18581866
}
18591867

1860-
bool MarkDependenceInst::
1861-
visitNonEscapingLifetimeEnds(llvm::function_ref<bool (Operand *)> func) const {
1868+
// FIXME: Rather than recursing through all results, this should only recurse
1869+
// through ForwardingInstruction and OwnershipTransitionInstruction and the
1870+
// client should prove that any other uses cannot be upstream from a consume of
1871+
// the dependent value.
1872+
bool MarkDependenceInst::visitNonEscapingLifetimeEnds(
1873+
llvm::function_ref<bool (Operand *)> visitScopeEnd,
1874+
llvm::function_ref<bool (Operand *)> visitUnknownUse) const {
18621875
assert(getFunction()->hasOwnership() && isNonEscaping()
18631876
&& "only meaningful for nonescaping dependencies");
18641877
bool noUsers = true;
1865-
1866-
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func)) {
1878+
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, visitScopeEnd,
1879+
visitUnknownUse)) {
18671880
return false;
18681881
}
18691882
return !noUsers;

‎lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -327,13 +327,19 @@ bool swift::findInnerTransitiveGuaranteedUses(
327327
//
328328
// FIXME: visit[Extended]ScopeEndingUses can't return false here once dead
329329
// borrows are disallowed.
330-
if (!BorrowingOperand(use).visitScopeEndingUses([&](Operand *endUse) {
331-
if (endUse->getOperandOwnership() == OperandOwnership::Reborrow) {
330+
if (!BorrowingOperand(use).visitScopeEndingUses(
331+
[&](Operand *endUse) {
332+
if (endUse->getOperandOwnership() == OperandOwnership::Reborrow) {
333+
foundPointerEscape = true;
334+
}
335+
leafUse(endUse);
336+
return true;
337+
},
338+
[&](Operand *unknownUse) {
332339
foundPointerEscape = true;
333-
}
334-
leafUse(endUse);
335-
return true;
336-
})) {
340+
leafUse(unknownUse);
341+
return true;
342+
})) {
337343
// Special case for dead borrows. This is dangerous because clients
338344
// don't expect a begin_borrow to be in the use list.
339345
leafUse(use);
@@ -439,16 +445,12 @@ bool swift::findExtendedUsesOfSimpleBorrowedValue(
439445
break;
440446
}
441447
case OperandOwnership::Borrow:
442-
// FIXME: visitExtendedScopeEndingUses can't return false here once dead
443-
// borrows are disallowed.
444448
if (!BorrowingOperand(use).visitExtendedScopeEndingUses(
445449
[&](Operand *endUse) {
446450
recordUse(endUse);
447451
return true;
448452
})) {
449-
// Special case for dead borrows. This is dangerous because clients
450-
// don't expect a begin_borrow to be in the use list.
451-
recordUse(use);
453+
return false;
452454
}
453455
break;
454456
}
@@ -677,7 +679,8 @@ bool BorrowingOperand::hasEmptyRequiredEndingUses() const {
677679
}
678680

679681
bool BorrowingOperand::visitScopeEndingUses(
680-
function_ref<bool(Operand *)> func) const {
682+
function_ref<bool(Operand *)> visitScopeEnd,
683+
function_ref<bool(Operand *)> visitUnknownUse) const {
681684
switch (kind) {
682685
case BorrowingOperandKind::Invalid:
683686
llvm_unreachable("Using invalid case");
@@ -686,7 +689,7 @@ bool BorrowingOperand::visitScopeEndingUses(
686689
for (auto *use : cast<BeginBorrowInst>(op->getUser())->getUses()) {
687690
if (use->isLifetimeEnding()) {
688691
deadBorrow = false;
689-
if (!func(use))
692+
if (!visitScopeEnd(use))
690693
return false;
691694
}
692695
}
@@ -700,7 +703,7 @@ bool BorrowingOperand::visitScopeEndingUses(
700703
for (auto *use : cast<StoreBorrowInst>(op->getUser())->getUses()) {
701704
if (isa<EndBorrowInst>(use->getUser())) {
702705
deadBorrow = false;
703-
if (!func(use))
706+
if (!visitScopeEnd(use))
704707
return false;
705708
}
706709
}
@@ -714,7 +717,7 @@ bool BorrowingOperand::visitScopeEndingUses(
714717
auto *user = cast<BeginApplyInst>(op->getUser());
715718
for (auto *use : user->getTokenResult()->getUses()) {
716719
deadApply = false;
717-
if (!func(use))
720+
if (!visitScopeEnd(use))
718721
return false;
719722
}
720723
return !deadApply;
@@ -725,12 +728,12 @@ bool BorrowingOperand::visitScopeEndingUses(
725728
// The closure's borrow lifetimes end when the closure itself ends its
726729
// lifetime. That may happen transitively through conversions that forward
727730
// ownership of the closure.
728-
return user->visitOnStackLifetimeEnds(func);
731+
return user->visitOnStackLifetimeEnds(visitScopeEnd);
729732
}
730733
case BorrowingOperandKind::MarkDependenceNonEscaping: {
731734
auto *user = cast<MarkDependenceInst>(op->getUser());
732735
assert(user->isNonEscaping() && "escaping dependencies don't borrow");
733-
return user->visitNonEscapingLifetimeEnds(func);
736+
return user->visitNonEscapingLifetimeEnds(visitScopeEnd, visitUnknownUse);
734737
}
735738
case BorrowingOperandKind::BeginAsyncLet: {
736739
auto user = cast<BuiltinInst>(op->getUser());
@@ -743,7 +746,7 @@ bool BorrowingOperand::visitScopeEndingUses(
743746
|| builtinUser->getBuiltinKind() != BuiltinValueKind::EndAsyncLetLifetime)
744747
continue;
745748

746-
if (!func(use)) {
749+
if (!visitScopeEnd(use)) {
747750
return false;
748751
}
749752
}
@@ -761,7 +764,7 @@ bool BorrowingOperand::visitScopeEndingUses(
761764
for (auto *use : br->getArgForOperand(op)->getUses()) {
762765
if (use->isLifetimeEnding()) {
763766
deadBranch = false;
764-
if (!func(use))
767+
if (!visitScopeEnd(use))
765768
return false;
766769
}
767770
}
@@ -772,13 +775,14 @@ bool BorrowingOperand::visitScopeEndingUses(
772775
}
773776

774777
bool BorrowingOperand::visitExtendedScopeEndingUses(
775-
function_ref<bool(Operand *)> visitor) const {
778+
function_ref<bool(Operand *)> visitor,
779+
function_ref<bool(Operand *)> visitUnknownUse) const {
776780

777781
if (hasBorrowIntroducingUser()) {
778782
auto borrowedValue = getBorrowIntroducingUserResult();
779783
return borrowedValue.visitExtendedScopeEndingUses(visitor);
780784
}
781-
return visitScopeEndingUses(visitor);
785+
return visitScopeEndingUses(visitor, visitUnknownUse);
782786
}
783787

784788
BorrowedValue BorrowingOperand::getBorrowIntroducingUserResult() const {
@@ -840,10 +844,11 @@ void BorrowingOperand::getImplicitUses(
840844
SmallVectorImpl<Operand *> &foundUses) const {
841845
// FIXME: this visitScopeEndingUses should never return false once dead
842846
// borrows are disallowed.
843-
if (!visitScopeEndingUses([&](Operand *endOp) {
847+
auto handleUse = [&](Operand *endOp) {
844848
foundUses.push_back(endOp);
845849
return true;
846-
})) {
850+
};
851+
if (!visitScopeEndingUses(handleUse, handleUse)) {
847852
// Special-case for dead borrows.
848853
foundUses.push_back(op);
849854
}

‎lib/SIL/Utils/PrunedLiveness.cpp

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -234,16 +234,23 @@ PrunedLiveRange<LivenessWithDefs>::updateForBorrowingOperand(Operand *operand) {
234234
//
235235
// Note: Ownership liveness should follow reborrows that are dominated by the
236236
// ownership definition.
237-
if (!BorrowingOperand(operand).visitScopeEndingUses([this](Operand *end) {
238-
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
239-
return false;
240-
}
241-
updateForUse(end->getUser(), /*lifetimeEnding*/ false);
242-
return true;
243-
})) {
244-
return InnerBorrowKind::Reborrowed;
237+
auto innerBorrowKind = InnerBorrowKind::Contained;
238+
if (!BorrowingOperand(operand).visitScopeEndingUses(
239+
[&](Operand *end) {
240+
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
241+
innerBorrowKind = InnerBorrowKind::Reborrowed;
242+
}
243+
updateForUse(end->getUser(), /*lifetimeEnding*/ false);
244+
return true;
245+
}, [&](Operand *unknownUse) {
246+
updateForUse(unknownUse->getUser(), /*lifetimeEnding*/ false);
247+
innerBorrowKind = InnerBorrowKind::Escaped;
248+
return true;
249+
})) {
250+
// Handle dead borrows.
251+
updateForUse(operand->getUser(), /*lifetimeEnding*/ false);
245252
}
246-
return InnerBorrowKind::Contained;
253+
return innerBorrowKind;
247254
}
248255

249256
template <typename LivenessWithDefs>

‎lib/SILOptimizer/Utils/CanonicalizeBorrowScope.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -352,17 +352,15 @@ class FindBorrowScopeUses {
352352
// For borrows, record the scope-ending instructions to outer use
353353
// points. Note: The logic in filterOuterBorrowUseInsts that checks
354354
// whether a borrow scope is an outer use must visit the same set of uses.
355-
//
356-
// FIXME: visitExtendedScopeEndingUses can't return false here once dead
357-
// borrows are disallowed.
358355
if (!borrowingOper.visitExtendedScopeEndingUses([&](Operand *endBorrow) {
359356
auto *endInst = endBorrow->getUser();
360357
if (!isUserInLiveOutBlock(endInst)) {
361358
useInsts.insert(endInst);
362359
}
363360
return true;
364361
})) {
365-
useInsts.insert(user);
362+
// Bail out on dead borrow scopes and scopes with unknown uses.
363+
return false;
366364
}
367365
}
368366
return true;

‎test/SILOptimizer/copy_propagation.sil

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -841,10 +841,9 @@ bb4:
841841
//
842842
// CHECK-LABEL: sil hidden [ossa] @testDeadBorrow : {{.*}} {
843843
// CHECK: bb0:
844-
// CHECK: copy_value %1 : $C
845-
// CHECK: destroy_value
846-
// CHECK: copy_value %1 : $C
844+
// CHECK-NOT: copy_value
847845
// CHECK: begin_borrow
846+
// CHECK: destroy_value
848847
// CHECK: unreachable
849848
// CHECK-LABEL: } // end sil function 'testDeadBorrow'
850849
sil hidden [ossa] @testDeadBorrow : $@convention(thin) () -> () {

0 commit comments

Comments
 (0)