Skip to content

Commit 2decfcb

Browse files
committed
[CanonicalizeOSSALifetime] Extend Onone lifetimes.
To improve the debugging experience of values whose lifetimes are canonicalized without compromising the semantics expressed in the source language, when canonicalizing OSSA lifetimes at Onone, lengthen lifetimes as much as possible without incurring copies that would be eliminated at O. rdar://99618502
1 parent 9cd4cdb commit 2decfcb

File tree

6 files changed

+143
-12
lines changed

6 files changed

+143
-12
lines changed

include/swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ class CanonicalizeOSSALifetime {
205205
/// liveness may be pruned during canonicalization.
206206
bool pruneDebugMode;
207207

208+
/// If true, lifetimes will not be shortened.
209+
bool ononeMode;
210+
208211
/// If true and we are processing a value of move_only type, emit a diagnostic
209212
/// when-ever we need to insert a copy_value.
210213
std::function<void(Operand *)> moveOnlyCopyValueNotification;
@@ -248,7 +251,7 @@ class CanonicalizeOSSALifetime {
248251
/// The destroys of the value. These are not uses, but need to be recorded so
249252
/// that we know when the last use in a consuming block is (without having to
250253
/// repeatedly do use-def walks from destroys).
251-
SmallPtrSet<SILInstruction *, 8> destroys;
254+
SmallPtrSetVector<SILInstruction *, 8> destroys;
252255

253256
/// Information about consuming instructions discovered in this canonical OSSA
254257
/// lifetime.
@@ -286,11 +289,12 @@ class CanonicalizeOSSALifetime {
286289
}
287290

288291
CanonicalizeOSSALifetime(
289-
bool pruneDebugMode, NonLocalAccessBlockAnalysis *accessBlockAnalysis,
290-
DominanceInfo *domTree, InstructionDeleter &deleter,
292+
bool pruneDebugMode, bool ononeMode,
293+
NonLocalAccessBlockAnalysis *accessBlockAnalysis, DominanceInfo *domTree,
294+
InstructionDeleter &deleter,
291295
std::function<void(Operand *)> moveOnlyCopyValueNotification = nullptr,
292296
std::function<void(Operand *)> moveOnlyFinalConsumingUse = nullptr)
293-
: pruneDebugMode(pruneDebugMode),
297+
: pruneDebugMode(pruneDebugMode), ononeMode(ononeMode),
294298
moveOnlyCopyValueNotification(moveOnlyCopyValueNotification),
295299
moveOnlyFinalConsumingUse(moveOnlyFinalConsumingUse),
296300
accessBlockAnalysis(accessBlockAnalysis), domTree(domTree),
@@ -344,6 +348,8 @@ class CanonicalizeOSSALifetime {
344348

345349
void findExtendedBoundary(PrunedLivenessBoundary &boundary);
346350

351+
void extendUnconsumedLiveness(PrunedLivenessBoundary &boundary);
352+
347353
void insertDestroysOnBoundary(PrunedLivenessBoundary &boundary);
348354

349355
void rewriteCopies();

lib/SILOptimizer/Mandatory/MoveOnlyAddressChecker.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,8 @@ struct MoveOnlyChecker {
746746
instToDelete->eraseFromParent();
747747
})),
748748
canonicalizer(
749-
false /*pruneDebugMode*/, accessBlockAnalysis, domTree, deleter,
749+
false /*pruneDebugMode*/, !fn->shouldOptimize() /*ononeMode*/,
750+
accessBlockAnalysis, domTree, deleter,
750751
[&](Operand *use) { consumingUsesNeedingCopy.push_back(use); },
751752
[&](Operand *use) { finalConsumingUses.push_back(use); }) {}
752753

lib/SILOptimizer/Mandatory/MoveOnlyObjectChecker.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -401,8 +401,9 @@ bool MoveOnlyChecker::check(NonLocalAccessBlockAnalysis *accessBlockAnalysis,
401401
};
402402

403403
CanonicalizeOSSALifetime canonicalizer(
404-
false /*pruneDebugMode*/, accessBlockAnalysis, domTree, deleter,
405-
foundConsumingUseNeedingCopy, foundConsumingUseNotNeedingCopy);
404+
false /*pruneDebugMode*/, !fn->shouldOptimize() /*ononeMode*/,
405+
accessBlockAnalysis, domTree, deleter, foundConsumingUseNeedingCopy,
406+
foundConsumingUseNotNeedingCopy);
406407
auto moveIntroducers = llvm::makeArrayRef(moveIntroducersToProcess.begin(),
407408
moveIntroducersToProcess.end());
408409
SmallPtrSet<MarkMustCheckInst *, 4> valuesWithDiagnostics;

