Merge pull request #64339 from nate-chandler/copy-propagation/redundant-move-elim

[CopyPropagation] Eliminate redundant moves.

Author: nate-chandler

include/swift/SIL/OwnershipUtils.h

@@ -1347,6 +1347,14 @@ void visitTransitiveEndBorrows(
 /// - the value is itself a begin_borrow [lexical]
 bool isNestedLexicalBeginBorrow(BeginBorrowInst *bbi);
 
+/// Whether specified move_value is redundant.
+///
+/// A move_value is redundant if the lifetimes that it separates both have the
+/// same characteristics with respect to
+/// - lexicality
+/// - escaping
+bool isRedundantMoveValue(MoveValueInst *mvi);
+
 } // namespace swift
 
 #endif

lib/SIL/Utils/OwnershipUtils.cpp

@@ -2381,3 +2381,25 @@ bool swift::isNestedLexicalBeginBorrow(BeginBorrowInst *bbi) {
     return false;
   });
 }
+
+bool swift::isRedundantMoveValue(MoveValueInst *mvi) {
+  auto original = mvi->getOperand();
+
+  // If the moved-from value has none ownership, hasPointerEscape can't handle
+  // it, so it can't be used to determine whether escaping matches.
+  if (original->getOwnershipKind() != OwnershipKind::Owned) {
+    return false;
+  }
+
+  // First, check whether lexicality matches, the cheaper check.
+  if (mvi->isLexical() != original->isLexical()) {
+    return false;
+  }
+
+  // Then, check whether escaping matches, the more expensive check.
+  if (hasPointerEscape(mvi) != hasPointerEscape(original)) {
+    return false;
+  }
+
+  return true;
+}

lib/SILOptimizer/SemanticARC/RedundantMoveValueElimination.cpp

@@ -39,25 +39,10 @@ bool SemanticARCOptVisitor::visitMoveValueInst(MoveValueInst *mvi) {
   if (!ctx.shouldPerform(ARCTransformKind::RedundantMoveValueElim))
     return false;
 
-  auto original = mvi->getOperand();
-
-  // If the moved-from value has none ownership, hasPointerEscape can't handle
-  // it, so it can't be used to determine whether escaping matches.
-  if (original->getOwnershipKind() != OwnershipKind::Owned) {
-    return false;
-  }
-
-  // First, check whether lexicality matches, the cheaper check.
-  if (mvi->isLexical() != original->isLexical()) {
-    return false;
-  }
-
-  // Then, check whether escaping matches, the more expensive check.
-  if (hasPointerEscape(mvi) != hasPointerEscape(original)) {
+  if (!isRedundantMoveValue(mvi))
     return false;
-  }
 
   // Both characteristics match.
-  eraseAndRAUWSingleValueInstruction(mvi, original);
+  eraseAndRAUWSingleValueInstruction(mvi, mvi->getOperand());
   return true;
 }

lib/SILOptimizer/Transforms/CopyPropagation.cpp

@@ -42,6 +42,7 @@
 #include "swift/SIL/BasicBlockDatastructures.h"
 #include "swift/SIL/BasicBlockUtils.h"
 #include "swift/SIL/DebugUtils.h"
+#include "swift/SIL/OwnershipUtils.h"
 #include "swift/SIL/SILUndef.h"
 #include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
 #include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h"
@@ -263,6 +264,24 @@ static bool convertExtractsToDestructures(CanonicalDefWorklist &copiedDefs,
   return changed;
 }
 
