Skip to content

Commit c56a313

Browse files
Merge pull request #76114 from nate-chandler/rdar133969821_2
[DAH] Bail on pointer use if ignoring barriers.
2 parents 6c1b3cc + 74c4bc9 commit c56a313

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)