MemoryLifetime: support init_existential_addr and open_existential_addr

eeckstein · eeckstein · commit 45f08bd7c42b · 2021-03-07T09:10:22.000+01:00
Also, relax the check for enums a bit. Instead of only accepting single-payload enums, just require that the payload type is the same for an enum location.
diff --git a/include/swift/SIL/MemoryLifetime.h b/include/swift/SIL/MemoryLifetime.h
@@ -189,18 +189,16 @@ class MemoryLocations {
   /// small. They can be handled separately with handleSingleBlockLocations().
   llvm::SmallVector<SingleValueInstruction *, 16> singleBlockLocations;
 
-  /// A Cache for single-payload enums.
-  llvm::DenseMap<SILType, EnumElementDecl *> singlePayloadEnums;
-
   /// The bit-set of locations for which numNonTrivialFieldsNotCovered is > 0.
   Bits nonTrivialLocations;
 
-  /// If true, support init_enum_data_addr and unchecked_take_enum_data_addr
-  bool handleEnumDataProjections;
+  /// If true, support init_enum_data_addr, unchecked_take_enum_data_addr,
+  /// init_existential_addr and open_existential_addr.
+  bool handleNonTrivialProjections;
 
 public:
-  MemoryLocations(bool handleEnumDataProjections) :
-    handleEnumDataProjections(handleEnumDataProjections) {}
+  MemoryLocations(bool handleNonTrivialProjections) :
+    handleNonTrivialProjections(handleNonTrivialProjections) {}
 
   MemoryLocations(const MemoryLocations &) = delete;
   MemoryLocations &operator=(const MemoryLocations &) = delete;
@@ -297,15 +295,6 @@ class MemoryLocations {
   // (locationIdx, fieldNr) -> subLocationIdx
   using SubLocationMap = llvm::DenseMap<std::pair<unsigned, unsigned>, unsigned>;
 
-  /// Returns the payload case of a single-payload enum.
-  ///
-  /// Returns null if \p enumTy is not a single-payload enum.
-  /// We are currently only handling enum data projections for single-payload
-  /// enums, because it's much simpler to represent them with Locations. We
-  /// could also support multi-payload enums, but that gets complicated. Most
-  /// importantly, we can handle Swift.Optional.
-  EnumElementDecl *getSinglePayloadEnumCase(SILType enumTy);
-
   /// Helper function called by analyzeLocation to check all uses of the
   /// location recursively.
   ///
diff --git a/lib/SIL/Verifier/MemoryLifetime.cpp b/lib/SIL/Verifier/MemoryLifetime.cpp
@@ -100,7 +100,8 @@ MemoryLocations::Location::Location(SILValue val, unsigned index, int parentIdx)
       parentIdx(parentIdx) {
   assert(((parentIdx >= 0) ==
     (isa<StructElementAddrInst>(val) || isa<TupleElementAddrInst>(val) ||
-     isa<InitEnumDataAddrInst>(val) || isa<UncheckedTakeEnumDataAddrInst>(val)))
+     isa<InitEnumDataAddrInst>(val) || isa<UncheckedTakeEnumDataAddrInst>(val) ||
+     isa<InitExistentialAddrInst>(val) || isa<OpenExistentialAddrInst>(val)))
     && "sub-locations can only be introduced with struct/tuple/enum projections");
   setBitAndResize(subLocations, index);
   setBitAndResize(selfAndParents, index);
@@ -280,35 +281,17 @@ void MemoryLocations::clear() {
   nonTrivialLocations.clear();
 }
 
-static EnumElementDecl *computeSinglePayloadEnumCase(SILType enumTy) {
-  EnumDecl *enumDecl = enumTy.getEnumOrBoundGenericEnum();
-  if (!enumDecl)
-    return nullptr;
-  EnumElementDecl *payloadCase = nullptr;
-  for (EnumElementDecl *elem : enumDecl->getAllElements()) {
-    if (elem->hasAssociatedValues()) {
-      if (payloadCase)
-        return nullptr;
-      payloadCase = elem;
-    }
-  }
-  return payloadCase;
-}
-
-EnumElementDecl *MemoryLocations::getSinglePayloadEnumCase(SILType enumTy) {
-  auto iter = singlePayloadEnums.find(enumTy);
-  if (iter != singlePayloadEnums.end())
-    return iter->second;
-  
-  EnumElementDecl *&payloadCase = singlePayloadEnums[enumTy];
-  payloadCase = computeSinglePayloadEnumCase(enumTy);
-  return payloadCase;
-}
-
 bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx,
                                     SmallVectorImpl<SILValue> &collectedVals,
                                     SubLocationMap &subLocationMap) {
   for (Operand *use : V->getUses()) {
+    // We can safely ignore type dependent operands, because the lifetime of a
+    // type is decoupled from the lifetime of its value. For example, even if
+    // the result of an open_existential_addr is destroyed its type is still
+    // valid.
+    if (use->isTypeDependent())
+      continue;
+  
     SILInstruction *user = use->getUser();
 
     // We only handle addr-instructions which are planned to be used with
@@ -335,21 +318,18 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx
                                             collectedVals, subLocationMap))
           return false;
         break;
