Merge pull request #61143 from gottesmm/pr-7e04f5d62651e78c425d1a21b829abaf5096b2b0

gottesmm · web-flow · commit c72ddcfb54e5 · 2022-09-15T23:17:47.000-07:00
[move-only] Add support for trivial move only var/mutating self/inout.
diff --git a/include/swift/SIL/PrunedLiveness.h b/include/swift/SIL/PrunedLiveness.h
@@ -752,17 +752,30 @@ struct TypeTreeLeafTypeRange {
   SubElementNumber startEltOffset;
   SubElementNumber endEltOffset;
 
+private:
+  TypeTreeLeafTypeRange(SubElementNumber start, SubElementNumber end)
+      : startEltOffset(start), endEltOffset(end) {}
+
+public:
   /// The leaf type range for the entire type tree.
   TypeTreeLeafTypeRange(SILValue rootAddress)
       : startEltOffset(0), endEltOffset(TypeSubElementCount(rootAddress)) {}
 
   /// The leaf type sub-range of the type tree of \p rootAddress, consisting of
   /// \p projectedAddress and all of \p projectedAddress's descendent fields in
   /// the type tree.
-  TypeTreeLeafTypeRange(SILValue projectedAddress, SILValue rootAddress)
-      : startEltOffset(
-            *SubElementNumber::compute(projectedAddress, rootAddress)),
-        endEltOffset(startEltOffset + TypeSubElementCount(projectedAddress)) {}
+  ///
+  /// \returns None if we are unable to understand the path in between \p
+  /// projectedAddress and \p rootAddress.
+  static Optional<TypeTreeLeafTypeRange> get(SILValue projectedAddress,
+                                             SILValue rootAddress) {
+    auto startEltOffset =
+        SubElementNumber::compute(projectedAddress, rootAddress);
+    if (!startEltOffset)
+      return None;
+    return {{*startEltOffset,
+             *startEltOffset + TypeSubElementCount(projectedAddress)}};
+  }
 
   /// Is the given leaf type specified by \p singleLeafElementNumber apart of
   /// our \p range of leaf type values in the our larger type.
diff --git a/lib/SIL/Utils/PrunedLiveness.cpp b/lib/SIL/Utils/PrunedLiveness.cpp
@@ -507,14 +507,12 @@ SubElementNumber::compute(SILValue projectionDerivedFromRoot,
       continue;
     }
 
-#ifndef NDEBUG
-    if (!isa<InitExistentialAddrInst>(projectionDerivedFromRoot)) {
-      llvm::errs() << "Unknown access path instruction!\n";
-      llvm::errs() << "Value: " << *projectionDerivedFromRoot;
-      llvm_unreachable("standard error");
-    }
-#endif
-    // Cannot promote loads and stores from within an existential projection.
+    // If we do not know how to handle this case, just return None.
+    //
+    // NOTE: We use to assert here, but since this is used for diagnostics, we
+    // really do not want to abort. Instead, our caller can choose to abort if
+    // they get back a None. This ensures that we do not abort in cases where we
+    // just want to emit to the user a "I do not understand" error.
     return None;
   }
 }
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyAddressChecker.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyAddressChecker.cpp
@@ -841,17 +841,22 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
 
     // TODO: What about copy_addr of itself. We really should just pre-process
     // those maybe.
+    auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+    if (!leafRange)
+      return false;
+
     assert(!useState.initInsts.count(user));
