Merge pull request #77160 from atrick/moveonly_markdep

Add move-only checker support for dependencies

Author: atrick

include/swift/SIL/MemAccessUtils.h

@@ -1650,6 +1650,21 @@ inline bool isAccessStorageIdentityCast(SingleValueInstruction *svi) {
   }
 }
 
+// Strip access markers and casts that preserve the address type.
+//
+// Consider using RelativeAccessStorageWithBase::compute().
+inline SILValue stripAccessAndIdentityCasts(SILValue v) {
+  if (auto *bai = dyn_cast<BeginAccessInst>(v)) {
+    return stripAccessAndIdentityCasts(bai->getOperand());
+  }
+  if (auto *svi = dyn_cast<SingleValueInstruction>(v)) {
+    if (isAccessStorageIdentityCast(svi)) {
+      return stripAccessAndIdentityCasts(svi->getAllOperands()[0].get());
+    }
+  }
+  return v;
+}
+
 /// An address, pointer, or box cast that occurs outside of the formal
 /// access. These convert the base of accessed storage without affecting the
 /// AccessPath. Useful for both use-def and def-use traversal. The source

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -2379,7 +2379,7 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
       if (moveChecker.canonicalizer.foundAnyConsumingUses()) {
         LLVM_DEBUG(llvm::dbgs()
                    << "Found mark must check [nocopy] error: " << *user);
-        auto operand = stripAccessMarkers(markedValue->getOperand());
+        auto operand = stripAccessAndIdentityCasts(markedValue->getOperand());
         auto *fArg = dyn_cast<SILFunctionArgument>(operand);
         auto *ptrToAddr = dyn_cast<PointerToAddressInst>(operand);
 

lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp

@@ -1540,6 +1540,12 @@ static bool gatherBorrows(SILValue rootValue,
     // escape. Is it legal to canonicalize ForwardingUnowned?
     case OperandOwnership::ForwardingUnowned:
     case OperandOwnership::PointerEscape:
+      if (auto *md = dyn_cast<MarkDependenceInst>(use->getUser())) {
+        // mark_depenence uses only keep its base value alive; they do not use
+        // the base value itself and are irrelevant for destructuring.
+        if (use == &md->getOperandRef(MarkDependenceInst::Base))
+          continue;
+      }
       return false;
 
     case OperandOwnership::InstantaneousUse:

lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp

@@ -219,15 +219,12 @@ bool MoveOnlyObjectCheckerPImpl::checkForSameInstMultipleUseErrors(
     case OperandOwnership::NonUse:
       continue;
 
-    // Conservatively treat a conversion to an unowned value as a pointer
-    // escape. If we see this in the SIL, fail and return false so we emit a
-    // "compiler doesn't understand error".
     case OperandOwnership::ForwardingUnowned:
     case OperandOwnership::PointerEscape:
     case OperandOwnership::BitwiseEscape:
-      LLVM_DEBUG(llvm::dbgs()
-                 << "        Found forwarding unowned or escape!\n");
-      return false;
+      // None of the "unowned" uses can consume the original value. Simply
+      // ignore them.
+      continue;
 
     case OperandOwnership::TrivialUse:
     case OperandOwnership::InstantaneousUse:

lib/SILOptimizer/Utils/VariableNameUtils.cpp

@@ -642,6 +642,7 @@ SILValue VariableNameInferrer::findDebugInfoProvidingValueHelper(
         isa<ProjectBoxInst>(searchValue) || isa<CopyValueInst>(searchValue) ||
         isa<ConvertFunctionInst>(searchValue) ||
         isa<MarkUninitializedInst>(searchValue) ||
+        isa<MarkDependenceInst>(searchValue) ||
         isa<CopyableToMoveOnlyWrapperAddrInst>(searchValue) ||
         isa<MoveOnlyWrapperToCopyableAddrInst>(searchValue) ||
         isa<MoveOnlyWrapperToCopyableValueInst>(searchValue) ||

test/SILOptimizer/moveonly_unsafeAddress.sil

@@ -0,0 +1,57 @@
+// RUN: %target-sil-opt -sil-move-only-address-checker -enable-sil-verify-all -verify %s | %FileCheck %s
+
+// Verify move-only diagnostics on unsafe addressors.
+//
+// Note: .sil tests cannot easily recover variable names, so we see 'unknown' variables.
+
+sil_stage raw
+
+import Builtin
+import Swift
+import SwiftShims
+
+class C {
+  deinit
+  init()
+}
+
+struct NC : ~Copyable {
+  @_hasStorage @_hasInitialValue var c: C { get set }
+  deinit
+  init()
+  init(c: C = C())
+}
+
+struct SNC : ~Copyable {
+  var data: NC { get }
+  var mutableData: NC { get set }
+  init()
+}
+
+sil @unsafeAddressor : $@convention(method) (@guaranteed SNC) -> UnsafePointer<NC>
+
+sil @consumeNC : $@convention(thin) (@owned NC) -> ()
+
+// CHECK-LABEL: sil hidden [ossa] @test_read_borrow : $@convention(thin) (@inout SNC) -> () {
+sil hidden [ossa] @test_read_borrow : $@convention(thin) (@inout SNC) -> () {
+bb0(%0 : $*SNC):
+  debug_value %0 : $*SNC, var, name "snc", argno 1, expr op_deref
+  %2 = mark_unresolved_non_copyable_value [consumable_and_assignable] %0 : $*SNC
+  %3 = begin_access [read] [static] %2 : $*SNC
+  %4 = load_borrow [unchecked] %3 : $*SNC
+  %5 = function_ref @unsafeAddressor : $@convention(method) (@guaranteed SNC) -> UnsafePointer<NC>
+  %6 = apply %5(%4) : $@convention(method) (@guaranteed SNC) -> UnsafePointer<NC>
+  %7 = struct_extract %6 : $UnsafePointer<NC>, #UnsafePointer._rawValue
+  %8 = pointer_to_address %7 : $Builtin.RawPointer to [strict] $*NC
+  %9 = mark_dependence [unresolved] %8 : $*NC on %3 : $*SNC
+  %10 = begin_access [read] [unsafe] %9 : $*NC
+  %11 = mark_unresolved_non_copyable_value [no_consume_or_assign] %10 : $*NC  // expected-error {{'unknown' is borrowed and cannot be consumed}}
+  %12 = load [copy] %11 : $*NC
+  end_access %10 : $*NC
+  end_borrow %4 : $SNC
+  end_access %3 : $*SNC
+  %16 = function_ref @consumeNC : $@convention(thin) (@owned NC) -> ()
+  %17 = apply %16(%12) : $@convention(thin) (@owned NC) -> () // expected-note {{consumed here}}
+  %18 = tuple ()
+  return %18 : $()
+}