Merge pull request #64190 from nate-chandler/redundant-move-elim

[SILOptimizer] Add RedundantMoveValueElimination.

Author: nate-chandler

include/swift/SIL/OwnershipUtils.h

@@ -122,6 +122,12 @@ inline bool isForwardingConsume(SILValue value) {
 
 bool hasPointerEscape(BorrowedValue value);
 
+/// Whether the specified OSSA-lifetime introducer has a pointer escape.
+///
+/// precondition: \p value introduces an OSSA-lifetime, either a BorrowedValue
+/// can be constructed from it or it's an owned value
+bool hasPointerEscape(SILValue value);
+
 /// Find leaf "use points" of \p guaranteedValue that determine its lifetime
 /// requirement. Return true if no PointerEscape use was found.
 ///

lib/SIL/Utils/OwnershipUtils.cpp

@@ -83,6 +83,44 @@ bool swift::hasPointerEscape(BorrowedValue value) {
   return false;
 }
 
+bool swift::hasPointerEscape(SILValue original) {
+  if (auto borrowedValue = BorrowedValue(original)) {
+    return hasPointerEscape(borrowedValue);
+  }
+  assert(original->getOwnershipKind() == OwnershipKind::Owned);
+
+  ValueWorklist worklist(original->getFunction());
+  worklist.push(original);
+  if (auto *phi = SILArgument::asPhi(original)) {
+    phi->visitTransitiveIncomingPhiOperands([&](auto *phi, auto *operand) {
+      worklist.pushIfNotVisited(operand->get());
+      return true;
+    });
+  }
+  while (auto value = worklist.popAndForget()) {
+    for (auto use : value->getUses()) {
+      switch (use->getOperandOwnership()) {
+      case OperandOwnership::PointerEscape:
+      case OperandOwnership::ForwardingUnowned:
+        return true;
+      case OperandOwnership::ForwardingConsume: {
+        auto *branch = dyn_cast<BranchInst>(use->getUser());
+        if (!branch) {
+          // Non-phi forwarding consumes end the lifetime of an owned value.
+          break;
+        }
+        auto *phi = branch->getDestBB()->getArgument(use->getOperandNumber());
+        worklist.pushIfNotVisited(phi);
+        break;
+      }
+      default:
+        break;
+      }
+    }
+  }
+  return false;
+}
+
 bool swift::canOpcodeForwardInnerGuaranteedValues(SILValue value) {
   // If we have an argument from a transforming terminator, we can forward
   // guaranteed.

lib/SILOptimizer/SemanticARC/CMakeLists.txt

@@ -8,4 +8,5 @@ target_sources(swiftSILOptimizer PRIVATE
   Context.cpp
   SemanticARCOptVisitor.cpp
   OwnershipConversionElimination.cpp
+  RedundantMoveValueElimination.cpp
   )

lib/SILOptimizer/SemanticARC/RedundantMoveValueElimination.cpp

@@ -0,0 +1,63 @@
+//===--- RedundantMoveValueElimination.cpp - Delete spurious move_values --===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+// A move_value ends an owned lifetime and begins an owned lifetime.
+//
+// The new lifetime may have the same characteristics as the original lifetime
+// with regards to
+// - lexicality
+// - escaping
+//
+// If it has the same characteristics, there is no reason to have two separate
+// lifetimes--they are redundant.  This optimization deletes such redundant
+// move_values.
+//===----------------------------------------------------------------------===//
+
+#include "SemanticARC/SemanticARCOpts.h"
+#include "SemanticARCOptVisitor.h"
+#include "swift/SIL/LinearLifetimeChecker.h"
+
+using namespace swift;
+using namespace semanticarc;
+
+//===----------------------------------------------------------------------===//
+//                            Top Level Entrypoint
+//===----------------------------------------------------------------------===//
+
+bool SemanticARCOptVisitor::visitMoveValueInst(MoveValueInst *mvi) {
+  if (ctx.onlyMandatoryOpts)
+    return false;
+
+  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)) {
+    return false;
+  }
+
+  // Both characteristics match.
+  eraseAndRAUWSingleValueInstruction(mvi, original);
+  return true;
+}

