[move-only] Add a new pass called MoveOnlyTypeEliminator that runs after move only checking.

gottesmm · gottesmm · commit 2f9d67de1878 · 2022-06-22T15:31:57.000-07:00
This pass lowers moveonly-ness from the IR after we have finished move only
checking. The transform can be configured in two ways: such that it only handles
trivial types and such that it does trivial and non-trivial types. For ease of
use, I created two top level transforms (TrivialMoveOnlyTypeEliminator and
MoveOnlyTypeElimintor) that invoke the two behaviors by configuring the
underlying transform slightly differently.

For now, I am running first the trivial-only and then the all of the above
lowering. The trivial only pass will remain at this part of the pipeline
forever, but with time we are going to move the lower everything pass later into
the pipeline once I have audited the optimizer pipeline to just not perform any
work on move only types. That being said, currently we do not have this
guarantee and this patch at least improves the world and lets us codegen no
implicit copy code again.
diff --git a/include/swift/SIL/SILValue.h b/include/swift/SIL/SILValue.h
@@ -558,6 +558,19 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
 
   bool isLexical() const;
 
+  /// Unsafely eliminate moveonly from this value's type. Returns true if the
+  /// value's underlying type was move only and thus was changed. Returns false
+  /// otherwise.
+  ///
+  /// NOTE: Please do not use this directly! It is only meant to be used by the
+  /// optimizer pass: SILMoveOnlyTypeEliminator.
+  bool unsafelyEliminateMoveOnlyWrapper() {
+    if (!Type.isMoveOnlyWrapped())
+      return false;
+    Type = Type.removingMoveOnlyWrapper();
+    return true;
+  }
+
   static bool classof(SILNodePointer node) {
     return node->getKind() >= SILNodeKind::First_ValueBase &&
            node->getKind() <= SILNodeKind::Last_ValueBase;
diff --git a/include/swift/SILOptimizer/PassManager/Passes.def b/include/swift/SILOptimizer/PassManager/Passes.def
@@ -438,6 +438,10 @@ PASS(MoveOnlyChecker, "sil-move-only-checker",
 PASS(MoveKillsCopyableValuesChecker, "sil-move-kills-copyable-values-checker",
      "Pass that checks that any copyable (non-move only) value that is passed "
      "to _move do not have any uses later than the _move")
+PASS(TrivialMoveOnlyTypeEliminator, "sil-trivial-move-only-type-eliminator",
+     "Pass that rewrites SIL to remove move only types from values of trivial type")
+PASS(MoveOnlyTypeEliminator, "sil-move-only-type-eliminator",
+     "Pass that rewrites SIL to remove move only types from all values")
 PASS(LexicalLifetimeEliminator, "sil-lexical-lifetime-eliminator",
      "Pass that removes lexical lifetime markers from borrows and alloc stack")
 PASS(MoveKillsCopyableAddressesChecker, "sil-move-kills-copyable-addresses-checker",
diff --git a/lib/SILOptimizer/Mandatory/CMakeLists.txt b/lib/SILOptimizer/Mandatory/CMakeLists.txt
@@ -36,4 +36,5 @@ target_sources(swiftSILOptimizer PRIVATE
   YieldOnceCheck.cpp
   MandatoryCombine.cpp
   OSLogOptimization.cpp
+  MoveOnlyTypeEliminator.cpp
   OwnershipModelEliminator.cpp)
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyTypeEliminator.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyTypeEliminator.cpp
@@ -0,0 +1,305 @@
+//===--- SILMoveOnlyTypeEliminator.cpp ------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2021 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
+//
+//===----------------------------------------------------------------------===//
+///
+/// This file contains an optimizer pass that lowers away move only types from
+/// SIL. It can run on all types or just trivial types. It works by Walking all
+/// values in the IR and unsafely converting their type to be without move
+/// only. If a change is made, we add the defining instruction to a set list for
+/// post-processing. Once we have updated all types in the function, we revisit
+/// the instructions that we touched and update/delete them as appropriate.
+///
+//===----------------------------------------------------------------------===//
+
+#define DEBUG_TYPE "sil-move-only-type-eliminator"
+
+#include "swift/AST/DiagnosticsSIL.h"
+#include "swift/Basic/Defer.h"
+#include "swift/SIL/BasicBlockBits.h"
+#include "swift/SIL/DebugUtils.h"
+#include "swift/SIL/InstructionUtils.h"
+#include "swift/SIL/SILArgument.h"
+#include "swift/SIL/SILBuilder.h"
+#include "swift/SIL/SILFunction.h"
+#include "swift/SIL/SILInstruction.h"
+#include "swift/SIL/SILUndef.h"
+#include "swift/SIL/SILVisitor.h"
+#include "swift/SILOptimizer/Analysis/ClosureScope.h"
+#include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
+#include "swift/SILOptimizer/Analysis/NonLocalAccessBlockAnalysis.h"
+#include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
+#include "swift/SILOptimizer/PassManager/Transforms.h"
+#include "swift/SILOptimizer/Utils/CanonicalOSSALifetime.h"
+
+using namespace swift;
+
+//===----------------------------------------------------------------------===//
+//                                  Visitor
+//===----------------------------------------------------------------------===//
+
+namespace {
+
+struct SILMoveOnlyTypeEliminatorVisitor
+    : SILInstructionVisitor<SILMoveOnlyTypeEliminatorVisitor, bool> {
+  const SmallSetVector<SILArgument *, 8> &touchedArgs;
+
+  SILMoveOnlyTypeEliminatorVisitor(
+      const SmallSetVector<SILArgument *, 8> &touchedArgs)
+      : touchedArgs(touchedArgs) {}
+
+  bool visitSILInstruction(SILInstruction *inst) {
+    llvm::errs() << "Unhandled SIL Instruction: " << *inst;
+    llvm_unreachable("error");
+  }
+
+  bool eraseFromParent(SILInstruction *i) {
+    LLVM_DEBUG(llvm::dbgs() << "Erasing Inst: " << *i);
+    i->eraseFromParent();
+    return true;
+  }
+
+  bool visitLoadInst(LoadInst *li) {
+    if (!li->getType().isTrivial(*li->getFunction()))
+      return false;
+    li->setOwnershipQualifier(LoadOwnershipQualifier::Trivial);
+    return true;
+  }
+
+  bool visitStoreInst(StoreInst *si) {
+    if (!si->getSrc()->getType().isTrivial(*si->getFunction()))
+      return false;
+    si->setOwnershipQualifier(StoreOwnershipQualifier::Trivial);
+    return true;
+  }
+
+  bool visitStoreBorrowInst(StoreBorrowInst *si) {
+    if (!si->getSrc()->getType().isTrivial(*si->getFunction()))
+      return false;
+    SILBuilderWithScope b(si);
+    b.emitStoreValueOperation(si->getLoc(), si->getSrc(), si->getDest(),
+                              StoreOwnershipQualifier::Trivial);
+    return eraseFromParent(si);
+  }
+
+  bool visitLoadBorrowInst(LoadBorrowInst *li) {
+    if (!li->getType().isTrivial(*li->getFunction()))
+      return false;
+    SILBuilderWithScope b(li);
+    auto newVal = b.emitLoadValueOperation(li->getLoc(), li->getOperand(),
+                                           LoadOwnershipQualifier::Trivial);
+    li->replaceAllUsesWith(newVal);
+    return eraseFromParent(li);
+  }
+
+#define RAUW_IF_TRIVIAL_RESULT(CLS)                                            \
+  bool visit##CLS##Inst(CLS##Inst *inst) {                                     \
+    if (!inst->getType().isTrivial(*inst->getFunction())) {                    \
+      return false;                                                            \
+    }                                                                          \
+    inst->replaceAllUsesWith(inst->getOperand());                              \
+    return eraseFromParent(inst);                                              \
+  }
+  RAUW_IF_TRIVIAL_RESULT(CopyValue)
+  RAUW_IF_TRIVIAL_RESULT(ExplicitCopyValue)
+  RAUW_IF_TRIVIAL_RESULT(BeginBorrow)
+#undef RAUW_IF_TRIVIAL_RESULT
+
+#define RAUW_ALWAYS(CLS)                                                       \
+  bool visit##CLS##Inst(CLS##Inst *inst) {                                     \
+    inst->replaceAllUsesWith(inst->getOperand());                              \
+    return eraseFromParent(inst);                                              \
+  }
+  RAUW_ALWAYS(MoveOnlyWrapperToCopyableValue)
+  RAUW_ALWAYS(CopyableToMoveOnlyWrapperValue)
+#undef RAUW_ALWAYS
+
+#define DELETE_IF_TRIVIAL_OP(CLS)                                              \
+  bool visit##CLS##Inst(CLS##Inst *inst) {                                     \
+    if (!inst->getOperand()->getType().isTrivial(*inst->getFunction())) {      \
+      return false;                                                            \
+    }                                                                          \
+    return eraseFromParent(inst);                                              \
+  }
+  DELETE_IF_TRIVIAL_OP(DestroyValue)
+  DELETE_IF_TRIVIAL_OP(EndBorrow)
+#undef DELETE_IF_TRIVIAL_OP
+
+#define NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_OP(CLS)                  \
+  bool visit##CLS##Inst(CLS##Inst *inst) {                                     \
+    if (!inst->getOperand()->getType().isTrivial(*inst->getFunction()))        \
+      return false;                                                            \
+    inst->setForwardingOwnershipKind(OwnershipKind::None);                     \
+    return true;                                                               \
+  }
+  NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_OP(StructExtract)
+  NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_OP(TupleExtract)
+  NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_OP(UncheckedEnumData)
+  NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_OP(SwitchEnum)
+#undef NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_OP
+
+#define NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_RESULT(CLS)              \
+  bool visit##CLS##Inst(CLS##Inst *inst) {                                     \
+    if (!inst->getType().isTrivial(*inst->getFunction()))                      \
+      return false;                                                            \
+    inst->setForwardingOwnershipKind(OwnershipKind::None);                     \
+    return true;                                                               \
+  }
+  NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_RESULT(Enum)
+  NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_RESULT(Struct)
+  NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_RESULT(Tuple)
+#undef NEED_TO_CONVERT_FORWARDING_TO_NONE_IF_TRIVIAL_RESULT
+
+#define NO_UPDATE_NEEDED(CLS)                                                  \
+  bool visit##CLS##Inst(CLS##Inst *inst) { return false; }
+  NO_UPDATE_NEEDED(AllocStack)
+  NO_UPDATE_NEEDED(DebugValue)
+  NO_UPDATE_NEEDED(StructElementAddr)
+  NO_UPDATE_NEEDED(TupleElementAddr)
+  NO_UPDATE_NEEDED(UncheckedTakeEnumDataAddr)
+  NO_UPDATE_NEEDED(DestructureTuple)
+  NO_UPDATE_NEEDED(DestructureStruct)
+  NO_UPDATE_NEEDED(SelectEnum)
+  NO_UPDATE_NEEDED(SelectValue)
+  NO_UPDATE_NEEDED(MarkDependence)
+  NO_UPDATE_NEEDED(DestroyAddr)
+  NO_UPDATE_NEEDED(DeallocStack)
+  NO_UPDATE_NEEDED(Branch)
+  NO_UPDATE_NEEDED(UncheckedAddrCast)
+  NO_UPDATE_NEEDED(RefElementAddr)
+  NO_UPDATE_NEEDED(Upcast)
+  NO_UPDATE_NEEDED(CheckedCastBranch)
+  NO_UPDATE_NEEDED(Object)
+  NO_UPDATE_NEEDED(OpenExistentialRef)
+  NO_UPDATE_NEEDED(ConvertFunction)
+  NO_UPDATE_NEEDED(RefToBridgeObject)
+  NO_UPDATE_NEEDED(BridgeObjectToRef)
+  NO_UPDATE_NEEDED(UnconditionalCheckedCast)
+#undef NO_UPDATE_NEEDED
+};
+
+} // namespace
+
+//===----------------------------------------------------------------------===//
+//                             Top Levelish Code?
+//===----------------------------------------------------------------------===//
+
+namespace {
+
+struct SILMoveOnlyTypeEliminator {
+  SILFunction *fn;
+  bool trivialOnly;
+
+  SILMoveOnlyTypeEliminator(SILFunction *fn, bool trivialOnly)
+      : fn(fn), trivialOnly(trivialOnly) {}
+
+  bool process();
+};
+
+} // namespace
+
+bool SILMoveOnlyTypeEliminator::process() {
+  bool madeChange = true;
+
+  SmallSetVector<SILArgument *, 8> touchedArgs;
+  SmallSetVector<SILInstruction *, 8> touchedInsts;
+
+  for (auto &bb : *fn) {
+    // We should (today) never have move only function arguments. Instead we
+    // convert them in the prologue block.
+    if (&bb != &fn->front()) {
+      for (auto *arg : bb.getArguments()) {
+        if (!arg->getType().isMoveOnlyWrapped())
+          continue;
+
+        // If we are looking at trivial only, skip non-trivial function args.
+        if (trivialOnly &&
+            !arg->getType().removingMoveOnlyWrapper().isTrivial(*fn))
+          continue;
+
+        arg->unsafelyEliminateMoveOnlyWrapper();
+
+        // If our new type is trivial, convert the arguments ownership to
+        // None. Otherwise, preserve the ownership kind of the argument.
+        if (arg->getType().isTrivial(*fn))
+          arg->setOwnershipKind(OwnershipKind::None);
+        touchedArgs.insert(arg);
+        for (auto *use : arg->getNonTypeDependentUses())
+          touchedInsts.insert(use->getUser());
+      }
+    }
+
+    for (auto &ii : bb) {
+      for (SILValue v : ii.getResults()) {
+        if (!v->getType().isMoveOnlyWrapped())
+          continue;
+
+        if (trivialOnly &&
+            !v->getType().removingMoveOnlyWrapper().isTrivial(*fn))
+          continue;
+
+        v->unsafelyEliminateMoveOnlyWrapper();
+        touchedInsts.insert(&ii);
+
+        // Add all users as well. This ensures we visit things like
+        // destroy_value and end_borrow.
+        for (auto *use : v->getNonTypeDependentUses())
+          touchedInsts.insert(use->getUser());
+        madeChange = true;
+      }
+    }
+  }
+
+  SILMoveOnlyTypeEliminatorVisitor visitor(touchedArgs);
+  while (!touchedInsts.empty()) {
+    visitor.visit(touchedInsts.pop_back_val());
+  }
+
+  return madeChange;
+}
+
+//===----------------------------------------------------------------------===//
+//                            Top Level Entrypoint
+//===----------------------------------------------------------------------===//
+
+namespace {
+
+struct SILMoveOnlyTypeEliminatorPass : SILFunctionTransform {
+  bool trivialOnly;
+
+  SILMoveOnlyTypeEliminatorPass(bool trivialOnly)
+      : SILFunctionTransform(), trivialOnly(trivialOnly) {}
+
+  void run() override {
+    auto *fn = getFunction();
+
+    // Don't rerun on deserialized functions. We lower trivial things earlier
+    // during Raw SIL.
+    if (getFunction()->wasDeserializedCanonical())
+      return;
+
+    assert(fn->getModule().getStage() == SILStage::Raw &&
+           "Should only run on Raw SIL");
+
+    if (SILMoveOnlyTypeEliminator(getFunction(), trivialOnly).process()) {
+      invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
+    }
+  }
+};
+
+} // anonymous namespace
+
+SILTransform *swift::createTrivialMoveOnlyTypeEliminator() {
+  return new SILMoveOnlyTypeEliminatorPass(true /*trivial only*/);
+}
+
+SILTransform *swift::createMoveOnlyTypeEliminator() {
+  return new SILMoveOnlyTypeEliminatorPass(false /*trivial only*/);
+}
diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp
@@ -173,6 +173,16 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
                                          //   value.
   P.addMoveOnlyChecker();                // Check noImplicitCopy isn't copied.
 
+  // Now that we have run move only checking, eliminate SILMoveOnly wrapped
+  // trivial types from the IR. We cannot introduce extra "copies" of trivial
+  // things so we can simplify our implementation by eliminating them here.
+  P.addTrivialMoveOnlyTypeEliminator();
+
+  // As a temporary measure, we also eliminate move only for non-trivial types
+  // until we can audit the later part of the pipeline. Eventually, this should
+  // occur before IRGen.
+  P.addMoveOnlyTypeEliminator();
+
   // This phase performs optimizations necessary for correct interoperation of
   // Swift os log APIs with C os_log ABIs.
   // Pass dependencies: this pass depends on MandatoryInlining and Mandatory
diff --git a/test/SILOptimizer/move_only_checker_trivial.swift b/test/SILOptimizer/move_only_checker_trivial.swift
diff --git a/test/SILOptimizer/move_only_type_eliminator.sil b/test/SILOptimizer/move_only_type_eliminator.sil
