From 74c4bc9c55f77b3b189fdd1273c415b2150db05b Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Tue, 27 Aug 2024 12:08:02 -0700 Subject: [PATCH] [DAH] Bail on pointer use if ignoring barriers. Unknown uses of raw pointers should not result in bailing out when an address is lexical--the destroy of the address will already not be hoisted over any instructions which may access pointers. If the address is not lexical however (such as any address when lexical lifetimes are disabled), that rationale does not apply, so unknown uses of raw pointers must cause hoisting to bail. rdar://133969821 --- .../Transforms/DestroyAddrHoisting.cpp | 27 ++++++++++++----- test/SILOptimizer/hoist_destroy_addr.sil | 30 +++++++++++++++++-- .../SILOptimizer/rdar133969821.swift | 23 ++++++++++++++ 3 files changed, 69 insertions(+), 11 deletions(-) create mode 100644 validation-test/SILOptimizer/rdar133969821.swift diff --git a/lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp b/lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp index a1009241b0bb0..b892ae506cb97 100644 --- a/lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp +++ b/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 storageUsers; llvm::SmallSetVector originalDestroys; SmallPtrSet 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(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(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; diff --git a/test/SILOptimizer/hoist_destroy_addr.sil b/test/SILOptimizer/hoist_destroy_addr.sil index 71b5e1003488a..a1ae48f9a3e97 100644 --- a/test/SILOptimizer/hoist_destroy_addr.sil +++ b/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 diff --git a/validation-test/SILOptimizer/rdar133969821.swift b/validation-test/SILOptimizer/rdar133969821.swift new file mode 100644 index 0000000000000..02a423ebf34e2 --- /dev/null +++ b/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 { + weak var rawValue: T? = nil +} +typealias Tuple = (instanceIdentifier: C, object: Weak) +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]) { }