+//===----------------------------------------------------------------------===//
+//                MARK: Eliminate redundant moves
+//===----------------------------------------------------------------------===//
+
+/// If the specified move_value is redundant (there's no benefit to separating
+/// the lifetime at it), replace its uses with uses of the moved-from value and
+/// delete it.
+static bool eliminateRedundantMove(MoveValueInst *mvi,
+                                   InstructionDeleter &deleter) {
+  if (!isRedundantMoveValue(mvi))
+    return false;
+  mvi->replaceAllUsesWith(mvi->getOperand());
+  // Call InstructionDeleter::forceDeleteWithUsers to avoid "fixing up"
+  // ownership of the moved-from value, i.e. inserting a destroy_value.
+  deleter.forceDeleteWithUsers(mvi);
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 //                   MARK: Sink owned forwarding operations
 //===----------------------------------------------------------------------===//
@@ -419,15 +438,18 @@ void CopyPropagation::run() {
   InstructionDeleter deleter(std::move(callbacks));
   bool changed = false;
 
-  GraphNodeWorklist<BeginBorrowInst *, 16> beginBorrowsToShrink;
+  StackList<BeginBorrowInst *> beginBorrowsToShrink(f);
+  StackList<MoveValueInst *> moveValues(f);
 
   // Driver: Find all copied or borrowed defs.
   for (auto &bb : *f) {
     for (auto &i : bb) {
       if (auto *copy = dyn_cast<CopyValueInst>(&i)) {
         defWorklist.updateForCopy(copy);
       } else if (auto *borrow = dyn_cast<BeginBorrowInst>(&i)) {
-        beginBorrowsToShrink.insert(borrow);
+        beginBorrowsToShrink.push_back(borrow);
+      } else if (auto *move = dyn_cast<MoveValueInst>(&i)) {
+        moveValues.push_back(move);
       } else if (canonicalizeAll) {
         if (auto *destroy = dyn_cast<DestroyValueInst>(&i)) {
           defWorklist.updateForCopy(destroy->getOperand());
@@ -446,7 +468,7 @@ void CopyPropagation::run() {
   // NOTE: We assume that the function is in reverse post order so visiting the
   //       blocks and pushing begin_borrows as we see them and then popping them
   //       off the end will result in shrinking inner borrow scopes first.
-  while (auto *bbi = beginBorrowsToShrink.pop()) {
+  for (auto *bbi : beginBorrowsToShrink) {
     bool firstRun = true;
     // Run the sequence of utilities:
     // - ShrinkBorrowScope
@@ -480,9 +502,13 @@ void CopyPropagation::run() {
           hoistDestroysOfOwnedLexicalValue(folded, *f, deleter, calleeAnalysis);
       // Keep running even if the new move's destroys can't be hoisted.
       (void)hoisted;
+      eliminateRedundantMove(folded, deleter);
       firstRun = false;
     }
   }
+  for (auto *mvi : moveValues) {
+    eliminateRedundantMove(mvi, deleter);
+  }
   for (auto *argument : f->getArguments()) {
     if (argument->getOwnershipKind() == OwnershipKind::Owned) {
       hoistDestroysOfOwnedLexicalValue(argument, *f, deleter, calleeAnalysis);

test/SILOptimizer/copy_propagation.sil

@@ -851,16 +851,19 @@ bb0(%0 : @owned $C):
 // Test that copy propagation doesn't hoist a destroy_value corresponding to
 // a move value [lexical] over a barrier.
 // CHECK-LABEL: sil [ossa] @dont_hoist_move_value_lexical_destroy_over_barrier_apply : {{.*}} {
-// CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
+// CHECK:         [[GET_OWNED_C:%.*]] = function_ref @getOwnedC
+// CHECK:         [[INSTANCE:%.*]] = apply [[GET_OWNED_C]]
 // CHECK:         [[LIFETIME:%[^,]+]] = move_value [lexical] [[INSTANCE]]
 // CHECK:         [[BARRIER:%[^,]+]] = function_ref @barrier
 // CHECK:         [[TAKE_GUARANTEED_C:%[^,]+]] = function_ref @takeGuaranteedC
 // CHECK:         apply [[TAKE_GUARANTEED_C]]([[LIFETIME]])
 // CHECK:         apply [[BARRIER]]()
 // CHECK:         destroy_value [[LIFETIME]]
 // CHECK-LABEL: } // end sil function 'dont_hoist_move_value_lexical_destroy_over_barrier_apply'
-sil [ossa] @dont_hoist_move_value_lexical_destroy_over_barrier_apply : $@convention(thin) (@owned C) -> () {
-entry(%instance : @owned $C):
+sil [ossa] @dont_hoist_move_value_lexical_destroy_over_barrier_apply : $@convention(thin) () -> () {
+entry:
+  %getOwnedC = function_ref @getOwnedC : $@convention(thin) () -> (@owned C)
+  %instance = apply %getOwnedC() : $@convention(thin) () -> (@owned C)
   %lifetime = move_value [lexical] %instance : $C
   %barrier = function_ref @barrier : $@convention(thin) () -> ()
   %takeGuaranteedC = function_ref @takeGuaranteedC : $@convention(thin) (@guaranteed C) -> ()