Skip to content

Commit 74c4bc9

Browse files
committed
[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
1 parent ee17ef1 commit 74c4bc9

File tree

3 files changed

+69
-11
lines changed

3 files changed

+69
-11
lines changed

lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,17 @@ namespace {
112112
/// Step #1: Find all known uses of the unique storage object.
113113
struct KnownStorageUses : UniqueStorageUseVisitor {
114114
bool preserveDebugInfo;
115+
bool ignoreDeinitBarriers;
115116

116117
SmallPtrSet<SILInstruction *, 16> storageUsers;
117118
llvm::SmallSetVector<SILInstruction *, 4> originalDestroys;
118119
SmallPtrSet<SILInstruction *, 4> debugInsts;
119120

120-
KnownStorageUses(AccessStorage storage, SILFunction *function)
121+
KnownStorageUses(AccessStorage storage, SILFunction *function,
122+
bool ignoreDeinitBarriers)
121123
: UniqueStorageUseVisitor(storage, function),
122-
preserveDebugInfo(function->preserveDebugInfo()) {}
124+
preserveDebugInfo(function->preserveDebugInfo()),
125+
ignoreDeinitBarriers(ignoreDeinitBarriers) {}
123126

124127
bool empty() const {
125128
return storageUsers.empty() && originalDestroys.empty() &&
@@ -178,11 +181,19 @@ struct KnownStorageUses : UniqueStorageUseVisitor {
178181

179182
bool visitUnknownUse(Operand *use) override {
180183
auto *user = use->getUser();
181-
if (isa<BuiltinRawPointerType>(use->get()->getType().getASTType())) {
182-
// Destroy hoisting considers address_to_pointer to be a leaf use because
183-
// any potential pointer access is already considered to be a
184-
// deinitialization barrier. Consequently, any instruction that uses a
185-
// value produced by address_to_pointer isn't regarded as a storage use.
184+
if (isa<BuiltinRawPointerType>(use->get()->getType().getASTType()) &&
185+
!ignoreDeinitBarriers) {
186+
// When respecting deinit barriers, destroy hoisting considers
187+
// address_to_pointer to be a leaf use because any potential pointer
188+
// access is already considered to be a barrier to hoisting (because as a
189+
// pointer access it's a deinitialization barrier). Consequently, any
190+
// instruction that uses a value produced by address_to_pointer isn't
191+
// regarded as a storage use.
192+
//
193+
// On the other hand, when _not_ respecting deinit barriers, potential
194+
// pointer accesses are _not_ already considered to be barriers to
195+
// hoisting (deinit barriers being ignored); so uses of the pointer must
196+
// obstruct all hoisting.
186197
return true;
187198
}
188199
LLVM_DEBUG(llvm::dbgs() << "Unknown user " << *user);
@@ -473,7 +484,7 @@ bool HoistDestroys::perform() {
473484
storage.getKind() != AccessStorage::Kind::Nested)
474485
return false;
475486

476-
KnownStorageUses knownUses(storage, getFunction());
487+
KnownStorageUses knownUses(storage, getFunction(), ignoreDeinitBarriers);
477488
if (!knownUses.findUses())
478489
return false;
479490

test/SILOptimizer/hoist_destroy_addr.sil

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -630,13 +630,37 @@ entry(%instance : $*X):
630630
return %value : $X
631631
}
632632

633-
// Hoist even if the pointerified address gets used.
633+
// If lexical, hoist even if the pointerified address gets used.
634634
//
635635
// CHECK-LABEL: sil [ossa] @hoist_despite_use_of_pointer : {{.*}} {
636-
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] :
636+
// CHECK: [[INSTANCE:%[^,]+]] = alloc_stack [lexical]
637+
// CHECK-NEXT: copy_addr {{.*}} to [init] [[INSTANCE]]
637638
// CHECK-NEXT: destroy_addr [[INSTANCE]]
638639
// CHECK-LABEL: } // end sil function 'hoist_despite_use_of_pointer'
639-
sil [ossa] @hoist_despite_use_of_pointer : $@convention(thin) (@inout X) -> () {
640+
sil [ossa] @hoist_despite_use_of_pointer : $@convention(thin) (@in_guaranteed X) -> () {
641+
entry(%original : $*X):
642+
%instance = alloc_stack [lexical] $X
643+
copy_addr %original to [init] %instance : $*X
644+
%addr_for_pointer = alloc_stack $Builtin.RawPointer
645+
%pointer = address_to_pointer %instance : $*X to $Builtin.RawPointer
646+
store %pointer to [trivial] %addr_for_pointer : $*Builtin.RawPointer
647+
%retval = tuple ()
648+
dealloc_stack %addr_for_pointer : $*Builtin.RawPointer
649+
destroy_addr %instance : $*X
650+
dealloc_stack %instance : $*X
651+
return %retval : $()
652+
}
653+
654+
// If non-lexical, _don't_ hoist if the pointerified address gets used--deinit
655+
// barriers are ignored.
656+
//
657+
// CHECK-LABEL: sil [ossa] @no_hoist_nonlexical_because_of_use_of_pointer : {{.*}} {
658+
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] :
659+
// CHECK: destroy_addr [[INSTANCE]]
660+
// CHECK-NEXT: dealloc_stack
661+
// CHECK-NEXT: return
662+
// CHECK-LABEL: } // end sil function 'no_hoist_nonlexical_because_of_use_of_pointer'
663+
sil [ossa] @no_hoist_nonlexical_because_of_use_of_pointer : $@convention(thin) (@inout X) -> () {
640664
entry(%instance : $*X):
641665
%addr_for_pointer = alloc_stack $Builtin.RawPointer
642666
%pointer = address_to_pointer %instance : $*X to $Builtin.RawPointer
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// RUN: %target-run-simple-swift(-Xfrontend -disable-availability-checking) | %FileCheck %s
2+
3+
// REQUIRES: executable_test
4+
// REQUIRES: concurrency
5+
// REQUIRES: concurrency_runtime
6+
// UNSUPPORTED: back_deployment_runtime
7+
8+
class C {
9+
init() {}
10+
}
11+
struct Weak<T: AnyObject> {
12+
weak var rawValue: T? = nil
13+
}
14+
typealias Tuple = (instanceIdentifier: C, object: Weak<C>)
15+
let object = C()
16+
let tuple: Tuple = (instanceIdentifier: .init(), object: .init(rawValue: object))
17+
let closureResult = [tuple].map { $0.object.rawValue }.first
18+
let keypathResult = [tuple].map(\.object.rawValue).first
19+
print("Closure result: \(String(describing: closureResult))")
20+
// CHECK: Closure result: Optional(Optional(main.C))
21+
print("Keypath result: \(String(describing: keypathResult))")
22+
// CHECK: Keypath result: Optional(Optional(main.C))
23+
withExtendedLifetime([object]) { }

0 commit comments

Comments
 (0)