[ownership] When RAUWing an unowned value with a owned/guaranteed value only insert unchecked_ownership_conversion for terminator uses.

Already, we allow for owned/guaranteed values to be passed to values with
unowned ownership. This just cleans up the SIL a bit.

That being said, we still need unchecked_ownership_conversion for the terminator
case since any owned/guaranteed's value (ignoreing function parameters) will end
before the return meaning that we need some way of escaping it.

Author: gottesmm

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

@@ -608,14 +608,19 @@ SILBasicBlock::iterator OwnershipRAUWUtility::handleUnowned() {
     // value, we need to ensure that the guaranteed value is live at all use
     // points of the unowned value. If so, just replace and continue.
     //
-    // TODO: Implement this.
+    // TODO: Implement this for more interesting cases.
+    if (isa<SILFunctionArgument>(newValue))
+      return replaceAllUsesAndErase(oldValue, newValue, callbacks);
 
     // Otherwise, we need to lifetime extend the borrow over all of the use
-    // points. To do so, we copy the value, borrow it, insert an unchecked
-    // ownership conversion to unowned at oldValue and then RAUW.
+    // points. To do so, we copy the value, borrow it, and insert an unchecked
+    // ownership conversion to unowned at all uses that are terminator uses.
+    //
+    // We need to insert the conversion since if we have a non-argument
+    // guaranteed value since its scope will end before the terminator so we
+    // need to convert the value to unowned early.
     //
-    // We need to insert the conversion to ensure that we do not violate
-    // ownership propagation rules of forwarding insts.
+    // TODO: Do we need a separate array here?
     SmallVector<Operand *, 8> oldValueUses(oldValue->getUses());
     for (auto *use : oldValueUses) {
       if (auto *ti = dyn_cast<TermInst>(use->getUser())) {
@@ -628,14 +633,12 @@ SILBasicBlock::iterator OwnershipRAUWUtility::handleUnowned() {
         }
       }
     }
+
     auto extender = getLifetimeExtender();
     SILValue borrow =
         extender.createPlusZeroBorrow(newValue, oldValue->getUses());
     SILBuilderWithScope builder(oldValue);
-    auto *newInst = builder.createUncheckedOwnershipConversion(
-        oldValue->getLoc(), borrow, OwnershipKind::Unowned);
-    callbacks.createdNewInst(newInst);
-    return replaceAllUsesAndErase(oldValue, newInst, callbacks);
+    return replaceAllUsesAndErase(oldValue, borrow, callbacks);
   }
   case OwnershipKind::Owned: {
     // If we have an unowned value that we want to replace with an owned value,
@@ -646,13 +649,15 @@ SILBasicBlock::iterator OwnershipRAUWUtility::handleUnowned() {
 
     // Otherwise, insert a copy of the owned value and lifetime extend that over
     // all uses of the value and then RAUW.
+    //
+    // NOTE: For terminator uses, we funnel the use through an
+    // unchecked_ownership_conversion to ensure that we can end the lifetime of
+    // our owned/guaranteed value before the terminator.
     SmallVector<Operand *, 8> oldValueUses(oldValue->getUses());
     for (auto *use : oldValueUses) {
       if (auto *ti = dyn_cast<TermInst>(use->getUser())) {
         if (ti->isFunctionExiting()) {
           SILBuilderWithScope builder(ti);
-          // We insert this to ensure that we can extend our owned value's
-          // lifetime to before the function end point.
           auto *newInst = builder.createUncheckedOwnershipConversion(
               ti->getLoc(), use->get(), OwnershipKind::Unowned);
           callbacks.createdNewInst(newInst);
@@ -663,10 +668,7 @@ SILBasicBlock::iterator OwnershipRAUWUtility::handleUnowned() {
     auto extender = getLifetimeExtender();
     SILValue copy = extender.createPlusZeroCopy(newValue, oldValue->getUses());
     SILBuilderWithScope builder(oldValue);
-    auto *newInst = builder.createUncheckedOwnershipConversion(
-        oldValue->getLoc(), copy, OwnershipKind::Unowned);
-    callbacks.createdNewInst(newInst);
-    auto result = replaceAllUsesAndErase(oldValue, newInst, callbacks);
+    auto result = replaceAllUsesAndErase(oldValue, copy, callbacks);
     return result;
   }
   }

test/SILOptimizer/ossa_rauw_tests.sil

@@ -318,6 +318,40 @@ bb0(%0 : @guaranteed $Klass):
   return %3 : $(Klass, Klass)
 }
 
+// CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2a : $@convention(thin) (@guaranteed Builtin.NativeObject) -> (Klass, Klass) {
+// CHECK: bb0(
+// CHECK-NEXT: unchecked_ref_cast
+// CHECK-NEXT: tuple
+// CHECK-NEXT: return
+// CHECK: } // end sil function 'unowned_to_guaranteed_rauw_2a'
+sil [ossa] @unowned_to_guaranteed_rauw_2a : $@convention(thin) (@guaranteed Builtin.NativeObject) -> (Klass, Klass) {
+bb0(%0 : @guaranteed $Builtin.NativeObject):
+  %0a = unchecked_ref_cast %0 : $Builtin.NativeObject to $Klass
+  %1 = unchecked_bitwise_cast %0a : $Klass to $SubKlass
+  %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass
+  %3 = tuple(%2 : $Klass, %2 : $Klass)
+  return %3 : $(Klass, Klass)
+}
+
+// We need the unchecked_ownership_conversion since our base value is
+// guaranteed, not a function argument, and our user is a function exiting
+// terminator.
+//
+// CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2b : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Klass {
+// CHECK: bb0(
+// CHECK-NEXT: unchecked_ref_cast
+// CHECK-NEXT: unchecked_ownership_conversion
+// CHECK-NEXT: return
+// CHECK: } // end sil function 'unowned_to_guaranteed_rauw_2b'
+sil [ossa] @unowned_to_guaranteed_rauw_2b : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Klass {
+bb0(%0 : @guaranteed $Builtin.NativeObject):
+  %0a = unchecked_ref_cast %0 : $Builtin.NativeObject to $Klass
+  %1 = unchecked_bitwise_cast %0a : $Klass to $SubKlass
+  %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass
+  return %2 : $Klass
+}
+
+
 // CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2_loop : $@convention(thin) (@guaranteed Klass) -> @owned FakeOptional<(Klass, Klass)> {
 // CHECK: bb0([[ARG:%.*]] : @guaranteed $Klass):
 // CHECK-NOT: unchecked_bitwise_cast
@@ -376,7 +410,6 @@ bbExitBlock(%result : @owned $FakeOptional<(Klass, Klass)>):
 
 // CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_3 : $@convention(thin) (@guaranteed Klass) -> Klass {
 // CHECK: bb0(
-// CHECK-NEXT: unchecked_ownership_conversion
 // CHECK-NEXT: return
 // CHECK: } // end sil function 'unowned_to_guaranteed_rauw_3'
 sil [ossa] @unowned_to_guaranteed_rauw_3 : $@convention(thin) (@guaranteed Klass) -> Klass {

test/SILOptimizer/sil_combine_ossa.sil

@@ -2024,6 +2024,7 @@ bb0(%0 : @owned $Builtin.NativeObject, %1 : $Builtin.RawPointer):
 // CHECK-NEXT: unchecked_trivial_bit_cast
 // CHECK-NEXT: unchecked_ref_cast
 // CHECK-NOT: unchecked_bitwise_cast
+// CHECK-NOT: unchecked_ownership_conversion
 // CHECK: } // end sil function 'bitwise_combines_owned'
 sil [ossa] @bitwise_combines_owned : $@convention(thin) (@owned Builtin.NativeObject, Builtin.RawPointer) -> @owned (Optional<Builtin.NativeObject>, Builtin.RawPointer, Optional<Builtin.NativeObject>) {
 bb0(%0 : @owned $Builtin.NativeObject, %1 : $Builtin.RawPointer):
@@ -2050,6 +2051,7 @@ bb0(%0 : @owned $Builtin.NativeObject, %1 : $Builtin.RawPointer):
 // CHECK-NEXT: unchecked_trivial_bit_cast
 // CHECK-NEXT: unchecked_ref_cast
 // CHECK-NOT: unchecked_bitwise_cast
+// CHECK-NOT: unchecked_ownership_conversion
 // CHECK: } // end sil function 'bitwise_combines_guaranteed'
 sil [ossa] @bitwise_combines_guaranteed : $@convention(thin) (@guaranteed Builtin.NativeObject, Builtin.RawPointer) -> @owned (Optional<Builtin.NativeObject>, Builtin.RawPointer, Optional<Builtin.NativeObject>) {
 bb0(%0 : @guaranteed $Builtin.NativeObject, %1 : $Builtin.RawPointer):