Merge pull request swiftlang#34693 from gottesmm/pr-1fdd1272490dcfe27f62410aee521669e0059430

[ownership] Try harder to make sure we do not propagate ownership info when ownership is disabled.

Author: gottesmm

lib/SIL/IR/OperandOwnership.cpp

@@ -913,6 +913,19 @@ OwnershipConstraintClassifier::visitBuiltinInst(BuiltinInst *bi) {
 Optional<OwnershipConstraint> Operand::getOwnershipConstraint() const {
   if (isTypeDependent())
     return None;
+
+  // If we do not have ownership enabled, just return any. This ensures that we
+  // do not have any consuming uses and everything from an ownership perspective
+  // is just a liveness use short-circuiting many of the optimizations.
+  //
+  // We do not ever call this function when an instruction isn't in a block.
+  assert(getUser()->getParent() &&
+         "Can not lookup ownership constraint unless inserted into block");
+  if (auto *block = getUser()->getParent())
+    if (auto *func = block->getParent())
+      if (!func->hasOwnership())
+        return {{OwnershipKind::Any, UseLifetimeConstraint::NonLifetimeEnding}};
+
   OwnershipConstraintClassifier classifier(getUser()->getModule(), *this);
   return classifier.visit(const_cast<SILInstruction *>(getUser()));
 }

lib/SIL/IR/SILUndef.cpp

@@ -16,6 +16,8 @@
 using namespace swift;
 
 static ValueOwnershipKind getOwnershipKindForUndef(SILType type, const SILFunction &f) {
+  if (!f.hasOwnership())
+    return OwnershipKind::None;
   if (type.isAddress() || type.isTrivial(f))
     return OwnershipKind::None;
   return OwnershipKind::Owned;

lib/SIL/IR/ValueOwnership.cpp

@@ -583,6 +583,19 @@ ValueOwnershipKindClassifier::visitBuiltinInst(BuiltinInst *BI) {
 //===----------------------------------------------------------------------===//
 
 ValueOwnershipKind SILValue::getOwnershipKind() const {
+  // If we do not have an undef, we should always be able to get to our function
+  // here. If we do not have ownership enabled, just return none for everything
+  // to short circuit ownership optimizations. If we have an undef we may still
+  // get some results that are slightly wonky but hopefully when we lower
+  // ownership we remove that.
+  //
+  // We assume that any time we are in SILBuilder and call this without having a
+  // value in a block yet, ossa is enabled.
+  if (auto *block = Value->getParentBlock())
+    if (auto *f = block->getParent())
+      if (!f->hasOwnership())
+        return OwnershipKind::None;
+
   ValueOwnershipKindClassifier Classifier;
   auto result = Classifier.visit(const_cast<ValueBase *>(Value));
   assert(result && "Returned ownership kind invalid on values");

lib/SIL/Verifier/SILVerifier.cpp

@@ -1022,8 +1022,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     assert(F && "Expected value base with parent function");
     // If we do not have qualified ownership, then do not verify value base
     // ownership.
-    if (!F->hasOwnership())
+    if (!F->hasOwnership()) {
+      require(SILValue(V).getOwnershipKind() == OwnershipKind::None,
+              "Once ownership is gone, all values should have none ownership");
       return;
+    }
     SILValue(V).verifyOwnership(&DEBlocks);
   }
 
@@ -1100,14 +1103,36 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
               "instruction's operand's owner isn't the instruction");
       require(isInValueUses(&operand), "operand value isn't used by operand");
 
-      if (I->isTypeDependentOperand(operand)) {
+      if (operand.isTypeDependent()) {
         require(isa<SILInstruction>(I),
                "opened archetype operand should refer to a SILInstruction");
       }
 
       // Make sure that if operand is generic that its primary archetypes match
       // the function context.
       checkLegalType(I->getFunction(), operand.get(), I);
+
+      // If we are not in OSSA, our operand constraint should be invalid for a
+      // type dependent operand (that is Optional::None) and if we have a non
+      // type dependent operand then we should have a constraint of
+      // OwnershipKind::Any, UseLifetimeConstraint::NonLifetimeEnding.
+      if (!I->getFunction()->hasOwnership()) {
+        if (operand.isTypeDependent()) {
+          require(
+              !operand.getOwnershipConstraint(),
+              "Non Optional::None constraint for a type dependent operand?!");
+        } else {
+          auto constraint = operand.getOwnershipConstraint();
+          require(constraint.hasValue(),
+                  "All non-type dependent operands must have a "
+                  "non-Optional::None constraint?!");
+          require(constraint->getPreferredKind() == OwnershipKind::Any &&
+                      constraint->getLifetimeConstraint() ==
+                          UseLifetimeConstraint::NonLifetimeEnding,
+                  "In non-ossa all non-type dependent operands must have a "
+                  "constraint of Any, NonLifetimeEnding");
+        }
+      }
     }
 
     // TODO: There should be a use of an opened archetype inside the instruction for

test/AutoDiff/SILOptimizer/differentiation_function_canonicalization.sil

@@ -45,9 +45,7 @@ bb0(%0 : $@differentiable @callee_guaranteed (Float) -> Float):
 // CHECK: bb0([[ARG:%.*]] : $@differentiable @callee_guaranteed (Float) -> Float):
 // CHECK:   [[ORIG_FN:%.*]] = differentiable_function_extract [original] [[ARG]]
 // CHECK:   [[JVP_FN:%.*]] = differentiable_function_extract [jvp] [[ARG]]
-// CHECK:   strong_retain [[JVP_FN]]
 // CHECK:   [[VJP_FN:%.*]] = differentiable_function_extract [vjp] [[ARG]]
-// CHECK:   strong_retain [[VJP_FN]]
 // CHECK:   differentiable_function [parameters 0] [results 0] [[ORIG_FN]] : {{.*}} with_derivative {[[JVP_FN]] : {{.*}}, [[VJP_FN]] : {{.*}}}
 // CHECK: }
 
@@ -134,8 +132,6 @@ bb0(%0 : $Class):
 // CHECK-LABEL: sil @test_class_method_partial_apply
 // CHECK: bb0([[ARG:%.*]] : $Class):
 // CHECK:  [[ORIG_FN:%.*]] = class_method [[ARG]] : $Class, #Class.method
-// CHECK:  strong_retain [[ARG]]
-// CHECK:  strong_retain [[ARG]]
 // CHECK:  [[ORIG_FN_PARTIALLY_APPLIED:%.*]] = partial_apply [callee_guaranteed] [[ORIG_FN]]([[ARG]])
 // CHECK:  [[JVP_FN:%.*]] = class_method [[ARG]] : $Class, #Class.method!jvp.SU
 // CHECK:  [[JVP_FN_PARTIALLY_APPLIED:%.*]] = partial_apply [callee_guaranteed] [[JVP_FN]]([[ARG]])

test/SIL/Parser/basic.sil

@@ -1591,11 +1591,12 @@ bb0(%0 : $Builtin.NativeObject):
   return undef : $()
 }
 
-// CHECK-LABEL: sil @test_unchecked_ownership_conversion : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
+// CHECK-LABEL: sil [ossa] @test_unchecked_ownership_conversion : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
 // CHECK: unchecked_ownership_conversion {{%.*}} : $Builtin.NativeObject, @guaranteed to @owned
-sil @test_unchecked_ownership_conversion : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
-bb0(%0 : $Builtin.NativeObject):
+sil [ossa] @test_unchecked_ownership_conversion : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
+bb0(%0 : @guaranteed $Builtin.NativeObject):
   %1 = unchecked_ownership_conversion %0 : $Builtin.NativeObject, @guaranteed to @owned
+  end_lifetime %1 : $Builtin.NativeObject
   return undef : $()
 }