Skip to content

Commit be1ebe0

Browse files
committed
[SSADestroyHoisting] Recombine store [init]s.
Before hoisting destroy_addrs, we split store [assign]s into destroy_addrs and store [init]s. If those destroy_addrs were not able to be hoisted, though, recombine the two back into a store [assign]. Doing so avoids introducing extra ARC traffic.
1 parent ed1b3f2 commit be1ebe0

File tree

1 file changed

+52
-6
lines changed

1 file changed

+52
-6
lines changed

lib/SILOptimizer/Transforms/SSADestroyHoisting.cpp

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@ bool DeinitBarriers::DestroyReachability::checkReachablePhiBarrier(
494494
class HoistDestroys {
495495
SILValue storageRoot;
496496
bool ignoreDeinitBarriers;
497+
SmallPtrSetImpl<SILInstruction *> &remainingDestroyAddrs;
497498
InstructionDeleter &deleter;
498499

499500
// Book-keeping for the rewriting stage.
@@ -503,9 +504,11 @@ class HoistDestroys {
503504

504505
public:
505506
HoistDestroys(SILValue storageRoot, bool ignoreDeinitBarriers,
507+
SmallPtrSetImpl<SILInstruction *> &remainingDestroyAddrs,
506508
InstructionDeleter &deleter)
507509
: storageRoot(storageRoot), ignoreDeinitBarriers(ignoreDeinitBarriers),
508-
deleter(deleter), destroyMergeBlocks(getFunction()) {}
510+
remainingDestroyAddrs(remainingDestroyAddrs), deleter(deleter),
511+
destroyMergeBlocks(getFunction()) {}
509512

510513
bool perform();
511514

@@ -593,6 +596,7 @@ bool HoistDestroys::rewriteDestroys(const KnownStorageUses &knownUses,
593596
if (reusedDestroys.contains(destroyInst))
594597
continue;
595598

599+
remainingDestroyAddrs.erase(destroyInst);
596600
deleter.forceDelete(destroyInst);
597601
}
598602
deleter.cleanupDeadInstructions();
@@ -697,6 +701,7 @@ void HoistDestroys::mergeDestroys(SILBasicBlock *mergeBlock) {
697701
createDestroy(&mergeBlock->front(), deadDestroys[0]->getDebugScope());
698702

699703
for (auto *deadDestroy : deadDestroys) {
704+
remainingDestroyAddrs.erase(deadDestroy);
700705
deleter.forceDelete(deadDestroy);
701706
}
702707
}
@@ -706,6 +711,7 @@ void HoistDestroys::mergeDestroys(SILBasicBlock *mergeBlock) {
706711
// =============================================================================
707712

708713
bool hoistDestroys(SILValue root, bool ignoreDeinitBarriers,
714+
SmallPtrSetImpl<SILInstruction *> &remainingDestroyAddrs,
709715
InstructionDeleter &deleter) {
710716
LLVM_DEBUG(llvm::dbgs() << "Performing destroy hoisting on " << root);
711717

@@ -716,7 +722,9 @@ bool hoistDestroys(SILValue root, bool ignoreDeinitBarriers,
716722
// The algorithm assumes no critical edges.
717723
assert(function->hasOwnership() && "requires OSSA");
718724

719-
return HoistDestroys(root, ignoreDeinitBarriers, deleter).perform();
725+
return HoistDestroys(root, ignoreDeinitBarriers, remainingDestroyAddrs,
726+
deleter)
727+
.perform();
720728
}
721729

722730
// =============================================================================
@@ -770,24 +778,41 @@ void SSADestroyHoisting::run() {
770778
// store [init]
771779
//
772780
// sequences to create more destroy_addrs to hoist.
781+
//
782+
// Record the newly created destroy_addrs and the stores they were split off
783+
// of. After hoisting, if they have not been hoisted away from the store
784+
// instruction, we will merge them back together.
785+
llvm::SmallVector<std::pair<DestroyAddrInst *, StoreInst *>, 8>
786+
splitDestroysAndStores;
787+
// The destroy_addrs that were created that have not been deleted. Items are
788+
// erased from the set as the destroy_addrs are deleted.
789+
SmallPtrSet<SILInstruction *, 8> remainingDestroyAddrs;
790+
// The number of destroys that were split off of store [init]s and not
791+
// recombined.
792+
int splitDestroys = 0;
773793
for (auto *si : sis) {
774794
auto builder = SILBuilderWithScope(si);
775-
builder.createDestroyAddr(
795+
auto *dai = builder.createDestroyAddr(
776796
RegularLocation::getAutoGeneratedLocation(si->getLoc()),
777797
si->getOperand(1));
778798
si->setOwnershipQualifier(StoreOwnershipQualifier::Init);
799+
splitDestroysAndStores.push_back({dai, si});
800+
remainingDestroyAddrs.insert(dai);
801+
++splitDestroys;
779802
}
780803

781804
// We assume that the function is in reverse post order so visiting the
782805
// blocks and pushing begin_access as we see them and then popping them off
783806
// the end will result in hoisting inner begin_access' destroy_addrs first.
784807
while (!bais.empty()) {
785808
auto *bai = bais.pop_back_val();
786-
changed |= hoistDestroys(bai, /*ignoreDeinitBarriers=*/true, deleter);
809+
changed |= hoistDestroys(bai, /*ignoreDeinitBarriers=*/true,
810+
remainingDestroyAddrs, deleter);
787811
}
788812
// Alloc stacks always enclose their accesses.
789813
for (auto *asi : asis) {
790-
changed |= hoistDestroys(asi, /*ignoreDeinitBarriers=*/false, deleter);
814+
changed |= hoistDestroys(asi, /*ignoreDeinitBarriers=*/false,
815+
remainingDestroyAddrs, deleter);
791816
}
792817
// Arguments enclose everything.
793818
for (auto *arg : getFunction()->getArguments()) {
@@ -796,9 +821,30 @@ void SSADestroyHoisting::run() {
796821
->getArgumentConvention()
797822
.isInoutConvention();
798823
changed |= hoistDestroys(arg, /*ignoreDeinitBarriers=*/
799-
isInout, deleter);
824+
isInout, remainingDestroyAddrs, deleter);
825+
}
826+
}
827+
828+
for (auto pair : splitDestroysAndStores) {
829+
auto *dai = pair.first;
830+
if (!remainingDestroyAddrs.contains(dai))
831+
continue;
832+
auto *si = pair.second;
833+
if (dai->getNextInstruction() == si) {
834+
// No stores should have been rewritten during hoisting. Their ownership
835+
// qualifiers were set to [init] when splitting off the destroy_addrs.
836+
assert(si->getOwnershipQualifier() == StoreOwnershipQualifier::Init);
837+
// If a newly created destroy_addr has not been hoisted from its previous
838+
// location, combine it back together with the store [init] which it was
839+
// split off from.
840+
deleter.forceDelete(dai);
841+
si->setOwnershipQualifier(StoreOwnershipQualifier::Assign);
842+
--splitDestroys;
800843
}
801844
}
845+
// If there were any destroy_addrs split off of stores and not recombined
846+
// with them, then the function has changed.
847+
changed |= splitDestroys > 0;
802848

803849
if (changed) {
804850
invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);

0 commit comments

Comments
 (0)