lib/SILOptimizer/SemanticARC/SemanticARCOptVisitor.h

@@ -143,6 +143,7 @@ struct LLVM_LIBRARY_VISIBILITY SemanticARCOptVisitor
   bool visitCopyValueInst(CopyValueInst *cvi);
   bool visitBeginBorrowInst(BeginBorrowInst *bbi);
   bool visitLoadInst(LoadInst *li);
+  bool visitMoveValueInst(MoveValueInst *mvi);
   bool
   visitUncheckedOwnershipConversionInst(UncheckedOwnershipConversionInst *uoci);
 
@@ -153,6 +154,7 @@ struct LLVM_LIBRARY_VISIBILITY SemanticARCOptVisitor
     case SILInstructionKind::CopyValueInst:
     case SILInstructionKind::BeginBorrowInst:
     case SILInstructionKind::LoadInst:
+    case SILInstructionKind::MoveValueInst:
     case SILInstructionKind::UncheckedOwnershipConversionInst:
       return true;
     }

lib/SILOptimizer/SemanticARC/SemanticARCOpts.cpp

@@ -51,7 +51,11 @@ static llvm::cl::list<ARCTransformKind> TransformsToPerform(
                    "sil-semantic-arc-owned-to-guaranteed-phi",
                    "Perform Owned To Guaranteed Phi. NOTE: Seeded by peephole "
                    "optimizer for compile time saving purposes, so run this "
-                   "after running peepholes)")),
+                   "after running peepholes)"),
+        clEnumValN(ARCTransformKind::RedundantMoveValueElim,
+                   "sil-semantic-arc-redundant-move-value-elim",
+                   "Eliminate move_value which don't change owned lifetime "
+                   "characteristics.  (Escaping, Lexical).")),
     llvm::cl::desc(
         "For testing purposes only run the specified list of semantic arc "
         "optimization. If the list is empty, we run all transforms"));
