Skip to content

Commit a336bcd

Browse files
committed
Fix findTransitiveUses to only record leaf uses.
Recording uses is now optional. This utility is also used simply to check for PointerEscape. When uses are recorded, they are used to extend a live range. There could be a large number of transitive uses that we don't want to pass back to the caller.
1 parent 1678354 commit a336bcd

File tree

5 files changed

+58
-45
lines changed

5 files changed

+58
-45
lines changed

include/swift/SIL/OwnershipUtils.h

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,9 @@ inline bool isForwardingConsume(SILValue value) {
7777
return canOpcodeForwardOwnedValues(value);
7878
}
7979

80-
/// Find all "use points" of \p guaranteedValue that determine its lifetime
81-
/// requirement.
80+
/// Find leaf "use points" of \p guaranteedValue that determine its lifetime
81+
/// requirement. If \p usePoints is nullptr, then the simply returns true if no
82+
/// PointerEscape use was found.
8283
///
8384
/// Precondition: \p guaranteedValue is not a BorrowedValue.
8485
///
@@ -99,10 +100,10 @@ inline bool isForwardingConsume(SILValue value) {
99100
/// When this is called on a value that does not introduce a new scope, none of
100101
/// the use points can be EndBorrows or Reborrows. Those uses are only allowed
101102
/// on borrow-introducing values.
102-
bool findInnerTransitiveGuaranteedUses(SILValue guaranteedValue,
103-
SmallVectorImpl<Operand *> &usePoints);
103+
bool findInnerTransitiveGuaranteedUses(
104+
SILValue guaranteedValue, SmallVectorImpl<Operand *> *usePoints = nullptr);
104105

105-
/// Find all "use points" of a guaranteed value within its enclosing borrow
106+
/// Find leaf "use points" of a guaranteed value within its enclosing borrow
106107
/// scope (without looking through reborrows). To find the use points of the
107108
/// extended borrow scope, after looking through reborrows, use
108109
/// findExtendedTransitiveGuaranteedUses() instead.
@@ -666,7 +667,7 @@ BorrowedValue getSingleBorrowIntroducingValue(SILValue inputValue);
666667
/// be transitive uses of the given address. Used to implement \see
667668
/// findTransitiveUses.
668669
bool findTransitiveUsesForAddress(
669-
SILValue address, SmallVectorImpl<Operand *> &foundUses,
670+
SILValue address, SmallVectorImpl<Operand *> *foundUses = nullptr,
670671
std::function<void(Operand *)> *onError = nullptr);
671672

672673
class InteriorPointerOperandKind {
@@ -834,14 +835,17 @@ struct InteriorPointerOperand {
834835
llvm_unreachable("Covered switch isn't covered?!");
835836
}
836837

837-
/// Transitively compute the list of uses that this interior pointer operand
838-
/// puts on its parent guaranted value.
838+
/// Transitively compute the list of leaf uses that this interior pointer
839+
/// operand puts on its parent guaranted value.
840+
///
841+
/// If \p foundUses is nullptr, this simply returns true if no PointerEscapes
842+
/// were found.
839843
///
840844
/// Example: Uses of a ref_element_addr can not occur outside of the lifetime
841845
/// of the instruction's operand. The uses of that address act as liveness
842846
/// requirements to ensure that the underlying class is alive at all use
843847
/// points.
844-
bool findTransitiveUses(SmallVectorImpl<Operand *> &foundUses,
848+
bool findTransitiveUses(SmallVectorImpl<Operand *> *foundUses = nullptr,
845849
std::function<void(Operand *)> *onError = nullptr) {
846850
return findTransitiveUsesForAddress(getProjectedAddress(), foundUses,
847851
onError);
@@ -913,7 +917,7 @@ struct AddressOwnership {
913917

914918
/// Transitively compute uses of this base address.
915919
bool findTransitiveUses(SmallVectorImpl<Operand *> &foundUses) {
916-
return findTransitiveUsesForAddress(base.getBaseAddress(), foundUses);
920+
return findTransitiveUsesForAddress(base.getBaseAddress(), &foundUses);
917921
}
918922

919923
/// Return true of all \p uses occur before the end of the address' lifetime

lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include "swift/SIL/OwnershipUtils.h"
1414
#include "swift/Basic/Defer.h"
15+
#include "swift/Basic/DAGNodeWorklist.h"
1516
#include "swift/Basic/SmallPtrSetVector.h"
1617
#include "swift/SIL/InstructionUtils.h"
1718
#include "swift/SIL/LinearLifetimeChecker.h"
@@ -160,35 +161,33 @@ bool swift::canOpcodeForwardOwnedValues(Operand *use) {
160161
// points. Transitively find all nested scope-ending instructions by looking
161162
// through nested reborrows. Nested reborrows are not use points and \p
162163
// visitReborrow is not called for them.
163-
bool swift::
164-
findInnerTransitiveGuaranteedUses(SILValue guaranteedValue,
165-
SmallVectorImpl<Operand *> &usePoints) {
164+
bool swift::findInnerTransitiveGuaranteedUses(
165+
SILValue guaranteedValue, SmallVectorImpl<Operand *> *usePoints) {
166166
// Push the value's immediate uses.
167-
unsigned firstOffset = usePoints.size();
167+
//
168+
// TODO: The worklist can be a simple vector without any a membership check if
169+
// destructures are changed to be represented as reborrows. Currently a
170+
// destructure forwards multiple results! This means that usePoints could grow
171+
// exponentially without the membership check. It's fine to do this membership
172+
// check locally in this function (within a borrow scope) because it isn't
173+
// needed for the immediate uses, only the transitive uses.
174+
DAGNodeWorklist<Operand *, 8> worklist;
168175
for (Operand *use : guaranteedValue->getUses()) {
169176
if (use->getOperandOwnership() != OperandOwnership::NonUse)
170-
usePoints.push_back(use);
177+
worklist.insert(use);
171178
}
172179

173180
// --- Transitively follow forwarded uses and look for escapes.
174181

175-
// TODO: Remove this SmallPtrSet if destructures are changed to be represented
176-
// as reborrows. Currently it forwards multiple results! This means that
177-
// usePoints could grow exponentially without a membership check. It's fine to
178-
// do this membership check locally in this function (within a borrow
179-
// scope). It isn't needed for the immediate uses, only the transitive uses.
180-
SmallPtrSet<Operand *, 16> visitedUses;
181-
auto pushUse = [&](Operand *use) {
182-
if (use->getOperandOwnership() != OperandOwnership::NonUse) {
183-
if (visitedUses.insert(use).second)
184-
usePoints.push_back(use);
182+
auto recordLeafUse = [&](Operand *use) {
183+
if (usePoints && use->getOperandOwnership() != OperandOwnership::NonUse) {
184+
usePoints->push_back(use);
185185
}
186186
return true;
187187
};
188188

189189
// usePoints grows in this loop.
190-
for (unsigned i = firstOffset; i < usePoints.size(); ++i) {
191-
Operand *use = usePoints[i];
190+
while (Operand *use = worklist.pop()) {
192191
switch (use->getOperandOwnership()) {
193192
case OperandOwnership::NonUse:
194193
case OperandOwnership::TrivialUse:
@@ -228,14 +227,17 @@ findInnerTransitiveGuaranteedUses(SILValue guaranteedValue,
228227
if (transitiveValue.getOwnershipKind() == OwnershipKind::None)
229228
return true;
230229
for (auto *transitiveUse : transitiveValue->getUses()) {
231-
pushUse(transitiveUse);
230+
if (transitiveUse->getOperandOwnership()
231+
!= OperandOwnership::NonUse) {
232+
worklist.insert(use);
233+
}
232234
}
233235
return true;
234236
});
235237
break;
236238

237239
case OperandOwnership::Borrow:
238-
BorrowingOperand(use).visitExtendedScopeEndingUses(pushUse);
240+
BorrowingOperand(use).visitExtendedScopeEndingUses(recordLeafUse);
239241
}
240242
}
241243
return true;
@@ -271,7 +273,7 @@ bool swift::findTransitiveGuaranteedUses(
271273
}
272274
return true;
273275
}
274-
return findInnerTransitiveGuaranteedUses(guaranteedValue, usePoints);
276+
return findInnerTransitiveGuaranteedUses(guaranteedValue, &usePoints);
275277
}
276278

277279
// Find all use points of \p guaranteedValue within its borrow scope. If the
@@ -705,23 +707,24 @@ bool BorrowedValue::visitInteriorPointerOperandHelper(
705707
}
706708

707709
bool swift::findTransitiveUsesForAddress(
708-
SILValue projectedAddress, SmallVectorImpl<Operand *> &foundUses,
710+
SILValue projectedAddress, SmallVectorImpl<Operand *> *foundUses,
709711
std::function<void(Operand *)> *onError) {
710712
SmallVector<Operand *, 8> worklist(projectedAddress->getUses());
711713

712714
bool foundError = false;
713715

716+
auto leafUse = [foundUses](Operand *use) {
717+
if (foundUses)
718+
foundUses->push_back(use);
719+
};
720+
714721
while (!worklist.empty()) {
715722
auto *op = worklist.pop_back_val();
716723

717724
// Skip type dependent operands.
718725
if (op->isTypeDependent())
719726
continue;
720727

721-
// Before we do anything, add this operand to our implicit regular user
722-
// list.
723-
foundUses.push_back(op);
724-
725728
// Then update the worklist with new things to find if we recognize this
726729
// inst and then continue. If we fail, we emit an error at the bottom of the
727730
// loop that we didn't recognize the user.
@@ -741,6 +744,7 @@ bool swift::findTransitiveUsesForAddress(
741744
isa<SwitchEnumAddrInst>(user) || isa<CheckedCastAddrBranchInst>(user) ||
742745
isa<SelectEnumAddrInst>(user) || isa<InjectEnumAddrInst>(user) ||
743746
isa<IsUniqueInst>(user)) {
747+
leafUse(op);
744748
continue;
745749
}
746750

@@ -763,33 +767,39 @@ bool swift::findTransitiveUsesForAddress(
763767
if (auto *builtin = dyn_cast<BuiltinInst>(user)) {
764768
if (auto kind = builtin->getBuiltinKind()) {
765769
if (*kind == BuiltinValueKind::TSanInoutAccess) {
770+
leafUse(op);
766771
continue;
767772
}
768773
}
769774
}
770775

771776
// If we have a load_borrow, add it's end scope to the liveness requirement.
772777
if (auto *lbi = dyn_cast<LoadBorrowInst>(user)) {
773-
transform(lbi->getEndBorrows(), std::back_inserter(foundUses),
774-
[](EndBorrowInst *ebi) { return &ebi->getAllOperands()[0]; });
778+
if (foundUses) {
779+
transform(lbi->getEndBorrows(), std::back_inserter(*foundUses),
780+
[](EndBorrowInst *ebi) { return &ebi->getAllOperands()[0]; });
781+
}
775782
continue;
776783
}
777784

778785
// TODO: Merge this into the full apply site code below.
779786
if (auto *beginApply = dyn_cast<BeginApplyInst>(user)) {
780-
// TODO: Just add this to implicit regular user list?
781-
llvm::copy(beginApply->getTokenResult()->getUses(),
782-
std::back_inserter(foundUses));
787+
if (foundUses) {
788+
llvm::copy(beginApply->getTokenResult()->getUses(),
789+
std::back_inserter(*foundUses));
790+
}
783791
continue;
784792
}
785793

786794
if (auto fas = FullApplySite::isa(user)) {
795+
leafUse(op);
787796
continue;
788797
}
789798

790799
if (auto *mdi = dyn_cast<MarkDependenceInst>(user)) {
791800
// If this is the base, just treat it as a liveness use.
792801
if (op->get() == mdi->getBase()) {
802+
leafUse(op);
793803
continue;
794804
}
795805

lib/SIL/Verifier/SILOwnershipVerifier.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ bool SILValueOwnershipChecker::gatherUsers(
377377
});
378378
};
379379
foundError |= !interiorPointerOperand.findTransitiveUses(
380-
nonLifetimeEndingUsers, &onError);
380+
&nonLifetimeEndingUsers, &onError);
381381
}
382382

383383
// Finally add the op to the non lifetime ending user list.

lib/SILOptimizer/Utils/BasicBlockOptUtils.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,7 @@ static bool canBorrowGuaranteedResult(SILValue guaranteedResult) {
100100
// conversion to a non-guaranteed value. Either way, not interesting.
101101
return true;
102102
}
103-
SmallVector<Operand *, 16> usePoints;
104-
return findInnerTransitiveGuaranteedUses(guaranteedResult, usePoints);
103+
return findInnerTransitiveGuaranteedUses(guaranteedResult);
105104
}
106105

107106
bool swift::canCloneTerminator(TermInst *termInst) {

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,7 +1150,7 @@ OwnershipRAUWHelper::OwnershipRAUWHelper(OwnershipFixupContext &inputCtx,
11501150
auto &oldValueUses = ctx->extraAddressFixupInfo.allAddressUsesFromOldValue;
11511151
// FIXME: The return value of findTransitiveUsesForAddress is currently
11521152
// inverted.
1153-
if (findTransitiveUsesForAddress(oldValue, oldValueUses)) {
1153+
if (!findTransitiveUsesForAddress(oldValue, &oldValueUses)) {
11541154
invalidate();
11551155
return;
11561156
}
@@ -1520,7 +1520,7 @@ void GuaranteedPhiBorrowFixup::insertEndBorrowsAndFindPhis(
15201520
return;
15211521
}
15221522
SmallVector<Operand *, 16> usePoints;
1523-
bool result = findInnerTransitiveGuaranteedUses(phi, usePoints);
1523+
bool result = findInnerTransitiveGuaranteedUses(phi, &usePoints);
15241524
assert(result && "should be checked by canCloneTerminator");
15251525
(void)result;
15261526

0 commit comments

Comments
 (0)