+      case SILInstructionKind::InitExistentialAddrInst:
+      case SILInstructionKind::OpenExistentialAddrInst:
       case SILInstructionKind::InitEnumDataAddrInst:
       case SILInstructionKind::UncheckedTakeEnumDataAddrInst:
-        if (!handleEnumDataProjections)
-          return false;
-        if (!getSinglePayloadEnumCase(user->getOperand(0)->getType()))
+        if (!handleNonTrivialProjections)
           return false;
         // The payload is represented as a single sub-location of the enum.
         if (!analyzeAddrProjection(cast<SingleValueInstruction>(user), locIdx,
                                   /*fieldNr*/ 0, collectedVals, subLocationMap))
           return false;
         break;
       case SILInstructionKind::InjectEnumAddrInst:
-        if (!getSinglePayloadEnumCase(user->getOperand(0)->getType()))
-          return false;
-        break;
       case SILInstructionKind::LoadInst:
       case SILInstructionKind::StoreInst:
       case SILInstructionKind::StoreBorrowInst:
@@ -368,6 +348,7 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx
       case SILInstructionKind::YieldInst:
       case SILInstructionKind::DeallocStackInst:
       case SILInstructionKind::SwitchEnumAddrInst:
+      case SILInstructionKind::WitnessMethodInst:
         break;
       default:
         return false;
@@ -411,6 +392,24 @@ bool MemoryLocations::analyzeAddrProjection(
         idx = loc.parentIdx;
       } while (idx >= 0);
     }
+  } else if (!isa<OpenExistentialAddrInst>(projection)) {
+    Location *loc = &locations[subLocIdx];
+    if (loc->representativeValue->getType() != projection->getType()) {
+      assert(isa<InitEnumDataAddrInst>(projection) ||
+             isa<UncheckedTakeEnumDataAddrInst>(projection) ||
+             isa<InitExistentialAddrInst>(projection));
+             
+      // We can only handle a single enum payload type for a location or or a
+      // single concrete existential type. Mismatching types can have a differnt
+      // number of (non-trivial) sub-locations and we cannot handle this.
+      // But we ignore opened existential types, because those cannot have
+      // sub-locations (there cannot be an address projection on an
+      // open_existential_addr).
+      if (!isa<OpenExistentialAddrInst>(loc->representativeValue))
+        return false;
+      assert(loc->representativeValue->getType().isOpenedExistential());
+      loc->representativeValue = projection;
+    }
   }
 
   if (!analyzeLocationUsesRecursively(projection, subLocIdx, collectedVals,
@@ -451,11 +450,6 @@ void MemoryLocations::initFieldsCounter(Location &loc) {
     }
     return;
   }
-  if (EnumElementDecl *elem = getSinglePayloadEnumCase(ty)) {
-    // The payload is represented as a single sub-location of the enum.
-    loc.updateFieldCounters(ty.getEnumElementType(elem, function), +1);
-    return;
-  }
   loc.updateFieldCounters(ty, +1);
 }
 
@@ -695,7 +689,7 @@ class MemoryLifetimeVerifier {
 
 public:
   MemoryLifetimeVerifier(SILFunction *function) :
-    function(function), locations(/*handleEnumDataProjections*/ true) {}
+    function(function), locations(/*handleNonTrivialProjections*/ true) {}
 
   /// The main entry point to verify the lifetime of all memory locations in
   /// the function.
@@ -1152,12 +1146,16 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) {
         requireNoStoreBorrowLocation(IEAI->getOperand(), &I);
         break;
       }
