Merge pull request swiftlang#34756 from gottesmm/pr-506a6eb10f358603d6870a894a758dddd7d4ab42

[ownership] Make SILUndef always have ValueOwnershipKind::None.

Author: gottesmm

include/swift/SIL/SILModule.h

@@ -238,7 +238,7 @@ class SILModule {
   llvm::DenseMap<Identifier, BuiltinInfo> BuiltinIDCache;
 
   /// This is the set of undef values we've created, for uniquing purposes.
-  llvm::DenseMap<std::pair<SILType, unsigned>, SILUndef *> UndefValues;
+  llvm::DenseMap<SILType, SILUndef *> UndefValues;
 
   /// The stage of processing this module is at.
   SILStage Stage;

include/swift/SIL/SILUndef.h

@@ -23,25 +23,23 @@ class SILInstruction;
 class SILModule;
 
 class SILUndef : public ValueBase {
-  ValueOwnershipKind ownershipKind;
-
-  SILUndef(SILType type, ValueOwnershipKind ownershipKind);
+  SILUndef(SILType type);
 
 public:
   void operator=(const SILArgument &) = delete;
   void operator delete(void *, size_t) = delete;
 
-  static SILUndef *get(SILType ty, SILModule &m, ValueOwnershipKind ownershipKind);
+  static SILUndef *get(SILType ty, SILModule &m);
   static SILUndef *get(SILType ty, const SILFunction &f);
 
   template <class OwnerTy>
   static SILUndef *getSentinelValue(SILType type, OwnerTy owner) {
     // Ownership kind isn't used here, the value just needs to have a unique
     // address.
-    return new (*owner) SILUndef(type, OwnershipKind::None);
+    return new (*owner) SILUndef(type);
   }
 
-  ValueOwnershipKind getOwnershipKind() const { return ownershipKind; }
+  ValueOwnershipKind getOwnershipKind() const { return OwnershipKind::None; }
 
   static bool classof(const SILArgument *) = delete;
   static bool classof(const SILInstruction *) = delete;

lib/SIL/IR/SILUndef.cpp

@@ -15,26 +15,16 @@
 
 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;
-}
-
-SILUndef::SILUndef(SILType type, ValueOwnershipKind ownershipKind)
-    : ValueBase(ValueKind::SILUndef, type, IsRepresentative::Yes),
-      ownershipKind(ownershipKind) {}
+SILUndef::SILUndef(SILType type)
+    : ValueBase(ValueKind::SILUndef, type, IsRepresentative::Yes) {}
 
-SILUndef *SILUndef::get(SILType ty, SILModule &m, ValueOwnershipKind ownershipKind) {
-  SILUndef *&entry = m.UndefValues[std::make_pair(ty, unsigned(ownershipKind))];
+SILUndef *SILUndef::get(SILType ty, SILModule &m) {
+  SILUndef *&entry = m.UndefValues[ty];
   if (entry == nullptr)
-    entry = new (m) SILUndef(ty, ownershipKind);
+    entry = new (m) SILUndef(ty);
   return entry;
 }
 
 SILUndef *SILUndef::get(SILType ty, const SILFunction &f) {
-  auto ownershipKind = getOwnershipKindForUndef(ty, f);
-  return SILUndef::get(ty, f.getModule(), ownershipKind);
+  return SILUndef::get(ty, f.getModule());
 }

lib/SIL/IR/ValueOwnership.cpp

@@ -585,9 +585,8 @@ 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.
+  // to short circuit ownership optimizations. Since SILUndef in either case
+  // will be ValueOwnershipKind::None, we will not get any wonky behavior here.
   //
   // We assume that any time we are in SILBuilder and call this without having a
   // value in a block yet, ossa is enabled.

lib/SIL/Parser/ParseSIL.cpp

@@ -499,8 +499,7 @@ bool SILParser::parseVerbatim(StringRef name) {
 SILParser::~SILParser() {
   for (auto &Entry : ForwardRefLocalValues) {
     if (ValueBase *dummyVal = LocalValues[Entry.first()]) {
-      dummyVal->replaceAllUsesWith(
-          SILUndef::get(dummyVal->getType(), SILMod, OwnershipKind::None));
+      dummyVal->replaceAllUsesWith(SILUndef::get(dummyVal->getType(), SILMod));
       SILInstruction::destroy(cast<GlobalAddrInst>(dummyVal));
       SILMod.deallocateInst(cast<GlobalAddrInst>(dummyVal));
     }

lib/Serialization/DeserializeSIL.cpp

@@ -277,9 +277,10 @@ SILValue SILDeserializer::getLocalValue(ValueID Id,
                                         SILType Type) {
   // The first two IDs are special undefined values.
   if (Id == 0)
-    return SILUndef::get(Type, SILMod, OwnershipKind::None);
-  else if (Id == 1)
-    return SILUndef::get(Type, SILMod, OwnershipKind::Owned);
+    return SILUndef::get(Type, SILMod);
+  assert(Id != 1 && "This used to be for SILUndef with OwnershipKind::Owned... "
+                    "but we don't support that anymore. Make sure no one "
+                    "changes that without updating this code if needed");
 
   // Check to see if this is already defined.
   ValueBase *Entry = LocalValues.lookup(Id);

test/SIL/ownership-verifier/undef.sil

@@ -12,6 +12,10 @@ struct MyInt {
   var i: Builtin.Int32
 }
 
+struct MyKlassWrapper {
+  var i: Klass
+}
+
 // Make sure that we handle undef in an appropriate way. Do to special handling
 // in SIL around undef, we test this using the ownership dumper for simplicity.
 
@@ -31,14 +35,22 @@ struct MyInt {
 // CHECK: Visiting: {{.*}}%4 = struct $MyInt (undef : $Builtin.Int32)
 // CHECK-NEXT: Ownership Constraint:
 // CHECK-NEXT: Op #: 0
-// CHECK_NEXT: Constraint: <Constraint Kind:none LifetimeConstraint:NonLifetimeEnding>
+// CHECK-NEXT: Constraint: <Constraint Kind:none LifetimeConstraint:NonLifetimeEnding>
+// CHECK-LABEL: Visiting:   {{%.*}} = struct $MyKlassWrapper (undef : $Klass)
+// CHECK-NEXT: Ownership Constraint:
+// CHECK-NEXT: Op #: 0
+// CHECK-NEXT: Constraint: <Constraint Kind:none LifetimeConstraint:NonLifetimeEnding>
+// CHECK-NEXT: Results Ownership Kinds:
+// CHECK-NEXT: Result:   {{%.*}} = struct $MyKlassWrapper (undef : $Klass)
+// CHECK-NEXT: Kind: none
 sil [ossa] @undef_addresses_have_any_ownership : $@convention(thin) () -> () {
 bb0:
   %0 = mark_uninitialized [var] undef : $*Klass
   %1 = mark_uninitialized [var] undef : $Klass
   destroy_value %1 : $Klass
   %2 = mark_uninitialized [var] undef : $*Builtin.Int32
   %3 = struct $MyInt(undef : $Builtin.Int32)
+  %4 = struct $MyKlassWrapper(undef : $Klass)
   %9999 = tuple()
   return %9999 : $()
 }