Skip to content

Commit 476f401

Browse files
committed
[pmo] NFC. Split addMissingDestroysForCopiedValues into a version for load_borrow and one for load.
This is a pure refactor so I can make some subsequent transformations easier to do. I also did a little bit of debridement when there were opportunities to flatten control flow by eliminating direct checks for one or the other classes.
1 parent f5f214d commit 476f401

File tree

1 file changed

+87
-30
lines changed

1 file changed

+87
-30
lines changed

lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp

Lines changed: 87 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -427,9 +427,11 @@ class AvailableValueAggregator {
427427
/// If as a result of us copying values, we may have unconsumed destroys, find
428428
/// the appropriate location and place the values there. Only used when
429429
/// ownership is enabled.
430-
SingleValueInstruction *
431-
addMissingDestroysForCopiedValues(SingleValueInstruction *li,
432-
SILValue newVal);
430+
SingleValueInstruction *addMissingDestroysForCopiedValues(LoadBorrowInst *li,
431+
SILValue newVal);
432+
433+
SingleValueInstruction *addMissingDestroysForCopiedValues(LoadInst *li,
434+
SILValue newVal);
433435

434436
void print(llvm::raw_ostream &os) const;
435437
void dump() const LLVM_ATTRIBUTE_USED;
@@ -751,14 +753,78 @@ SILValue AvailableValueAggregator::handlePrimitiveValue(SILType loadTy,
751753
}
752754

753755
SingleValueInstruction *
754-
AvailableValueAggregator::addMissingDestroysForCopiedValues(
755-
SingleValueInstruction *svi, SILValue newVal) {
756+
AvailableValueAggregator::addMissingDestroysForCopiedValues(LoadInst *li,
757+
SILValue newVal) {
756758
assert(B.hasOwnership() &&
757759
"We assume this is only called if we have ownership");
758760

759-
assert((isa<LoadBorrowInst>(svi) || isa<LoadInst>(svi)) &&
760-
"Expected to have a /real/ load here since we assume that we have a "
761-
"unary operand instruction");
761+
SmallPtrSet<SILBasicBlock *, 8> visitedBlocks;
762+
SmallVector<SILBasicBlock *, 8> leakingBlocks;
763+
bool foundLoop = false;
764+
auto loc = RegularLocation::getAutoGeneratedLocation();
765+
while (!insertedInsts.empty()) {
766+
auto *cvi = dyn_cast<CopyValueInst>(insertedInsts.pop_back_val());
767+
if (!cvi)
768+
continue;
769+
770+
// Clear our state.
771+
visitedBlocks.clear();
772+
leakingBlocks.clear();
773+
// The linear lifetime checker doesn't care if the passed in load is
774+
// actually a user of our copy_value. What we care about is that the load is
775+
// guaranteed to be in the block where we have reformed the tuple in a
776+
// consuming manner. This means if we add it as the consuming use of the
777+
// copy, we can find the leaking places if any exist.
778+
//
779+
// Then perform the linear lifetime check. If we succeed, continue. We have
780+
// no further work to do.
781+
auto errorKind = ownership::ErrorBehaviorKind::ReturnFalse;
782+
LinearLifetimeChecker checker(visitedBlocks, deadEndBlocks);
783+
auto error = checker.checkValue(
784+
cvi, {BranchPropagatedUser(&li->getAllOperands()[0])}, {}, errorKind,
785+
&leakingBlocks);
786+
if (!error.getFoundError())
787+
continue;
788+
789+
// Ok, we found some leaking blocks. Since we are using the linear lifetime
790+
// checker with memory, we do not have any guarantees that the store is out
791+
// side of a loop and a load is in a loop. In such a case, we want to
792+
// replace the load with a copy_value.
793+
foundLoop |= error.getFoundOverConsume();
794+
795+
// Ok, we found some leaking blocks. Insert destroys at the
796+
// beginning of these blocks for our copy_value.
797+
for (auto *bb : leakingBlocks) {
798+
SILBuilderWithScope b(bb->begin());
799+
b.emitDestroyValueOperation(loc, cvi);
800+
}
801+
}
802+
803+
// If we didn't find a loop, we are done, just return svi to get RAUWed.
804+
if (!foundLoop) {
805+
return li;
806+
}
807+
808+
// If we found a loop, then we know that our leaking blocks are the exiting
809+
// blocks of the loop and the value has been lifetime extended over the loop.
810+
811+
// If we have a load, we need to put in a copy so that the destroys within
812+
// the loop are properly balanced.
813+
newVal = SILBuilderWithScope(li).emitCopyValueOperation(loc, newVal);
814+
815+
li->replaceAllUsesWith(newVal);
816+
SILValue addr = li->getOperand();
817+
li->eraseFromParent();
818+
if (auto *addrI = addr->getDefiningInstruction())
819+
recursivelyDeleteTriviallyDeadInstructions(addrI);
820+
return nullptr;
821+
}
822+
823+
SingleValueInstruction *
824+
AvailableValueAggregator::addMissingDestroysForCopiedValues(LoadBorrowInst *lbi,
825+
SILValue newVal) {
826+
assert(B.hasOwnership() &&
827+
"We assume this is only called if we have ownership");
762828

763829
SmallPtrSet<SILBasicBlock *, 8> visitedBlocks;
764830
SmallVector<SILBasicBlock *, 8> leakingBlocks;
@@ -783,7 +849,7 @@ AvailableValueAggregator::addMissingDestroysForCopiedValues(
783849
auto errorKind = ownership::ErrorBehaviorKind::ReturnFalse;
784850
LinearLifetimeChecker checker(visitedBlocks, deadEndBlocks);
785851
auto error = checker.checkValue(
786-
cvi, {BranchPropagatedUser(&svi->getAllOperands()[0])}, {}, errorKind,
852+
cvi, {BranchPropagatedUser(&lbi->getAllOperands()[0])}, {}, errorKind,
787853
&leakingBlocks);
788854
if (!error.getFoundError())
789855
continue;
@@ -808,34 +874,25 @@ AvailableValueAggregator::addMissingDestroysForCopiedValues(
808874
// to borrow at the load point. This means we need to handle the destroying
809875
// of the value along paths reachable from the load_borrow. Luckily that
810876
// will exactly be after the end_borrows of the load_borrow.
811-
if (isa<LoadBorrowInst>(svi)) {
812-
for (auto *use : svi->getUses()) {
813-
if (auto *ebi = dyn_cast<EndBorrowInst>(use->getUser())) {
814-
auto next = std::next(ebi->getIterator());
815-
SILBuilderWithScope(next).emitDestroyValueOperation(ebi->getLoc(),
816-
newVal);
817-
}
877+
for (auto *use : lbi->getUses()) {
878+
if (auto *ebi = dyn_cast<EndBorrowInst>(use->getUser())) {
879+
auto next = std::next(ebi->getIterator());
880+
SILBuilderWithScope(next).emitDestroyValueOperation(ebi->getLoc(),
881+
newVal);
818882
}
819883
}
820-
return svi;
884+
return lbi;
821885
}
822886

823887
// If we found a loop, then we know that our leaking blocks are the exiting
824888
// blocks of the loop and the value has been lifetime extended over the loop.
825-
if (isa<LoadInst>(svi)) {
826-
// If we have a load, we need to put in a copy so that the destroys within
827-
// the loop are properly balanced.
828-
newVal = SILBuilderWithScope(svi).emitCopyValueOperation(loc, newVal);
829-
} else {
830-
// If we have a load_borrow, we create a begin_borrow for the end_borrows in
831-
// the loop.
832-
assert(isa<LoadBorrowInst>(svi));
833-
newVal = SILBuilderWithScope(svi).createBeginBorrow(svi->getLoc(), newVal);
834-
}
889+
// If we have a load_borrow, we create a begin_borrow for the end_borrows in
890+
// the loop.
891+
newVal = SILBuilderWithScope(lbi).createBeginBorrow(lbi->getLoc(), newVal);
835892

836-
svi->replaceAllUsesWith(newVal);
837-
SILValue addr = svi->getOperand(0);
838-
svi->eraseFromParent();
893+
lbi->replaceAllUsesWith(newVal);
894+
SILValue addr = lbi->getOperand();
895+
lbi->eraseFromParent();
839896
if (auto *addrI = addr->getDefiningInstruction())
840897
recursivelyDeleteTriviallyDeadInstructions(addrI);
841898
return nullptr;

0 commit comments

Comments
 (0)