+      case SILInstructionKind::InitExistentialAddrInst:
       case SILInstructionKind::InitEnumDataAddrInst: {
-        SILValue enumAddr = cast<InitEnumDataAddrInst>(&I)->getOperand();
-        requireBitsClear(bits, enumAddr, &I);
-        requireNoStoreBorrowLocation(enumAddr, &I);
+        SILValue addr = I.getOperand(0);
+        requireBitsClear(bits, addr, &I);
+        requireNoStoreBorrowLocation(addr, &I);
         break;
       }
+      case SILInstructionKind::OpenExistentialAddrInst:
+        requireBitsSet(bits, cast<OpenExistentialAddrInst>(&I)->getOperand(), &I);
+        break;
       case SILInstructionKind::UncheckedTakeEnumDataAddrInst: {
         // Note that despite the name, unchecked_take_enum_data_addr does _not_
         // "take" the payload of the Swift.Optional enum. This is a terrible
diff --git a/lib/SILOptimizer/Transforms/DestroyHoisting.cpp b/lib/SILOptimizer/Transforms/DestroyHoisting.cpp
@@ -124,9 +124,9 @@ class DestroyHoisting {
 public:
   DestroyHoisting(SILFunction *function, DominanceAnalysis *DA) :
     function(function),
-    // We currently don't handle enum data projections, because they cannot
-    // re-created easily. We could support this in future.
-    locations(/*handleEnumDataProjections*/ false), DA(DA) {}
+    // We currently don't handle enum and existential projections, because they
+    // cannot be re-created easily. We could support this in future.
+    locations(/*handleNonTrivialProjections*/ false), DA(DA) {}
 
   bool hoistDestroys();
 };
diff --git a/test/SIL/memory_lifetime.sil b/test/SIL/memory_lifetime.sil
@@ -560,3 +560,35 @@ bb0(%0 : $*U):
   %5 = tuple ()
   return %5 : $()
 }
+
+protocol P {}
+
+sil [ossa] @test_existentials : $@convention(thin) <U: P> (@in_guaranteed U) -> () {
+bb0(%0 : $*U):
+  %s = alloc_stack $P
+  %i = init_existential_addr %s : $*P, $U
+  copy_addr %0 to [initialization] %i : $*U
+  %o = open_existential_addr immutable_access %s : $*P to $*@opened("5B62392E-7C62-11EB-BF90-D0817AD9985D") P
+  %s2 = alloc_stack $@opened("5B62392E-7C62-11EB-BF90-D0817AD9985D") P
+  copy_addr %o to [initialization] %s2 : $*@opened("5B62392E-7C62-11EB-BF90-D0817AD9985D") P
+  destroy_addr %s2 : $*@opened("5B62392E-7C62-11EB-BF90-D0817AD9985D") P
+  dealloc_stack %s2 : $*@opened("5B62392E-7C62-11EB-BF90-D0817AD9985D") P
+  destroy_addr %s : $*P
+  dealloc_stack %s : $*P
+  %5 = tuple ()
+  return %5 : $()
+}
+
+sil [ossa] @test_init_two_existentials : $@convention(thin) <U: P, V: P> (@in_guaranteed U, @in_guaranteed V) -> () {
+bb0(%0 : $*U, %1 : $*V):
+  %s = alloc_stack $P
+  %t = init_existential_addr %s : $*P, $U
+  copy_addr %0 to [initialization] %t : $*U
+  destroy_addr %s : $*P
+  %v = init_existential_addr %s : $*P, $V
+  copy_addr %1 to [initialization] %v : $*V
+  destroy_addr %s : $*P
+  dealloc_stack %s : $*P
+  %5 = tuple ()
+  return %5 : $()
+}
diff --git a/test/SIL/memory_lifetime_failures.sil b/test/SIL/memory_lifetime_failures.sil
@@ -445,3 +445,32 @@ bb0(%0 : $*U):
   %5 = tuple ()
   return %5 : $()
 }
