Merge pull request #40040 from gottesmm/pr-270a55632e2b31495bf1b801e01cc69e742ddf6b

[moveOnly] Add checker that validates that if a let copyable value has _move applied to it, the let doesn't have any later uses.

Author: gottesmm

include/swift/AST/DiagnosticsParse.def

@@ -633,6 +633,8 @@ ERROR(sil_keypath_index_operand_type_conflict,none,
 ERROR(sil_keypath_no_use_of_operand_in_pattern,none,
       "operand %0 is not referenced by any component in the pattern",
       (unsigned))
+ERROR(sil_movevalue_invalid_optional_attribute,none,
+      "Optional attribute '[%0]' can not be applied to move_value", (StringRef))
 
 // SIL Basic Blocks
 ERROR(expected_sil_block_name,none,

include/swift/AST/DiagnosticsSIL.def

@@ -733,5 +733,15 @@ ERROR(sil_moveonlychecker_value_consumed_more_than_once, none,
 NOTE(sil_moveonlychecker_consuming_use_here, none,
      "consuming use", ())
 
+// move kills copyable values checker diagnostics
+ERROR(sil_movekillscopyablevalue_value_consumed_more_than_once, none,
+      "'%0' used after being moved", (StringRef))
+NOTE(sil_movekillscopyablevalue_move_here, none,
+     "move here", ())
+NOTE(sil_movekillscopyablevalue_use_here, none,
+     "use here", ())
+NOTE(sil_movekillscopyablevalue_value_consumed_in_loop, none,
+     "cyclic move here. move will occur multiple times in the loop", ())
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

include/swift/SIL/PrunedLiveness.h

@@ -91,6 +91,7 @@
 #define SWIFT_SILOPTIMIZER_UTILS_PRUNEDLIVENESS_H
 
 #include "swift/SIL/SILBasicBlock.h"
+#include "llvm/ADT/MapVector.h"
 
 namespace swift {
 
@@ -221,11 +222,19 @@ class PrunedLiveness {
   // they may be the last use in the block.
   //
   // Non-lifetime-ending within a LiveOut block are uninteresting.
-  llvm::SmallDenseMap<SILInstruction *, bool, 8> users;
+  llvm::SmallMapVector<SILInstruction *, bool, 8> users;
+
+  /// A side array that stores any non lifetime ending uses we find in live out
+  /// blocks. This is used to enable our callers to emit errors on non-lifetime
+  /// ending uses that extend liveness into a loop body.
+  SmallSetVector<SILInstruction *, 8> *nonLifetimeEndingUsesInLiveOut;
 
 public:
-  PrunedLiveness(SmallVectorImpl<SILBasicBlock *> *discoveredBlocks = nullptr)
-      : liveBlocks(discoveredBlocks) {}
+  PrunedLiveness(SmallVectorImpl<SILBasicBlock *> *discoveredBlocks = nullptr,
+                 SmallSetVector<SILInstruction *, 8>
+                     *nonLifetimeEndingUsesInLiveOut = nullptr)
+      : liveBlocks(discoveredBlocks),
+        nonLifetimeEndingUsesInLiveOut(nonLifetimeEndingUsesInLiveOut) {}
 
   bool empty() const {
     assert(!liveBlocks.empty() || users.empty());
@@ -235,6 +244,8 @@ class PrunedLiveness {
   void clear() {
     liveBlocks.clear();
     users.clear();
+    if (nonLifetimeEndingUsesInLiveOut)
+      nonLifetimeEndingUsesInLiveOut->clear();
   }
 
   unsigned numLiveBlocks() const { return liveBlocks.numLiveBlocks(); }
@@ -245,6 +256,47 @@ class PrunedLiveness {
     return liveBlocks.getDiscoveredBlocks();
   }
 
+  using NonLifetimeEndingUsesInLiveOutRange =
+      iterator_range<SILInstruction *const *>;
+
+  NonLifetimeEndingUsesInLiveOutRange
+  getNonLifetimeEndingUsesInLiveOut() const {
+    assert(nonLifetimeEndingUsesInLiveOut &&
+           "Called without passing in nonLifetimeEndingUsesInLiveOut to "
+           "constructor?!");
+    return llvm::make_range(nonLifetimeEndingUsesInLiveOut->begin(),
+                            nonLifetimeEndingUsesInLiveOut->end());
+  }
+
+  using NonLifetimeEndingUsesInLiveOutBlocksRange =
+      TransformRange<NonLifetimeEndingUsesInLiveOutRange,
+                     function_ref<SILBasicBlock *(const SILInstruction *&)>>;
+  NonLifetimeEndingUsesInLiveOutBlocksRange
+  getNonLifetimeEndingUsesInLiveOutBlocks() const {
+    function_ref<SILBasicBlock *(const SILInstruction *&)> op;
+    op = [](const SILInstruction *&ptr) -> SILBasicBlock * {
+      return ptr->getParent();
+    };
+    return NonLifetimeEndingUsesInLiveOutBlocksRange(
+        getNonLifetimeEndingUsesInLiveOut(), op);
+  }
+
+  using UserRange = iterator_range<const std::pair<SILInstruction *, bool> *>;
+  UserRange getAllUsers() const {
+    return llvm::make_range(users.begin(), users.end());
+  }
+
+  using UserBlockRange = TransformRange<
+      UserRange,
+      function_ref<SILBasicBlock *(const std::pair<SILInstruction *, bool> &)>>;
+  UserBlockRange getAllUserBlocks() const {
+    function_ref<SILBasicBlock *(const std::pair<SILInstruction *, bool> &)> op;
+    op = [](const std::pair<SILInstruction *, bool> &pair) -> SILBasicBlock * {
+      return pair.first->getParent();
+    };
+    return UserBlockRange(getAllUsers(), op);
+  }
+
   void initializeDefBlock(SILBasicBlock *defBB) {
     liveBlocks.initializeDefBlock(defBB);
   }

include/swift/SIL/SILCloner.h

@@ -1731,9 +1731,10 @@ void SILCloner<ImplClass>::visitExplicitCopyValueInst(
 template <typename ImplClass>
 void SILCloner<ImplClass>::visitMoveValueInst(MoveValueInst *Inst) {
   getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
-  recordClonedInstruction(
-      Inst, getBuilder().createMoveValue(getOpLocation(Inst->getLoc()),
-                                         getOpValue(Inst->getOperand())));
+  auto *MVI = getBuilder().createMoveValue(getOpLocation(Inst->getLoc()),
+                                           getOpValue(Inst->getOperand()));
+  MVI->setAllowsDiagnostics(Inst->getAllowDiagnostics());
+  recordClonedInstruction(Inst, MVI);
 }
 
 template <typename ImplClass>

include/swift/SIL/SILInstruction.h

@@ -7337,8 +7337,17 @@ class MoveValueInst
                                   SingleValueInstruction> {
   friend class SILBuilder;
 
+  /// If set to true, we should emit the kill diagnostic for this move_value. If
+  /// set to false, we shouldn't emit such a diagnostic. This is a short term
+  /// addition until we get MoveOnly wrapper types into the SIL type system.
+  bool allowDiagnostics = false;
+
   MoveValueInst(SILDebugLocation DebugLoc, SILValue operand)
       : UnaryInstructionBase(DebugLoc, operand, operand->getType()) {}
+
+public:
+  bool getAllowDiagnostics() const { return allowDiagnostics; }
+  void setAllowsDiagnostics(bool newValue) { allowDiagnostics = newValue; }
 };
 
 /// Given an object reference, return true iff it is non-nil and refers

include/swift/SILOptimizer/PassManager/Passes.def

@@ -422,6 +422,9 @@ PASS(AssemblyVisionRemarkGenerator, "assembly-vision-remark-generator",
      "Emit assembly vision remarks that provide source level guidance of where runtime calls ended up")
 PASS(MoveOnlyChecker, "sil-move-only-checker",
      "Pass that enforces move only invariants on raw SIL")
+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(PruneVTables, "prune-vtables",
      "Mark class methods that do not require vtable dispatch")
 PASS_RANGE(AllPasses, AADumper, PruneVTables)

lib/SIL/IR/SILPrinter.cpp

@@ -1884,6 +1884,8 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
   }
 
   void visitMoveValueInst(MoveValueInst *I) {
+    if (I->getAllowDiagnostics())
+      *this << "[allows_diagnostics] ";
     *this << getIDAndType(I->getOperand());
   }
 

lib/SIL/Parser/ParseSIL.cpp

@@ -3156,7 +3156,6 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
     UNARY_INSTRUCTION(DestroyAddr)
     UNARY_INSTRUCTION(CopyValue)
     UNARY_INSTRUCTION(ExplicitCopyValue)
-    UNARY_INSTRUCTION(MoveValue)
     UNARY_INSTRUCTION(EndBorrow)
     UNARY_INSTRUCTION(DestructureStruct)
     UNARY_INSTRUCTION(DestructureTuple)
@@ -3266,6 +3265,28 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
     break;
   }
 
+  case SILInstructionKind::MoveValueInst: {
+    bool allowsDiagnostics = false;
+    StringRef AttrName;
+    if (parseSILOptional(AttrName, *this)) {
+      if (!AttrName.equals("allows_diagnostics")) {
+        auto diag = diag::sil_movevalue_invalid_optional_attribute;
+        P.diagnose(InstLoc.getSourceLoc(), diag, AttrName);
+        return true;
+      }
+      allowsDiagnostics = true;
+    }
+
+    if (parseTypedValueRef(Val, B))
+      return true;
+    if (parseSILDebugLocation(InstLoc, B))
+      return true;
+    auto *MVI = B.createMoveValue(InstLoc, Val);
+    MVI->setAllowsDiagnostics(allowsDiagnostics);
+    ResultVal = MVI;
+    break;
+  }
+
   case SILInstructionKind::LoadInst: {
     Optional<LoadOwnershipQualifier> Qualifier;
     SourceLoc AddrLoc;

lib/SIL/Utils/PrunedLiveness.cpp

@@ -80,6 +80,8 @@ void PrunedLiveness::updateForUse(SILInstruction *user, bool lifetimeEnding) {
   // Record all uses of blocks on the liveness boundary. For blocks marked
   // LiveWithin, the boundary is considered to be the last use in the block.
   if (!lifetimeEnding && useBlockLive == PrunedLiveBlocks::LiveOut) {
+    if (nonLifetimeEndingUsesInLiveOut)
+      nonLifetimeEndingUsesInLiveOut->insert(user);
     return;
   }
   // Note that a user may use the current value from multiple operands. If any
@@ -93,7 +95,7 @@ void PrunedLiveness::updateForUse(SILInstruction *user, bool lifetimeEnding) {
   //
   // This call is not considered the end of %val's lifetime. The @owned
   // argument must be copied.
-  auto iterAndSuccess = users.try_emplace(user, lifetimeEnding);
+  auto iterAndSuccess = users.insert({user, lifetimeEnding});
   if (!iterAndSuccess.second)
     iterAndSuccess.first->second &= lifetimeEnding;
 }

lib/SILOptimizer/Mandatory/CMakeLists.txt

@@ -18,6 +18,7 @@ target_sources(swiftSILOptimizer PRIVATE
   LowerHopToActor.cpp
   MandatoryInlining.cpp
   MoveOnlyChecker.cpp
+  MoveKillsCopyableValuesChecker.cpp
   NestedSemanticFunctionCheck.cpp
   OptimizeHopToExecutor.cpp
   PerformanceDiagnostics.cpp