[constant-folding] When simplifying destructures, do not handle cases where we have a non-trivial value with .none ownership.

In certain cases, in OSSA, non-trivial values with .none ownership can be merged
into .owned aggregates such that when we extract the value back out from the
aggregate we lost the information that the original value was not .owned. As an
example of this consider the following SIL:

bb0(%0 : @owned Builtin.NativeObject):
  %1 = enum $Optional<Builtin.NativeObject>, #Optional.none!enumelt
  %2 = tuple(%0 : $Builtin.NativeObject, %1 : $Optional<Builtin.NativeObject>)
  (%3, %4) = destructure_tuple %2 : $(Builtin.NativeObject, Optional<Builtin.NativeObject>)

In this case, %4 has .owned ownership, while %1 has .none ownership. This is
because we have lost the refined information that we originally had a .none
value as input to the tuple we are destructuring. Due to this when we RAUW, we
would need to insert a destroy on %4 to make sure that we maintain ossa
invariants. This is safe since the destroy_value will be on a dynamically .none
value, which is a no-op.

That being said, the intention here was actually not to implement this pattern
in the code (as can be seen by not handling @owned destructures). Thus this
commit just makes the constant folding code more conservative to ensure that we
do not try to handle this case.

Author: gottesmm

lib/SILOptimizer/Utils/ConstantFolding.cpp

@@ -1414,9 +1414,22 @@ static bool constantFoldInstruction(Operand *Op, Optional<bool> &ResultsInError,
           [&](Operand &op) -> SILValue {
             SILValue operandValue = op.get();
             auto ownershipKind = operandValue.getOwnershipKind();
-            if (ownershipKind.isCompatibleWith(ValueOwnershipKind::Guaranteed))
-              return operandValue;
-            return SILValue();
+
+            // First check if we are not compatible with guaranteed. This means
+            // we would be Owned or Unowned. If so, return SILValue().
+            if (!ownershipKind.isCompatibleWith(ValueOwnershipKind::Guaranteed))
+              return SILValue();
+
+            // Otherwise check if our operand is non-trivial and None. In cases
+            // like that, the non-trivial type could be replacing an owned value
+            // where we lost that our underlying value is None due to
+            // intermediate aggregate literal operations. In that case, we /do
+            // not/ want to eliminate the destructure.
+            if (ownershipKind == ValueOwnershipKind::None &&
+                !operandValue->getType().isTrivial(*Struct->getFunction()))
+              return SILValue();
+
+            return operandValue;
           });
       return true;
     }
@@ -1435,9 +1448,22 @@ static bool constantFoldInstruction(Operand *Op, Optional<bool> &ResultsInError,
           [&](Operand &op) -> SILValue {
             SILValue operandValue = op.get();
             auto ownershipKind = operandValue.getOwnershipKind();
-            if (ownershipKind.isCompatibleWith(ValueOwnershipKind::Guaranteed))
-              return operandValue;
-            return SILValue();
+
+            // First check if we are not compatible with guaranteed. This means
+            // we would be Owned or Unowned. If so, return SILValue().
+            if (!ownershipKind.isCompatibleWith(ValueOwnershipKind::Guaranteed))
+              return SILValue();
+
+            // Otherwise check if our operand is non-trivial and None. In cases
+            // like that, the non-trivial type could be replacing an owned value
+            // where we lost that our underlying value is None due to
+            // intermediate aggregate literal operations. In that case, we /do
+            // not/ want to eliminate the destructure.
+            if (ownershipKind == ValueOwnershipKind::None &&
+                !operandValue->getType().isTrivial(*Tuple->getFunction()))
+              return SILValue();
+
+            return operandValue;
           });
       return true;
     }

test/SILOptimizer/constant_propagation_ownership.sil

@@ -29,6 +29,11 @@ class Klass {}
 sil @klass_allocator : $@convention(method) (@thick Klass.Type) -> @owned Klass
 sil @generic_user : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
 
+struct NativeObjectOptNativeObjectPair {
+  var lhs: Builtin.NativeObject
+  var rhs: Optional<Builtin.NativeObject>
+}
+
 ///////////
 // Tests //
 ///////////
@@ -1181,3 +1186,39 @@ bb3:
   %9999 = tuple()
   return %9999 : $()
 }
+
+// CHECK-LABEL: sil [ossa] @do_not_RAUW_owned_destructure_with_nontrivial_none_value_tuple : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+// CHECK: ({{%.*}}, [[DESTRUCTURE_RESULT_2:%.*]]) = destructure_tuple
+// CHECK: store [[DESTRUCTURE_RESULT_2]] to [init]
+// CHECK: } // end sil function 'do_not_RAUW_owned_destructure_with_nontrivial_none_value_tuple'
+sil [ossa] @do_not_RAUW_owned_destructure_with_nontrivial_none_value_tuple : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+bb0(%arg : @owned $Builtin.NativeObject):
+  %0 = alloc_stack $Optional<Builtin.NativeObject>
+  %1 = enum $Optional<Builtin.NativeObject>, #Optional.none!enumelt
+  %2 = tuple(%arg : $Builtin.NativeObject, %1 : $Optional<Builtin.NativeObject>)
+  (%3, %4) = destructure_tuple %2 : $(Builtin.NativeObject, Optional<Builtin.NativeObject>)
+  store %4 to [init] %0 : $*Optional<Builtin.NativeObject>
+  destroy_addr %0 : $*Optional<Builtin.NativeObject>
+  dealloc_stack %0 : $*Optional<Builtin.NativeObject>
+  destroy_value %3 : $Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// CHECK-LABEL: sil [ossa] @do_not_RAUW_owned_destructure_with_nontrivial_none_value_struct : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+// CHECK: ({{%.*}}, [[DESTRUCTURE_RESULT_2:%.*]]) = destructure_struct
+// CHECK: store [[DESTRUCTURE_RESULT_2]] to [init]
+// CHECK: } // end sil function 'do_not_RAUW_owned_destructure_with_nontrivial_none_value_struct'
+sil [ossa] @do_not_RAUW_owned_destructure_with_nontrivial_none_value_struct : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+bb0(%arg : @owned $Builtin.NativeObject):
+  %0 = alloc_stack $Optional<Builtin.NativeObject>
+  %1 = enum $Optional<Builtin.NativeObject>, #Optional.none!enumelt
+  %2 = struct $NativeObjectOptNativeObjectPair (%arg : $Builtin.NativeObject, %1 : $Optional<Builtin.NativeObject>)
+  (%3, %4) = destructure_struct %2 : $NativeObjectOptNativeObjectPair
+  store %4 to [init] %0 : $*Optional<Builtin.NativeObject>
+  destroy_addr %0 : $*Optional<Builtin.NativeObject>
+  dealloc_stack %0 : $*Optional<Builtin.NativeObject>
+  destroy_value %3 : $Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}