+
+protocol P {}
+
+// CHECK: SIL memory lifetime failure in @test_init_existential: memory is not initialized, but should
+sil [ossa] @test_init_existential : $@convention(thin) <U: P> (@in_guaranteed U) -> () {
+bb0(%0 : $*U):
+  %s = alloc_stack $P
+  %i = init_existential_addr %s : $*P, $U
+  destroy_addr %s : $*P
+  dealloc_stack %s : $*P
+  %5 = tuple ()
+  return %5 : $()
+}
+
+// CHECK: SIL memory lifetime failure in @test_open_existential: memory is not initialized, but should
+sil [ossa] @test_open_existential : $@convention(thin) <U: P> (@in_guaranteed U) -> () {
+bb0(%0 : $*U):
+  %s = alloc_stack $P
+  %o = open_existential_addr immutable_access %s : $*P to $*@opened("5B62392E-7C62-11EB-BF90-D0817AD9985D") P
+  %s2 = alloc_stack $@opened("5B62392E-7C62-11EB-BF90-D0817AD9985D") P
+  copy_addr %o to [initialization] %s2 : $*@opened("5B62392E-7C62-11EB-BF90-D0817AD9985D") P
+  destroy_addr %s2 : $*@opened("5B62392E-7C62-11EB-BF90-D0817AD9985D") P
+  dealloc_stack %s2 : $*@opened("5B62392E-7C62-11EB-BF90-D0817AD9985D") P
+  destroy_addr %s : $*P
+  dealloc_stack %s : $*P
+  %5 = tuple ()
+  return %5 : $()
+}
+
diff --git a/test/SILOptimizer/accessed_storage_analysis_ossa.sil b/test/SILOptimizer/accessed_storage_analysis_ossa.sil
@@ -451,7 +451,7 @@ enum TreeB<T> {
 sil [ossa] @readIndirectEnum : $@convention(thin) <T> (@in TreeB<T>) -> (@out TreeB<T>) {
 bb0(%0 : $*TreeB<T>, %1 : $*TreeB<T>):
   %enumAddr = unchecked_take_enum_data_addr %1 : $*TreeB<T>, #TreeB.Branch!enumelt
-  %box = load [copy] %enumAddr : $*<τ_0_0> { var (left: TreeB<τ_0_0>, right: TreeB<τ_0_0>) } <T>
+  %box = load [take] %enumAddr : $*<τ_0_0> { var (left: TreeB<τ_0_0>, right: TreeB<τ_0_0>) } <T>
   %boxAddr = project_box %box : $<τ_0_0> { var (left: TreeB<τ_0_0>, right: TreeB<τ_0_0>) } <T>, 0
   %boxAccess = begin_access [read] [dynamic] %boxAddr : $*(left: TreeB<T>, right: TreeB<T>)
   %leftAddr = tuple_element_addr %boxAccess : $*(left: TreeB<T>, right: TreeB<T>), 0
diff --git a/test/SILOptimizer/existential_transform_extras_ossa.sil b/test/SILOptimizer/existential_transform_extras_ossa.sil
@@ -113,6 +113,7 @@ bb0(%0 : $*P):
   %6 = open_existential_addr immutable_access %4 : $*P to $*@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P
   %7 = witness_method $@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P, #P.foo : <Self where Self : P> (Self) -> () -> Int32, %6 : $*@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
   %8 = apply %7<@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P>(%6) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
+  destroy_addr %4 : $*P
   dealloc_stack %4 : $*P
   return %8 : $Int32
 }
@@ -191,7 +192,9 @@ bb0(%0 : $*P, %1 : $*P):
   %19 = tuple_extract %17 : $(Builtin.Int32, Builtin.Int1), 1
   cond_fail %19 : $Builtin.Int1
   %21 = struct $Int32 (%18 : $Builtin.Int32)
+  destroy_addr %9 : $*P
   dealloc_stack %9 : $*P
+  destroy_addr %4 : $*P
   dealloc_stack %4 : $*P
   return %21 : $Int32
 }
diff --git a/test/SILOptimizer/looprotate_ossa.sil b/test/SILOptimizer/looprotate_ossa.sil
@@ -204,7 +204,7 @@ bb3:
 // CHECK-LABEL: sil [ossa] @looprotate_with_opened_archetype :
 // CHECK: return
 // CHECK-LABEL: } // end sil function 'looprotate_with_opened_archetype'
