Merge pull request #81952 from nate-chandler/rdar152431332

[DestroyAddrHoisting] Skip init_enum_data_addrs.

Author: nate-chandler

include/swift/SIL/MemAccessUtils.h

@@ -1606,6 +1606,7 @@ inline bool isAccessStorageTypeCast(SingleValueInstruction *svi) {
   // Simply pass-thru the incoming address.  But change its type!
   case SILInstructionKind::MoveOnlyWrapperToCopyableAddrInst:
   case SILInstructionKind::CopyableToMoveOnlyWrapperAddrInst:
+  case SILInstructionKind::MoveOnlyWrapperToCopyableBoxInst:
   // Simply pass-thru the incoming address.  But change its type!
   case SILInstructionKind::UncheckedAddrCastInst:
   // Casting to RawPointer does not affect the AccessPath. When converting
@@ -1652,7 +1653,6 @@ inline bool isAccessStorageIdentityCast(SingleValueInstruction *svi) {
   case SILInstructionKind::MarkDependenceInst:
   case SILInstructionKind::CopyValueInst:
   case SILInstructionKind::BeginBorrowInst:
-  case SILInstructionKind::MoveOnlyWrapperToCopyableBoxInst:
     return true;
   }
 }
@@ -1688,6 +1688,22 @@ inline bool isAccessStorageCast(SingleValueInstruction *svi) {
   return isAccessStorageTypeCast(svi) || isAccessStorageIdentityCast(svi);
 }
 
+// Strip access markers and casts that preserve the access storage.
+//
+// Compare to stripAccessAndIdentityCasts.  This function strips cast that
+// change the type.
+inline SILValue stripAccessAndAccessStorageCasts(SILValue v) {
+  if (auto *bai = dyn_cast<BeginAccessInst>(v)) {
+    return stripAccessAndAccessStorageCasts(bai->getOperand());
+  }
+  if (auto *svi = dyn_cast<SingleValueInstruction>(v)) {
+    if (isAccessStorageCast(svi)) {
+      return stripAccessAndAccessStorageCasts(svi->getAllOperands()[0].get());
+    }
+  }
+  return v;
+}
+
 /// Abstract CRTP class for a visiting instructions that are part of the use-def
 /// chain from an accessed address up to the storage base.
 ///

lib/SIL/Utils/MemAccessUtils.cpp

@@ -1402,7 +1402,7 @@ class AccessPathVisitor : public FindAccessVisitorImpl<AccessPathVisitor> {
           AccessPath::Index::forSubObjectProjection(projIdx.Index));
     } else {
       // Ignore everything in getAccessProjectionOperand that is an access
-      // projection with no affect on the access path.
+      // projection with no effect on the access path.
       assert(isa<OpenExistentialAddrInst>(projectedAddr) ||
              isa<InitEnumDataAddrInst>(projectedAddr) ||
              isa<UncheckedTakeEnumDataAddrInst>(projectedAddr)
@@ -1982,7 +1982,7 @@ AccessPathDefUseTraversal::visitSingleValueUser(SingleValueInstruction *svi,
     return IgnoredUse;
 
   // open_existential_addr and unchecked_take_enum_data_addr are classified as
-  // access projections, but they also modify memory. Both see through them and
+  // access projections, but they also modify memory. Both look through them and
   // also report them as uses.
   case SILInstructionKind::OpenExistentialAddrInst:
   case SILInstructionKind::UncheckedTakeEnumDataAddrInst:

lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp

@@ -164,6 +164,20 @@ struct KnownStorageUses : UniqueStorageUseVisitor {
   bool visitStore(Operand *use) override { return recordUser(use->getUser()); }
 
   bool visitDestroy(Operand *use) override {
+    // An init_enum_data_addr is viewed as an "identity projection", rather than
+    // a proper projection like a struct_element_addr.  As a result, its
+    // destroys are passed directly to visitors.  But such addresses aren't
+    // quite equivalent to the original address.  Specifically, a destroy of an
+    // init_enum_data_addr cannot in general be replaced with a destroy of the
+    // whole enum.  The latter destroy is only equivalent to the former if there
+    // has been an `inject_enum_addr`.
+    //
+    // Handle a destroy of such a projection just like we handle the destroy of
+    // a struct_element_addr: by bailing out.
+    if (isa<InitEnumDataAddrInst>(
+            stripAccessAndAccessStorageCasts(use->get()))) {
+      return false;
+    }
     originalDestroys.insert(use->getUser());
     return true;
   }

test/SILOptimizer/hoist_destroy_addr.sil

@@ -1280,3 +1280,39 @@ bb0(%aggregate : $*NCAggregate):
   %retval = tuple ()
   return %retval
 }
+
+// CHECK-LABEL: sil [ossa] @nohoist_ieda : {{.*}} {
+// CHECK:         [[IEDA:%[^,]+]] = init_enum_data_addr
+// CHECK:         tuple
+// CHECK-NEXT:    destroy_addr [[IEDA]]
+// CHECK-LABEL: } // end sil function 'nohoist_ieda'
+sil [ossa] @nohoist_ieda : $@convention(thin) () -> @out TwoCases {
+bb0(%out : $*TwoCases):
+  %ieda = init_enum_data_addr %out, #TwoCases.A!enumelt
+  apply undef(%ieda) : $@convention(thin) () -> @out X
+  %retval = tuple ()
+  destroy_addr %ieda
+  apply undef(%ieda) : $@convention(thin) () -> @out X
+  inject_enum_addr %out, #TwoCases.A!enumelt
+  return %retval
+}
+
+// CHECK-LABEL: sil [ossa] @nohoist_ieda_2 : {{.*}} {
+// CHECK:         [[IEDA:%[^,]+]] = init_enum_data_addr
+// CHECK:         tuple
+// CHECK:         [[CTM:%[^,]+]] = copyable_to_moveonlywrapper_addr [[IEDA]]
+// CHECK:         [[MTC:%[^,]+]] = moveonlywrapper_to_copyable_addr [[CTM]]
+// CHECK-NEXT:    destroy_addr [[MTC]]
+// CHECK-LABEL: } // end sil function 'nohoist_ieda_2'
+sil [ossa] @nohoist_ieda_2 : $@convention(thin) () -> @out TwoCases {
+bb0(%out : $*TwoCases):
+  %ieda = init_enum_data_addr %out, #TwoCases.A!enumelt
+  apply undef(%ieda) : $@convention(thin) () -> @out X
+  %retval = tuple ()
+  %ctm = copyable_to_moveonlywrapper_addr %ieda
+  %mtc = moveonlywrapper_to_copyable_addr %ctm
+  destroy_addr %mtc
+  apply undef(%ieda) : $@convention(thin) () -> @out X
+  inject_enum_addr %out, #TwoCases.A!enumelt
+  return %retval
+}