Merge pull request #34742 from atrick/fix-unchecked-ownership

Specify the unchecked_ownership_conversion instruction

Author: atrick

docs/SIL.rst

@@ -5894,6 +5894,25 @@ This instruction is assumed to forward a fixed ownership (set upon its
 construction) and lowers to 'unchecked_bitwise_cast' in non-ossa code. This
 causes the cast to lose its guarantee of layout-compatibility.
 
+unchecked_ownership_conversion
+``````````````````````````````
+::
+
+   sil-instruction ::= 'unchecked_ownership_conversion' sil-operand ',' sil-value-ownership-kind 'to' sil-value-ownership-kind
+
+   %1 = unchecked_ownership_conversion %0 : $A, @guaranteed to @owned
+
+Converts its operand to an identical value of the same type but with
+different ownership without performing any semantic operations
+normally required by for ownership conversion.
+
+This is used in Objective-C compatible destructors to convert a
+guaranteed parameter to an owned parameter without performing a
+semantic copy.
+
+The resulting value must meet the usual ownership requirements; for
+example, a trivial type must have '.none' ownership.
+
 ref_to_raw_pointer
 ``````````````````
 ::

include/swift/SIL/SILNodes.def

@@ -575,7 +575,7 @@ ABSTRACT_VALUE_AND_INST(SingleValueInstruction, ValueBase, SILInstruction)
                     SingleValueInstruction, MayHaveSideEffects, DoesNotRelease)
 #include "swift/AST/ReferenceStorage.def"
   SINGLE_VALUE_INST(UncheckedOwnershipConversionInst, unchecked_ownership_conversion,
-                    SingleValueInstruction, MayHaveSideEffects, MayRelease)
+                    SingleValueInstruction, None, MayRelease)
 
   // IsUnique does not actually write to memory but should be modeled
   // as such. Its operand is a pointer to an object reference. The

lib/SIL/Utils/MemAccessUtils.cpp

@@ -369,8 +369,9 @@ bool swift::isLetAddress(SILValue address) {
 // RC-identical would confuse ARC optimization, which might eliminate a retain
 // of such an object completely.
 //
-// The SILVerifier checks that none of these operations cast a nontrivial value
-// to a reference except unconditional_checked_cast[_value].
+// The SILVerifier checks that none of these operations cast a trivial value to
+// a reference except unconditional_checked_cast[_value], which is checked By
+// SILDynamicCastInst::isRCIdentityPreserving().
 bool swift::isRCIdentityPreservingCast(SingleValueInstruction *svi) {
   switch (svi->getKind()) {
   default:

lib/SIL/Verifier/LoadBorrowImmutabilityChecker.cpp

@@ -268,12 +268,11 @@ bool GatherWritesVisitor::visitUse(Operand *op, AccessUseType useTy) {
   if (!op->get()->getType().isAddress() && !user->mayWriteToMemory()) {
     return true;
   }
-  // If we did not recognize the user, just return conservatively that it was
-  // written to in a way we did not understand.
+  // If we did not recognize the user, print additional error diagnostics and
+  // return false to force SIL verification to fail.
   llvm::errs() << "Function: " << user->getFunction()->getName() << "\n";
   llvm::errs() << "Value: " << op->get();
   llvm::errs() << "Unknown instruction: " << *user;
-  llvm::report_fatal_error("Unexpected instruction using borrowed address?!");
   return false;
 }
 

lib/SIL/Verifier/SILVerifier.cpp

@@ -1958,6 +1958,17 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
         "Inst with qualified ownership in a function that is not qualified");
   }
 
+  void checkUncheckedOwnershipConversionInst(
+    UncheckedOwnershipConversionInst *uoci) {
+    require(
+        F.hasOwnership(),
+        "Inst with qualified ownership in a function that is not qualified");
+    require(!uoci->getType().isAddress(),
+            "cannot convert ownership of an address");
+    require(uoci->getType() == uoci->getOperand()->getType(),
+            "converting ownership does not affect the type");
+  }
+
   template <class AI>
   void checkAccessEnforcement(AI *AccessInst) {
     if (AccessInst->getModule().getStage() != SILStage::Raw) {

test/SILOptimizer/load_borrow_verify.sil

@@ -76,4 +76,24 @@ bb0(%0 : @owned $AnyObject):
   dealloc_stack %1 : $*Optional<AnyObject>
   %99 = tuple ()
   return %99 : $()
-}
+}
+
+// unchecked_ownership_conversion should not be considered a write to memory.
+class ObjectWrapper {
+  var object: AnyObject
+}
+
+// CHECK-LABEL: sil [ossa] @unchecked_ownership_conversion_test : $@convention(thin) (@guaranteed ObjectWrapper) -> @owned ObjectWrapper {
+// CHECK: bb0(%0 : @guaranteed $ObjectWrapper):
+// CHECK:   load_borrow
+// CHECK:   unchecked_ownership_conversion %0 : $ObjectWrapper, @guaranteed to @owned
+// CHECK:   end_borrow
+// CHECK-LABEL: } // end sil function 'unchecked_ownership_conversion_test'
+sil [ossa] @unchecked_ownership_conversion_test : $@convention(thin) (@guaranteed ObjectWrapper) -> @owned ObjectWrapper {
+bb0(%0 : @guaranteed $ObjectWrapper):
+  %1 = ref_element_addr %0 : $ObjectWrapper, #ObjectWrapper.object
+  %2 = load_borrow %1 : $*AnyObject
+  %3 = unchecked_ownership_conversion %0 : $ObjectWrapper, @guaranteed to @owned
+  end_borrow %2 : $AnyObject
+  return %3 : $ObjectWrapper
+}