-sil [ossa] @looprotate_with_opened_archetype : $@convention(thin) (Int32, @in P) -> Int32 {
+sil [ossa] @looprotate_with_opened_archetype : $@convention(thin) (Int32, @in_guaranteed P) -> Int32 {
 bb0(%0 : $Int32, %25: $*P):
   %1 = struct_extract %0 : $Int32, #Int32._value
   %2 = integer_literal $Builtin.Int32, 0
diff --git a/test/SILOptimizer/sil_combine_apply_ossa.sil b/test/SILOptimizer/sil_combine_apply_ossa.sil
@@ -403,20 +403,17 @@ bb0(%0 : $*MutatingProto, %1 : $MStruct):
 }
 
 // CHECK-LABEL: sil [ossa] @dont_replace_copied_self_in_mutating_method_call2
-sil [ossa] @dont_replace_copied_self_in_mutating_method_call2 : $@convention(thin) (@thick MutatingProto.Type) -> (@out MutatingProto) {
-bb0(%0 : $*MutatingProto, %1 : $@thick MutatingProto.Type):
-  %alloc1 = alloc_stack $MutatingProto, let, name "p"
+sil [ossa] @dont_replace_copied_self_in_mutating_method_call2 : $@convention(thin) (@thick MutatingProto.Type, @in_guaranteed MutatingProto) -> (@out MutatingProto) {
+bb0(%0 : $*MutatingProto, %1 : $@thick MutatingProto.Type, %2 : $*MutatingProto):
   %openType = open_existential_metatype %1 : $@thick MutatingProto.Type to $@thick (@opened("66A6DAFC-AF78-11E7-8F3B-28CFE9213F4F") MutatingProto).Type
-  %initType =  init_existential_addr %alloc1 : $*MutatingProto, $@opened("66A6DAFC-AF78-11E7-8F3B-28CFE9213F4F") MutatingProto
   %alloc2 = alloc_stack $MutatingProto
-  copy_addr %alloc1 to [initialization] %alloc2 : $*MutatingProto
+  copy_addr %2 to [initialization] %alloc2 : $*MutatingProto
   %oeaddr = open_existential_addr mutable_access %alloc2 : $*MutatingProto to $*@opened("6E02DCF6-AF78-11E7-8F3B-28CFE9213F4F") MutatingProto
   %witmethod = witness_method $@opened("66A6DAFC-AF78-11E7-8F3B-28CFE9213F4F") MutatingProto, #MutatingProto.mutatingMethod : <Self where Self : MutatingProto> (inout Self) -> () -> (), %openType : $@thick (@opened("66A6DAFC-AF78-11E7-8F3B-28CFE9213F4F") MutatingProto).Type : $@convention(witness_method: MutatingProto) <τ_0_0 where τ_0_0 : MutatingProto> (@inout τ_0_0) -> ()
   // CHECK: apply {{%.*}}<@opened("6E02DCF6-AF78-11E7-8F3B-28CFE9213F4F") MutatingProto>({{%.*}}) : $@convention(witness_method: MutatingProto) <τ_0_0 where τ_0_0 : MutatingProto> (@inout τ_0_0) -> () // type-defs
   %apply = apply %witmethod<@opened("6E02DCF6-AF78-11E7-8F3B-28CFE9213F4F") MutatingProto>(%oeaddr) : $@convention(witness_method: MutatingProto) <τ_0_0 where τ_0_0 : MutatingProto> (@inout τ_0_0) -> ()
   copy_addr [take] %alloc2 to [initialization] %0 : $*MutatingProto
   dealloc_stack %alloc2 : $*MutatingProto
-  dealloc_stack %alloc1 : $*MutatingProto
   %27 = tuple ()
   return %27 : $()
 }
@@ -1108,7 +1105,7 @@ bb2:
 // CHECK-NEXT: apply [[FN]]<@opened("5DD6F3D0-808A-11E6-93A0-34363BD08DA0") FakeProtocol>([[OPEN]])
 // CHECK-NEXT: tuple
 // CHECK-NEXT: return
-sil [ossa] @test_existential_partial_apply_apply : $@convention(thin) (@in FakeProtocol) -> () {
+sil [ossa] @test_existential_partial_apply_apply : $@convention(thin) (@in_guaranteed FakeProtocol) -> () {
 bb0(%0: $*FakeProtocol):
   %o = open_existential_addr immutable_access %0 : $*FakeProtocol to $*@opened("5DD6F3D0-808A-11E6-93A0-34363BD08DA0") FakeProtocol
   %f1 = witness_method $@opened("5DD6F3D0-808A-11E6-93A0-34363BD08DA0") FakeProtocol, #FakeProtocol.requirement, %o : $*@opened("5DD6F3D0-808A-11E6-93A0-34363BD08DA0") FakeProtocol : $@convention(witness_method: FakeProtocol) <T where T : FakeProtocol> (@in_guaranteed T) -> ()
