Merge pull request swiftlang#39672 from atrick/copyprop-reusedestroy

CopyPropagation: Avoid regenerating destroys.

Author: atrick

include/swift/SILOptimizer/Utils/CanonicalOSSALifetime.h

@@ -357,8 +357,8 @@ class CanonicalizeOSSALifetime {
 
   void findOrInsertDestroys();
 
-  void insertDestroyOnCFGEdge(SILBasicBlock *predBB, SILBasicBlock *succBB,
-                              bool needsPoison);
+  void findOrInsertDestroyOnCFGEdge(SILBasicBlock *predBB,
+                                    SILBasicBlock *succBB, bool needsPoison);
 
   void rewriteCopies();
 

lib/SILOptimizer/Utils/CanonicalOSSALifetime.cpp

@@ -343,25 +343,48 @@ void CanonicalizeOSSALifetime::extendLivenessThroughOverlappingAccess() {
 // liveness computed in Step 1.
 //===----------------------------------------------------------------------===//
 
+// Look past destroys and incidental uses to find a destroy on \p edgeBB that
+// destroys \p def.
+static DestroyValueInst *findDestroyOnCFGEdge(SILBasicBlock *edgeBB,
+                                              SILValue def) {
+  for (auto &inst : *edgeBB) {
+    if (isIncidentalUse(&inst))
+      continue;
+    if (auto *destroy = dyn_cast<DestroyValueInst>(&inst)) {
+      if (destroy->getOperand() == def)
+        return destroy;
+      continue;
+    }
+    break;
+  }
+  return nullptr;
+}
+
 /// The liveness boundary is at a CFG edge `predBB` -> `succBB`, meaning that
 /// `currentDef` is live out of at least one other `predBB` successor.
 ///
 /// Create and record a final destroy_value at the beginning of `succBB`
 /// (assuming no critical edges).
-void CanonicalizeOSSALifetime::insertDestroyOnCFGEdge(
-  SILBasicBlock *predBB, SILBasicBlock *succBB, bool needsPoison) {
+///
+/// Avoid deleting and recreating a destroy that was already placed on this
+/// edge. Ignore any intervening destroys that may have been placed while
+/// canonicalizing other values.  This is especially important when
+/// canonicalization is called within an iterative worklist such as SILCombine.
+void CanonicalizeOSSALifetime::findOrInsertDestroyOnCFGEdge(
+    SILBasicBlock *predBB, SILBasicBlock *succBB, bool needsPoison) {
 
   assert(succBB->getSinglePredecessorBlock() == predBB
          && "value is live-out on another predBB successor: critical edge?");
-
-  auto pos = succBB->begin();
-  // TODO: to improve debugability, consider advancing the poison position ahead
-  // of operations that are known not to access weak references.
-  SILBuilderWithScope builder(pos);
-  auto loc = RegularLocation::getAutoGeneratedLocation(pos->getLoc());
-  auto *di = builder.createDestroyValue(loc, currentDef, needsPoison);
-  getCallbacks().createdNewInst(di);
-
+  auto *di = findDestroyOnCFGEdge(succBB, currentDef);
+  if (!di) {
+    auto pos = succBB->begin();
+    // TODO: to improve debugability, consider advancing the poison position
+    // ahead of operations that are known not to access weak references.
+    SILBuilderWithScope builder(pos);
+    auto loc = RegularLocation::getAutoGeneratedLocation(pos->getLoc());
+    di = builder.createDestroyValue(loc, currentDef, needsPoison);
+    getCallbacks().createdNewInst(di);
+  }
   consumes.recordFinalConsume(di);
 
   ++NumDestroysGenerated;
@@ -431,7 +454,7 @@ void CanonicalizeOSSALifetime::findOrInsertDestroyInBlock(SILBasicBlock *bb) {
       // Insert a destroy after this non-consuming use.
       if (isa<TermInst>(inst)) {
         for (auto &succ : bb->getSuccessors()) {
-          insertDestroyOnCFGEdge(bb, succ, poisonRefsMode);
+          findOrInsertDestroyOnCFGEdge(bb, succ, poisonRefsMode);
         }
       } else {
         if (existingDestroy) {
@@ -540,7 +563,7 @@ void CanonicalizeOSSALifetime::findOrInsertDestroys() {
       // Continue searching upward to find the pruned liveness boundary.
       for (auto *predBB : bb->getPredecessorBlocks()) {
         if (liveness.getBlockLiveness(predBB) == PrunedLiveBlocks::LiveOut) {
-          insertDestroyOnCFGEdge(predBB, bb, poisonRefsMode);
+          findOrInsertDestroyOnCFGEdge(predBB, bb, poisonRefsMode);
         } else {
           if (poisonRefsMode) {
             remnantLiveOutBlocks.insert(predBB);