Skip to content

Commit 6a38f57

Browse files
committed
CopyPropagation: Avoid regenerating destroys.
CanonicalizeOSSA is now used iteratively in SILCombine. To avoid endless worklist iteration based on whether InstructionDeleter's callbacks fired, ensure that destroys are only deleted and recreated when necessary.
1 parent 790b549 commit 6a38f57

File tree

2 files changed

+38
-15
lines changed

2 files changed

+38
-15
lines changed

include/swift/SILOptimizer/Utils/CanonicalOSSALifetime.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,8 @@ class CanonicalizeOSSALifetime {
357357

358358
void findOrInsertDestroys();
359359

360-
void insertDestroyOnCFGEdge(SILBasicBlock *predBB, SILBasicBlock *succBB,
361-
bool needsPoison);
360+
void findOrInsertDestroyOnCFGEdge(SILBasicBlock *predBB,
361+
SILBasicBlock *succBB, bool needsPoison);
362362

363363
void rewriteCopies();
364364

lib/SILOptimizer/Utils/CanonicalOSSALifetime.cpp

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -343,25 +343,48 @@ void CanonicalizeOSSALifetime::extendLivenessThroughOverlappingAccess() {
343343
// liveness computed in Step 1.
344344
//===----------------------------------------------------------------------===//
345345

346+
// Look past destroys and incidental uses to find a destroy on \p edgeBB that
347+
// destroys \p def.
348+
static DestroyValueInst *findDestroyOnCFGEdge(SILBasicBlock *edgeBB,
349+
SILValue def) {
350+
for (auto &inst : *edgeBB) {
351+
if (isIncidentalUse(&inst))
352+
continue;
353+
if (auto *destroy = dyn_cast<DestroyValueInst>(&inst)) {
354+
if (destroy->getOperand() == def)
355+
return destroy;
356+
continue;
357+
}
358+
break;
359+
}
360+
return nullptr;
361+
}
362+
346363
/// The liveness boundary is at a CFG edge `predBB` -> `succBB`, meaning that
347364
/// `currentDef` is live out of at least one other `predBB` successor.
348365
///
349366
/// Create and record a final destroy_value at the beginning of `succBB`
350367
/// (assuming no critical edges).
351-
void CanonicalizeOSSALifetime::insertDestroyOnCFGEdge(
352-
SILBasicBlock *predBB, SILBasicBlock *succBB, bool needsPoison) {
368+
///
369+
/// Avoid deleting and recreating a destroy that was already placed on this
370+
/// edge. Ignore any intervening destroys that may have been placed while
371+
/// canonicalizing other values. This is especially important when
372+
/// canonicalization is called within an iterative worklist such as SILCombine.
373+
void CanonicalizeOSSALifetime::findOrInsertDestroyOnCFGEdge(
374+
SILBasicBlock *predBB, SILBasicBlock *succBB, bool needsPoison) {
353375

354376
assert(succBB->getSinglePredecessorBlock() == predBB
355377
&& "value is live-out on another predBB successor: critical edge?");
356-
357-
auto pos = succBB->begin();
358-
// TODO: to improve debugability, consider advancing the poison position ahead
359-
// of operations that are known not to access weak references.
360-
SILBuilderWithScope builder(pos);
361-
auto loc = RegularLocation::getAutoGeneratedLocation(pos->getLoc());
362-
auto *di = builder.createDestroyValue(loc, currentDef, needsPoison);
363-
getCallbacks().createdNewInst(di);
364-
378+
auto *di = findDestroyOnCFGEdge(succBB, currentDef);
379+
if (!di) {
380+
auto pos = succBB->begin();
381+
// TODO: to improve debugability, consider advancing the poison position
382+
// ahead of operations that are known not to access weak references.
383+
SILBuilderWithScope builder(pos);
384+
auto loc = RegularLocation::getAutoGeneratedLocation(pos->getLoc());
385+
di = builder.createDestroyValue(loc, currentDef, needsPoison);
386+
getCallbacks().createdNewInst(di);
387+
}
365388
consumes.recordFinalConsume(di);
366389

367390
++NumDestroysGenerated;
@@ -431,7 +454,7 @@ void CanonicalizeOSSALifetime::findOrInsertDestroyInBlock(SILBasicBlock *bb) {
431454
// Insert a destroy after this non-consuming use.
432455
if (isa<TermInst>(inst)) {
433456
for (auto &succ : bb->getSuccessors()) {
434-
insertDestroyOnCFGEdge(bb, succ, poisonRefsMode);
457+
findOrInsertDestroyOnCFGEdge(bb, succ, poisonRefsMode);
435458
}
436459
} else {
437460
if (existingDestroy) {
@@ -540,7 +563,7 @@ void CanonicalizeOSSALifetime::findOrInsertDestroys() {
540563
// Continue searching upward to find the pruned liveness boundary.
541564
for (auto *predBB : bb->getPredecessorBlocks()) {
542565
if (liveness.getBlockLiveness(predBB) == PrunedLiveBlocks::LiveOut) {
543-
insertDestroyOnCFGEdge(predBB, bb, poisonRefsMode);
566+
findOrInsertDestroyOnCFGEdge(predBB, bb, poisonRefsMode);
544567
} else {
545568
if (poisonRefsMode) {
546569
remnantLiveOutBlocks.insert(predBB);

0 commit comments

Comments
 (0)