-    useState.initInsts.insert(
-        {user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
+    useState.initInsts.insert({user, *leafRange});
     return true;
   }
 
   if (::memInstMustReinitialize(op)) {
     LLVM_DEBUG(llvm::dbgs() << "Found reinit: " << *user);
     assert(!useState.reinitInsts.count(user));
-    useState.reinitInsts.insert(
-        {user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
+    auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+    if (!leafRange)
+      return false;
+    useState.reinitInsts.insert({user, *leafRange});
     return true;
   }
 
@@ -891,9 +896,12 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
       return true;
     }
 
+    auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+    if (!leafRange)
+      return false;
+
     LLVM_DEBUG(llvm::dbgs() << "Found take: " << *user);
-    useState.takeInsts.insert(
-        {user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
+    useState.takeInsts.insert({user, *leafRange});
     return true;
   }
 
@@ -935,9 +943,13 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
 
         // If set, this will tell the checker that we can change this load into
         // a load_borrow.
+        auto leafRange =
+            TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+        if (!leafRange)
+          return false;
+
         LLVM_DEBUG(llvm::dbgs() << "Found potential borrow: " << *user);
-        useState.borrows.insert(
-            {user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
+        useState.borrows.insert({user, *leafRange});
         return true;
       }
 
@@ -963,33 +975,41 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
 
       // Then if we had any final consuming uses, mark that this liveness use is
       // a take and if not, mark this as a borrow.
+      auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+      if (!leafRange)
+        return false;
+
       if (moveChecker.finalConsumingUses.empty()) {
         LLVM_DEBUG(llvm::dbgs() << "Found borrow inst: " << *user);
-        useState.borrows.insert(
-            {user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
+        useState.borrows.insert({user, *leafRange});
       } else {
         LLVM_DEBUG(llvm::dbgs() << "Found take inst: " << *user);
-        useState.takeInsts.insert(
-            {user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
+        useState.takeInsts.insert({user, *leafRange});
       }
       return true;
     }
   }
 
   // Now that we have handled or loadTakeOrCopy, we need to now track our Takes.
   if (::memInstMustConsume(op)) {
+    auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+    if (!leafRange)
+      return false;
     LLVM_DEBUG(llvm::dbgs() << "Pure consuming use: " << *user);
-    useState.takeInsts.insert(
-        {user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
+    useState.takeInsts.insert({user, *leafRange});
     return true;
   }
 
   if (auto fas = FullApplySite::isa(op->getUser())) {
     switch (fas.getArgumentConvention(*op)) {
-    case SILArgumentConvention::Indirect_In_Guaranteed:
-      useState.livenessUses.insert(
-          {user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
+    case SILArgumentConvention::Indirect_In_Guaranteed: {
+      auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+      if (!leafRange)
+        return false;
+
+      useState.livenessUses.insert({user, *leafRange});
       return true;
+    }
 
     case SILArgumentConvention::Indirect_Inout:
     case SILArgumentConvention::Indirect_InoutAliasable:
@@ -1006,6 +1026,10 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
   // If we don't fit into any of those categories, just track as a liveness
   // use. We assume all such uses must only be reads to the memory. So we assert
   // to be careful.
+  auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+  if (!leafRange)
+    return false;
+
   LLVM_DEBUG(llvm::dbgs() << "Found liveness use: " << *user);
 #ifndef NDEBUG
   if (user->mayWriteToMemory()) {
@@ -1014,8 +1038,7 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
     llvm_unreachable("standard failure");
   }
 #endif
-  useState.livenessUses.insert(
-      {user, TypeTreeLeafTypeRange(op->get(), getRootAddress())});
+  useState.livenessUses.insert({user, *leafRange});
 
   return true;
 }
@@ -1036,8 +1059,26 @@ void MoveOnlyChecker::searchForCandidateMarkMustChecks() {
       // Skip any alloc_box due to heap to stack failing on a box capture. This
       // will just cause an error.
       if (auto *pbi = dyn_cast<ProjectBoxInst>(mmci->getOperand())) {
+        if (isa<AllocBoxInst>(pbi->getOperand())) {
+          LLVM_DEBUG(
+              llvm::dbgs()
+              << "Early emitting diagnostic for unsupported alloc box!\n");
+          if (mmci->getType().isMoveOnlyWrapped()) {
+            diagnose(fn->getASTContext(), mmci->getLoc().getSourceLoc(),
+                     diag::sil_moveonlychecker_not_understand_no_implicit_copy);
+          } else {
+            diagnose(fn->getASTContext(), mmci->getLoc().getSourceLoc(),
+                     diag::sil_moveonlychecker_not_understand_moveonly);
+          }
+          valuesWithDiagnostics.insert(mmci);
+          continue;
+        }
+
         if (auto *bbi = dyn_cast<BeginBorrowInst>(pbi->getOperand())) {
           if (isa<AllocBoxInst>(bbi->getOperand())) {
+            LLVM_DEBUG(
+                llvm::dbgs()
+                << "Early emitting diagnostic for unsupported alloc box!\n");
             if (mmci->getType().isMoveOnlyWrapped()) {
               diagnose(
                   fn->getASTContext(), mmci->getLoc().getSourceLoc(),
@@ -1604,6 +1645,8 @@ bool MoveOnlyChecker::check() {
 
     // Perform our address check.
     if (!performSingleCheck(markedValue)) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "Failed to perform single check! Emitting error!\n");
       // If we fail the address check in some way, set the diagnose!
       if (markedValue->getType().isMoveOnlyWrapped()) {
         diagnose(fn->getASTContext(), markedValue->getLoc().getSourceLoc(),
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyObjectChecker.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyObjectChecker.cpp
@@ -114,7 +114,7 @@ bool MoveOnlyChecker::searchForCandidateMarkMustChecks() {
       auto *mmci = dyn_cast<MarkMustCheckInst>(&*ii);
       ++ii;
 
-      if (!mmci || !mmci->hasMoveCheckerKind())
+      if (!mmci || !mmci->hasMoveCheckerKind() || !mmci->getType().isObject())
         continue;
 
       // Handle guaranteed/owned move arguments and values.
diff --git a/test/SILOptimizer/moveonly_addresschecker_diagnostics.swift b/test/SILOptimizer/moveonly_addresschecker_diagnostics.swift
@@ -1836,7 +1836,6 @@ public func closureClassUseAfterConsumeArg(_ argX: inout Klass) {
 public func closureCaptureClassUseAfterConsume() {
     var x2 = Klass()
     // expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
     x2 = Klass()
     let f = {
         classUseMoveOnlyWithoutEscaping(x2)
@@ -1849,7 +1848,6 @@ public func closureCaptureClassUseAfterConsume() {
 public func closureCaptureClassUseAfterConsumeError() {
     var x2 = Klass()
     // expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
     x2 = Klass()
     let f = {
         classUseMoveOnlyWithoutEscaping(x2)
@@ -1925,9 +1923,8 @@ public func deferCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
 public func closureAndDeferCaptureClassUseAfterConsume() {
     var x2 = Klass()
     // expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
-    // expected-error @-3 {{'x2' consumed but not reinitialized before end of function}}
-    // expected-error @-4 {{'x2' consumed more than once}}
+    // expected-error @-2 {{'x2' consumed but not reinitialized before end of function}}
+    // expected-error @-3 {{'x2' consumed more than once}}
     x2 = Klass()
     let f = {
         defer {
@@ -1946,8 +1943,7 @@ public func closureAndDeferCaptureClassUseAfterConsume2() {
     var x2 = Klass()
     // expected-error @-1 {{'x2' consumed but not reinitialized before end of function}}
     // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
-    // expected-error @-3 {{Usage of a move only type that the move checker does not know how to check!}}
-    // expected-error @-4 {{'x2' consumed more than once}}
+    // expected-error @-3 {{'x2' consumed more than once}}
     x2 = Klass()
     let f = {
         classConsume(x2)
@@ -1967,8 +1963,7 @@ public func closureAndDeferCaptureClassUseAfterConsume3() {
     var x2 = Klass()
     // expected-error @-1 {{'x2' consumed but not reinitialized before end of function}}
     // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
-    // expected-error @-3 {{Usage of a move only type that the move checker does not know how to check!}}
-    // expected-error @-4 {{'x2' consumed more than once}}
+    // expected-error @-3 {{'x2' consumed more than once}}
     x2 = Klass()
     let f = {
         classConsume(x2)
@@ -2008,7 +2003,6 @@ public func closureAndDeferCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
 public func closureAndClosureCaptureClassUseAfterConsume() {
     var x2 = Klass()
     // expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
     x2 = Klass()
     let f = {
         let g = {
@@ -2024,7 +2018,6 @@ public func closureAndClosureCaptureClassUseAfterConsume() {
 public func closureAndClosureCaptureClassUseAfterConsume2() {
     var x2 = Klass()
     // expected-error @-1 {{Usage of a move only type that the move checker does not know how to check!}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
     x2 = Klass()
     let f = {
         let g = {
diff --git a/test/SILOptimizer/moveonly_trivial_addresschecker_diagnostics.swift b/test/SILOptimizer/moveonly_trivial_addresschecker_diagnostics.swift