@@ -86,6 +90,7 @@ struct SemanticARCOpts : SILFunctionTransform {
       case ARCTransformKind::LoadCopyToLoadBorrowPeephole:
       case ARCTransformKind::AllPeepholes:
       case ARCTransformKind::OwnershipConversionElimPeephole:
+      case ARCTransformKind::RedundantMoveValueElim:
         // We never assume we are at fixed point when running these transforms.
         if (performPeepholesWithoutFixedPoint(visitor)) {
           invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);

lib/SILOptimizer/SemanticARC/SemanticARCOpts.h

@@ -31,12 +31,13 @@ enum class ARCTransformKind : uint64_t {
   RedundantCopyValueElimPeephole = 0x8,
   LifetimeJoiningPeephole = 0x10,
   OwnershipConversionElimPeephole = 0x20,
+  RedundantMoveValueElim = 0x40,
 
   AllPeepholes = LoadCopyToLoadBorrowPeephole |
                  RedundantBorrowScopeElimPeephole |
                  RedundantCopyValueElimPeephole | LifetimeJoiningPeephole |
                  OwnershipConversionElimPeephole,
-  All = AllPeepholes | OwnedToGuaranteedPhi,
+  All = AllPeepholes | OwnedToGuaranteedPhi | RedundantMoveValueElim,
 };
 
 inline ARCTransformKind operator&(ARCTransformKind lhs, ARCTransformKind rhs) {

lib/SILOptimizer/UtilityPasses/UnitTestRunner.cpp

@@ -71,6 +71,7 @@
 #include "swift/SIL/MemAccessUtils.h"
 #include "swift/SIL/OSSALifetimeCompletion.h"
 #include "swift/SIL/OwnershipLiveness.h"
+#include "swift/SIL/OwnershipUtils.h"
 #include "swift/SIL/PrunedLiveness.h"
 #include "swift/SIL/SILArgumentArrayRef.h"
 #include "swift/SIL/SILBasicBlock.h"
@@ -267,6 +268,22 @@ struct TestSpecificationTest : UnitTest {
   }
 };
 
+// Arguments:
+// - value: the value to check for escaping
+// Dumps:
+// - the value
+// - whether it has a pointer escape
+struct OwnershipUtilsHasPointerEscape : UnitTest {
+  OwnershipUtilsHasPointerEscape(UnitTestRunner *pass) : UnitTest(pass) {}
+  void invoke(Arguments &arguments) override {
+    auto value = arguments.takeValue();
+    auto has = hasPointerEscape(value);
+    value->print(llvm::errs());
+    auto *boolString = has ? "true" : "false";
+    llvm::errs() << boolString << "\n";
+  }
+};
+
 //===----------------------------------------------------------------------===//
 // MARK: OSSA Lifetime Unit Tests
 //===----------------------------------------------------------------------===//
@@ -789,6 +806,7 @@ void UnitTestRunner::withTest(StringRef name, Doit doit) {
     ADD_UNIT_TEST_SUBCLASS("find-borrow-introducers", FindBorrowIntroducers)
     ADD_UNIT_TEST_SUBCLASS("find-enclosing-defs", FindEnclosingDefsTest)
     ADD_UNIT_TEST_SUBCLASS("function-get-self-argument-index", FunctionGetSelfArgumentIndex)
+    ADD_UNIT_TEST_SUBCLASS("has-pointer-escape", OwnershipUtilsHasPointerEscape)
     ADD_UNIT_TEST_SUBCLASS("interior-liveness", InteriorLivenessTest)
     ADD_UNIT_TEST_SUBCLASS("is-deinit-barrier", IsDeinitBarrierTest)
     ADD_UNIT_TEST_SUBCLASS("is-lexical", IsLexicalTest)

test/SILOptimizer/ownership-utils-unit.sil

@@ -0,0 +1,44 @@
+// RUN: %target-sil-opt -unit-test-runner %s -o /dev/null 2>&1 | %FileCheck %s
+
+sil_stage canonical
+
+import Builtin
+
+class C {}
+
+sil @getOwned : $@convention(thin) () -> (@owned C)
+
+sil @borrow : $@convention(thin) (@guaranteed C) -> ()
+
+sil @useUnmanaged : $@convention(thin) (@sil_unmanaged C) -> ()
+
+sil [ossa] @test_escape_of_phi_transitive_incoming_value : $@convention(thin) () -> () {
+entry:
+  %getOwned = function_ref @getOwned : $@convention(thin) () -> (@owned C)
+  %useUnmanaged = function_ref @useUnmanaged : $@convention(thin) (@sil_unmanaged C) -> ()
+  cond_br undef, left, right
+left:
+  cond_br undef, left_left, left_right
+left_left:
+  %llinstance = apply %getOwned() : $@convention(thin) () -> (@owned C)
+  br left_bottom(%llinstance : $C)
+left_right:
+  %lrinstance = apply %getOwned() : $@convention(thin) () -> (@owned C)
+  %escape = ref_to_unmanaged %lrinstance : $C to $@sil_unmanaged C
+  apply %useUnmanaged(%escape) : $@convention(thin) (@sil_unmanaged C) -> ()
+  br left_bottom(%lrinstance : $C)
+left_bottom(%linstance : @owned $C):
+  br exit(%linstance : $C)
+right:
+  %rinstance = apply %getOwned() : $@convention(thin) () -> (@owned C)
+  br exit(%rinstance : $C)
+exit(%instance : @owned $C):
+// CHECK-LABEL: begin running test {{[0-9]+}} of {{[0-9]+}} on test_escape_of_phi_transitive_incoming_value: has-pointer-escape
+// CHECK:       %14 = argument of bb6 : $C
+// CHECK:       true
+// CHECK-LABEL: end running test {{[0-9]+}} of {{[0-9]+}} on test_escape_of_phi_transitive_incoming_value: has-pointer-escape with
+  test_specification "has-pointer-escape @block.argument"
+  destroy_value %instance : $C
+  %retval = tuple ()
+  return %retval : $()
+}