[DAH] Bail on pointer use if ignoring barriers.

lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp

@@ -112,14 +112,17 @@ namespace {
 /// Step #1: Find all known uses of the unique storage object.
 struct KnownStorageUses : UniqueStorageUseVisitor {
   bool preserveDebugInfo;
+  bool ignoreDeinitBarriers;
 
   SmallPtrSet<SILInstruction *, 16> storageUsers;
   llvm::SmallSetVector<SILInstruction *, 4> originalDestroys;
   SmallPtrSet<SILInstruction *, 4> debugInsts;
 
-  KnownStorageUses(AccessStorage storage, SILFunction *function)
+  KnownStorageUses(AccessStorage storage, SILFunction *function,
+                   bool ignoreDeinitBarriers)
       : UniqueStorageUseVisitor(storage, function),
-        preserveDebugInfo(function->preserveDebugInfo()) {}
+        preserveDebugInfo(function->preserveDebugInfo()),
+        ignoreDeinitBarriers(ignoreDeinitBarriers) {}
 
   bool empty() const {
     return storageUsers.empty() && originalDestroys.empty() &&
@@ -178,11 +181,19 @@ struct KnownStorageUses : UniqueStorageUseVisitor {
 
   bool visitUnknownUse(Operand *use) override {
     auto *user = use->getUser();
-    if (isa<BuiltinRawPointerType>(use->get()->getType().getASTType())) {
-      // Destroy hoisting considers address_to_pointer to be a leaf use because
-      // any potential pointer access is already considered to be a
-      // deinitialization barrier.  Consequently, any instruction that uses a
-      // value produced by address_to_pointer isn't regarded as a storage use.
+    if (isa<BuiltinRawPointerType>(use->get()->getType().getASTType()) &&
+        !ignoreDeinitBarriers) {
+      // When respecting deinit barriers, destroy hoisting considers
+      // address_to_pointer to be a leaf use because any potential pointer
+      // access is already considered to be a barrier to hoisting (because as a
+      // pointer access it's a deinitialization barrier). Consequently, any
+      // instruction that uses a value produced by address_to_pointer isn't
+      // regarded as a storage use.
+      //
+      // On the other hand, when _not_ respecting deinit barriers, potential
+      // pointer accesses are _not_ already considered to be barriers to
+      // hoisting (deinit barriers being ignored); so uses of the pointer must
+      // obstruct all hoisting.
       return true;
     }
     LLVM_DEBUG(llvm::dbgs() << "Unknown user " << *user);
@@ -473,7 +484,7 @@ bool HoistDestroys::perform() {
       storage.getKind() != AccessStorage::Kind::Nested)
     return false;
 
-  KnownStorageUses knownUses(storage, getFunction());
+  KnownStorageUses knownUses(storage, getFunction(), ignoreDeinitBarriers);
   if (!knownUses.findUses())
     return false;
 

test/SILOptimizer/hoist_destroy_addr.sil

@@ -630,13 +630,37 @@ entry(%instance : $*X):
   return %value : $X
 }
 
-// Hoist even if the pointerified address gets used.
+// If lexical, hoist even if the pointerified address gets used.
 //
 // CHECK-LABEL: sil [ossa] @hoist_despite_use_of_pointer : {{.*}} {
-// CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] :
+// CHECK:         [[INSTANCE:%[^,]+]] = alloc_stack [lexical]
+// CHECK-NEXT:    copy_addr {{.*}} to [init] [[INSTANCE]]
 // CHECK-NEXT:    destroy_addr [[INSTANCE]]
 // CHECK-LABEL: } // end sil function 'hoist_despite_use_of_pointer'
-sil [ossa] @hoist_despite_use_of_pointer : $@convention(thin) (@inout X) -> () {
+sil [ossa] @hoist_despite_use_of_pointer : $@convention(thin) (@in_guaranteed X) -> () {
+entry(%original : $*X):
+   %instance = alloc_stack [lexical] $X
+   copy_addr %original to [init] %instance : $*X
+   %addr_for_pointer = alloc_stack $Builtin.RawPointer
+   %pointer = address_to_pointer %instance : $*X to $Builtin.RawPointer
+   store %pointer to [trivial] %addr_for_pointer : $*Builtin.RawPointer
+   %retval = tuple ()
+   dealloc_stack %addr_for_pointer : $*Builtin.RawPointer
+   destroy_addr %instance : $*X
+   dealloc_stack %instance : $*X
+   return %retval : $()
+}
+
+// If non-lexical, _don't_ hoist if the pointerified address gets used--deinit
+// barriers are ignored.
+//
+// CHECK-LABEL: sil [ossa] @no_hoist_nonlexical_because_of_use_of_pointer : {{.*}} {
+// CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] :
+// CHECK:         destroy_addr [[INSTANCE]]
+// CHECK-NEXT:    dealloc_stack
+// CHECK-NEXT:    return
+// CHECK-LABEL: } // end sil function 'no_hoist_nonlexical_because_of_use_of_pointer'
+sil [ossa] @no_hoist_nonlexical_because_of_use_of_pointer : $@convention(thin) (@inout X) -> () {
 entry(%instance : $*X):
    %addr_for_pointer = alloc_stack $Builtin.RawPointer
    %pointer = address_to_pointer %instance : $*X to $Builtin.RawPointer

validation-test/SILOptimizer/rdar133969821.swift

@@ -0,0 +1,23 @@
+// RUN: %target-run-simple-swift(-Xfrontend -disable-availability-checking) | %FileCheck %s
+
+// REQUIRES: executable_test
+// REQUIRES: concurrency
+// REQUIRES: concurrency_runtime
+// UNSUPPORTED: back_deployment_runtime
+
+class C {
+  init() {}
+}
+struct Weak<T: AnyObject> {
+  weak var rawValue: T? = nil
+}
+typealias Tuple = (instanceIdentifier: C, object: Weak<C>)
+let object = C()
+let tuple: Tuple = (instanceIdentifier: .init(), object: .init(rawValue: object))
+let closureResult = [tuple].map { $0.object.rawValue }.first
+let keypathResult = [tuple].map(\.object.rawValue).first
+print("Closure result: \(String(describing: closureResult))")
+// CHECK: Closure result: Optional(Optional(main.C))
+print("Keypath result: \(String(describing: keypathResult))")
+// CHECK: Keypath result: Optional(Optional(main.C))
+withExtendedLifetime([object]) { }