lib/SILOptimizer/SILCombiner/SILCombine.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,10 @@ void SILCombiner::canonicalizeOSSALifetimes(SILInstruction *currentInst) {
350350
InstructionDeleter deleter(std::move(canonicalizeCallbacks));
351351

352352
DominanceInfo *domTree = DA->get(&Builder.getFunction());
353-
CanonicalizeOSSALifetime canonicalizer(false /*prune debug*/, NLABA, domTree,
354-
deleter);
353+
CanonicalizeOSSALifetime canonicalizer(
354+
false /*prune debug*/,
355+
!parentTransform->getFunction()->shouldOptimize() /*ononeMode*/, NLABA,
356+
domTree, deleter);
355357
CanonicalizeBorrowScope borrowCanonicalizer(deleter);
356358

357359
while (!defsToCanonicalize.empty()) {

lib/SILOptimizer/Transforms/CopyPropagation.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ static bool sinkOwnedForward(SILInstruction *ownedForward,
373373
namespace {
374374

375375
class CopyPropagation : public SILFunctionTransform {
376-
/// True if debug_value instructions should be pruned.
376+
/// If true, debug_value instructions should be pruned.
377377
bool pruneDebug;
378378
/// True if all values should be canonicalized.
379379
bool canonicalizeAll;
@@ -437,8 +437,9 @@ void CopyPropagation::run() {
437437

438438
// canonicalizer performs all modifications through deleter's callbacks, so we
439439
// don't need to explicitly check for changes.
440-
CanonicalizeOSSALifetime canonicalizer(pruneDebug, accessBlockAnalysis,
441-
domTree, deleter);
440+
CanonicalizeOSSALifetime canonicalizer(
441+
pruneDebug, /*ononeMode=*/!getFunction()->shouldOptimize(),
442+
accessBlockAnalysis, domTree, deleter);
442443

443444
// NOTE: We assume that the function is in reverse post order so visiting the
444445
// blocks and pushing begin_borrows as we see them and then popping them

lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@
5252
///
5353
/// 2. Liveness is extended out to original destroys to avoid spurious changes.
5454
///
55+
/// 3. In the Onone mode, liveness is preserved to its previous extent whenever
56+
/// doing so doesn't incur extra copies compared to what is done in the O mode.
57+
///
5558
//===----------------------------------------------------------------------===//
5659

5760
#define DEBUG_TYPE "copy-propagation"
@@ -611,6 +614,118 @@ void CanonicalizeOSSALifetime::insertDestroysOnBoundary(
611614
}
612615
}
613616

617+
/// At -Onone, there are some conflicting goals:
618+
/// On the one hand: good debugging experience.
619+
/// (1) do not shorten value's lifetime
620+
/// On the other: demonstrate semantics.
621+
/// (2) consume value at same places it will be consumed at -O
622+
/// (3) ensure there are no more copies than there would be at -O
623+
///
624+
/// (2) and (3) are equivalent--extra (compared to -O) copies arise from failing
625+
/// to end lifetimes at consuming uses (which then need their own copies).
626+
///
627+
/// We achieve (2) and (3) always. We achieve (1) where possible.
628+
///
629+
/// Conceptually, the strategy is the following:
630+
/// - Collect the blocks in which the value was live before canonicalization.
631+
/// These are the "original" live blocks (originalLiveBlocks).
632+
/// [Color these blocks green.]
633+
/// - From that collection, collect the blocks which occur after a _final_
634+
/// consuming use (that isn't a destroy).
635+
/// These are the "consumed" blocks (consumedBlocks).
636+
/// [Color these blocks red.]
637+
/// - Extend liveness up to the boundary between originalLiveBlocks and
638+
/// consumedBlocks blocks.
639+
/// [Extend liveness up to the boundary between green blocks and red.]
640+
/// - In particular, in regions of originalLiveBlocks which have no boundary
641+
/// with consumedBlocks, livness should be extended to its original extent.
642+
/// [Extend liveness up to the boundary between green blocks and uncolored.]
643+
void CanonicalizeOSSALifetime::extendUnconsumedLiveness(
644+
PrunedLivenessBoundary &boundary) {
645+
auto currentDef = getCurrentDef();
646+
647+
// First, collect the blocks that were _originally_ live. We can't use
648+
// liveness here because it doesn't include blocks that occur before a
649+
// destroy_value.
650+
BasicBlockSet originalLiveBlocks(currentDef->getFunction());
651+
{
652+
// Start the walk from the consuming blocks (which includes destroys as well
653+
// as the other consuming uses).
654+
BasicBlockWorklist worklist(currentDef->getFunction());
655+
for (auto *consumingBlock : consumingBlocks) {
656+
worklist.push(consumingBlock);
657+
}
658+
659+
// Walk backwards from consuming blocks.
660+
auto *currentDefBlock = currentDef->getParentBlock();
661+
while (auto *block = worklist.pop()) {
662+
originalLiveBlocks.insert(block);
663+
// Keep adding blocks until we find the def block.
664+
if (currentDefBlock == block)
665+
continue;
666+
for (auto *predecessor : block->getPredecessorBlocks()) {
667+
worklist.pushIfNotVisited(predecessor);
668+
}
669+
}
670+
}
671+
672+
// Second, collect the blocks which occur after a _final_ consuming use.
673+
BasicBlockSetVector finalConsumingBlocks(currentDef->getFunction());
674+
BasicBlockSetVector consumedBlocks(currentDef->getFunction());
675+
{
676+
// Start the walk from blocks which contain _final_ non-destroy consumes.
677+
// These are just our instructions on the boundary which aren't destroys.
678+
BasicBlockWorklist worklist(currentDef->getFunction());
679+
for (auto *instruction : boundary.lastUsers) {
680+
if (dynCastToDestroyOf(instruction, getCurrentDef()))
681+
continue;
682+
if (liveness.isInterestingUser(instruction) !=
683+
PrunedLiveness::IsInterestingUser::LifetimeEndingUse)
684+
continue;
685+
worklist.push(instruction->getParent());
686+
}
687+
while (auto *block = worklist.pop()) {
688+
finalConsumingBlocks.insert(block);
689+
for (auto *successor : block->getSuccessorBlocks()) {
690+
if (!originalLiveBlocks.contains(successor))
691+
continue;
692+
worklist.pushIfNotVisited(successor);
693+
consumedBlocks.insert(successor);
694+
}
695+
}
696+
}
697+
698+
// Third, find the blocks on the boundary between the originally-live blocks
699+
// and the originally-live-but-consumed blocks. These are new boundary
700+
// blocks.
701+
for (auto *block : consumedBlocks) {
702+
for (auto *predecessor : block->getPredecessorBlocks()) {
703+
if (!consumedBlocks.contains(predecessor) &&
704+
!finalConsumingBlocks.contains(predecessor)) {
705+
// Add "the instruction(s) before the terminator" of the predecessor to
706+
// liveness.
707+
if (auto *inst =
708+
predecessor->getTerminator()->getPreviousInstruction()) {
709+
liveness.updateForUse(inst, /*lifetimeEnding*/ false);
710+
} else {
711+
for (auto *grandPredecessor : predecessor->getPredecessorBlocks()) {
712+
liveness.updateForUse(grandPredecessor->getTerminator(),
713+
/*lifetimeEnding*/ false);
714+
}
715+
}
716+
}
717+
}
718+
}
719+
720+
// Finally, preserve the destroys which weren't in the consumed region in
721+
// place: hoisting such destroys would not avoid copies.
722+
for (auto *destroy : destroys) {
723+
if (consumedBlocks.contains(destroy->getParent()))
724+
continue;
725+
liveness.updateForUse(destroy, /*lifetimeEnding*/ true);
726+
}
727+
}
728+
614729
//===----------------------------------------------------------------------===//
615730
// MARK: Step 4. Rewrite copies and destroys
616731
//===----------------------------------------------------------------------===//
@@ -781,6 +896,11 @@ bool CanonicalizeOSSALifetime::canonicalizeValueLifetime(SILValue def) {
781896
// Step 2: compute boundary
782897
PrunedLivenessBoundary boundary;
783898
findExtendedBoundary(boundary);
899+
if (ononeMode) {
900+
extendUnconsumedLiveness(boundary);
901+
boundary.clear();
902+
findExtendedBoundary(boundary);
903+
}
784904
// Step 3: insert destroys and record consumes
785905
insertDestroysOnBoundary(boundary);
786906
// Step 4: rewrite copies and delete extra destroys

0 commit comments

Comments
 (0)