Skip to content

Commit d61e386

Browse files
drexindrexin
committed
Add tests and incorporate feedback
1 parent 014fd2b commit d61e386

File tree

3 files changed

+119
-5
lines changed

3 files changed

+119
-5
lines changed

‎lib/SILGen/SILGenDestructor.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -222,22 +222,22 @@ void SILGenFunction::destroyClassMember(SILLocation cleanupLoc,
222222

223223
llvm::SmallSetVector<VarDecl *, 4> findRecursiveLinks(DeclContext *DC,
224224
ClassDecl *cd) {
225-
auto SelfTy = DC->mapTypeIntoContext(cd->getDeclaredType());
225+
auto SelfTy = DC->mapTypeIntoContext(cd->getDeclaredInterfaceType());
226226

227227
// Collect all stored properties that would form a recursive structure,
228228
// so we can remove the recursion and prevent the call stack from
229229
// overflowing.
230230
llvm::SmallSetVector<VarDecl *, 4> recursiveLinks;
231231
for (VarDecl *vd : cd->getStoredProperties()) {
232-
auto Ty = vd->getType()->getOptionalObjectType();
232+
auto Ty = vd->getInterfaceType()->getOptionalObjectType();
233233
if (Ty && Ty->getCanonicalType() == SelfTy->getCanonicalType()) {
234234
recursiveLinks.insert(vd);
235235
}
236236
}
237237

238238
// NOTE: Right now we only optimize linear recursion, so if there is more than
239239
// one link, clear out the set and don't perform any recursion optimization.
240-
if (recursiveLinks.size() < 1) {
240+
if (recursiveLinks.size() > 1) {
241241
recursiveLinks.clear();
242242
}
243243

@@ -247,7 +247,7 @@ llvm::SmallSetVector<VarDecl *, 4> findRecursiveLinks(DeclContext *DC,
247247
void SILGenFunction::emitRecursiveChainDestruction(
248248
ManagedValue selfValue, ClassDecl *cd,
249249
SmallSetVector<VarDecl *, 4> recursiveLinks, CleanupLocation cleanupLoc) {
250-
auto SelfTy = F.mapTypeIntoContext(cd->getDeclaredType());
250+
auto SelfTy = F.mapTypeIntoContext(cd->getDeclaredInterfaceType());
251251

252252
assert(recursiveLinks.size() <= 1 && "Only linear recursion supported.");
253253

@@ -262,7 +262,7 @@ void SILGenFunction::emitRecursiveChainDestruction(
262262

263263
// var iter = self.link
264264
// self.link = nil
265-
auto Ty = getTypeLowering(vd->getType()).getLoweredType();
265+
auto Ty = getTypeLowering(vd->getInterfaceType()).getLoweredType();
266266
auto optionalNone = B.createOptionalNone(cleanupLoc, Ty);
267267
SILValue varAddr = B.createRefElementAddr(cleanupLoc, selfValue.getValue(),
268268
vd, Ty.getAddressType());

‎test/SILGen/deinit_recursive.swift

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// RUN: %target-swift-emit-silgen %s | %FileCheck %s
2+
3+
class Node {
4+
var elem: Int64
5+
var next: Node?
6+
}
7+
8+
// CHECK: sil hidden [ossa] @$s16deinit_recursive4NodeCfd : $@convention(method) (@guaranteed Node) -> @owned Builtin.NativeObject {
9+
// CHECK: [[SELF:%.*]] "self"
10+
// CHECK: bb0([[SELF]] : @guaranteed $Node):
11+
// CHECK: [[ELEM:%.*]] = ref_element_addr [[SELF]] : $Node, #Node.elem
12+
// CHECK: [[ELEM_ACCESS:%.*]] = begin_access [deinit] [static] [[ELEM]] : $*Int64
13+
// CHECK: destroy_addr [[ELEM_ACCESS]] : $*Int64
14+
// CHECK: end_access [[ELEM_ACCESS]] : $*Int64
15+
// CHECK: [[NIL:%.*]] = enum $Optional<Node>, #Optional.none!enumelt
16+
// CHECK: [[SELF_NEXT:%.*]] = ref_element_addr [[SELF]] : $Node, #Node.next
17+
// CHECK: [[ITER:%.*]] = alloc_stack $Optional<Node>
18+
// CHECK: [[SELF_NEXT_ACCESS:%.*]] = begin_access [modify] [static] [[SELF_NEXT]] : $*Optional<Node>
19+
// CHECK: [[SELF_NEXT_COPY:%.*]] = load [copy] [[SELF_NEXT_ACCESS]] : $*Optional<Node>
20+
// CHECK: store [[NIL]] to [assign] [[SELF_NEXT_ACCESS]] : $*Optional<Node>
21+
// CHECK: end_access [[SELF_NEXT_ACCESS]] : $*Optional<Node>
22+
// CHECK: store [[SELF_NEXT_COPY]] to [init] [[ITER]] : $*Optional<Node>
23+
// CHECK: br [[LOOPBB:bb.*]] //
24+
25+
// CHECK: [[LOOPBB]]:
26+
// CHECK: [[ITER_ADDR:%.*]] = load [copy] [[ITER]] : $*Optional<Node>
27+
// CHECK: [[ITER_COPY:%.*]] = copy_value [[ITER_ADDR]] : $Optional<Node>
28+
// CHECK: [[ITER_COPY_ADDR:%.*]] = alloc_stack $Optional<Node>
29+
// CHECK: store [[ITER_COPY]] to [init] [[ITER_COPY_ADDR]] : $*Optional<Node>
30+
// CHECK: destroy_value [[ITER_ADDR]] : $Optional<Node>
31+
// CHECK: switch_enum_addr [[ITER_COPY_ADDR]] : $*Optional<Node>, case #Optional.some!enumelt: [[IS_SOME_BB:bb.*]], case #Optional.none!enumelt: [[IS_NONE_BB:bb[0-9]+]]
32+
33+
// CHECK: [[IS_SOME_BB]]:
34+
// CHECK: destroy_addr [[ITER_COPY_ADDR]] : $*Optional<Node>
35+
// CHECK: dealloc_stack [[ITER_COPY_ADDR]] : $*Optional<Node>
36+
// CHECK: [[IS_UNIQUE:%.*]] = is_unique [[ITER]] : $*Optional<Node>
37+
// CHECK: cond_br [[IS_UNIQUE]], [[IS_UNIQUE_BB:bb.*]], [[NOT_UNIQUE_BB:bb[0-9]*]]
38+
39+
// CHECK: [[IS_UNIQUE_BB]]:
40+
// CHECK: [[ITER_ADDR:%.*]] = load [copy] [[ITER]] : $*Optional<Node>
41+
// CHECK: [[ITER_BORROW:%.*]] = begin_borrow [[ITER_ADDR]] : $Optional<Node>
42+
// CHECK: [[ITER_UNWRAPPED:%.*]] = unchecked_enum_data [[ITER_BORROW]] : $Optional<Node>, #Optional.some!enumelt
43+
// CHECK: [[NEXT_ADDR:%.*]] = ref_element_addr [[ITER_UNWRAPPED]] : $Node, #Node.next
44+
// CHECK: [[NEXT_ADDR_ACCESS:%.*]] = begin_access [read] [static] [[NEXT_ADDR]] : $*Optional<Node>
45+
// CHECK: [[NEXT_COPY:%.*]] = load [copy] [[NEXT_ADDR_ACCESS]] : $*Optional<Node>
46+
// CHECK: end_access [[NEXT_ADDR_ACCESS]] : $*Optional<Node>
47+
// CHECK: store [[NEXT_COPY]] to [assign] [[ITER]] : $*Optional<Node>
48+
// CHECK: end_borrow [[ITER_BORROW]] : $Optional<Node>
49+
// CHECK: destroy_value [[ITER_ADDR]] : $Optional<Node>
50+
// CHECK: br [[LOOPBB]]
51+
52+
// CHECK: [[NOT_UNIQUE_BB]]:
53+
// CHECK: br bb6
54+
55+
// CHECK: [[IS_NONE_BB]]:
56+
// CHECK: dealloc_stack [[ITER_COPY_ADDR]] : $*Optional<Node>
57+
// CHECK: br [[CLEAN_BB:bb[0-9]+]]
58+
59+
// CHECK: [[CLEAN_BB]]:
60+
// CHECK: destroy_addr [[ITER]] : $*Optional<Node>
61+
// CHECK: dealloc_stack [[ITER]] : $*Optional<Node>
62+
// CHECK: [[SELF_NATIVE:%.*]] = unchecked_ref_cast [[SELF]] : $Node to $Builtin.NativeObject
63+
// CHECK: [[SELF_NATIVE_OWNED:%.*]] = unchecked_ownership_conversion [[SELF_NATIVE]] : $Builtin.NativeObject, @guaranteed to @owned
64+
// CHECK: return [[SELF_NATIVE_OWNED]] : $Builtin.NativeObject
65+
// CHECK: } // end sil function '$s16deinit_recursive4NodeCfd'
66+
67+
68+
69+
70+
71+
// Non-linearly recursive structures should not get optimized
72+
class Tree {
73+
var left: Tree?
74+
var right: Tree?
75+
}
76+
77+
// CHECK: sil hidden [ossa] @$s16deinit_recursive4TreeCfd : $@convention(method) (@guaranteed Tree) -> @owned Builtin.NativeObject {
78+
// CHECK: // [[SELF:%.*]] "self"
79+
// CHECK: bb0([[SELF]] : @guaranteed $Tree):
80+
// CHECK: [[LEFT:%.*]] = ref_element_addr [[SELF]] : $Tree, #Tree.left
81+
// CHECK: [[LEFT_ACCESS:%.*]] = begin_access [deinit] [static] [[LEFT]] : $*Optional<Tree>
82+
// CHECK: destroy_addr [[LEFT_ACCESS]] : $*Optional<Tree>
83+
// CHECK: end_access [[LEFT_ACCESS]] : $*Optional<Tree>
84+
// CHECK: [[RIGHT:%.*]] = ref_element_addr [[SELF]] : $Tree, #Tree.right
85+
// CHECK: [[RIGHT_ACCESS:%.*]] = begin_access [deinit] [static] [[RIGHT]] : $*Optional<Tree>
86+
// CHECK: destroy_addr [[RIGHT_ACCESS]] : $*Optional<Tree>
87+
// CHECK: end_access [[RIGHT_ACCESS]] : $*Optional<Tree> // id: %9
88+
// CHECK: [[SELF_NATIVE:%.*]] = unchecked_ref_cast [[SELF]] : $Tree to $Builtin.NativeObject
89+
// CHECK: [[SELF_NATIVE_OWNED:%.*]] = unchecked_ownership_conversion [[SELF_NATIVE]] : $Builtin.NativeObject, @guaranteed to @owned
90+
// CHECK: return [[SELF_NATIVE_OWNED]] : $Builtin.NativeObject
91+
// CHECK: } // end sil function '$s16deinit_recursive4TreeCfd'

‎test/SILGen/deinit_recursive_no_overflow.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift %s -o %t/a.out
3+
// RUN: %target-codesign %t/a.out
4+
// RUN: %target-run %t/a.out
5+
6+
// REQUIRES: executable_test
7+
8+
class Node {
9+
var next: Node?
10+
}
11+
12+
var first: Node? = nil
13+
for _ in 1...3_000_000 {
14+
let next = Node()
15+
next.next = first
16+
first = next
17+
}
18+
19+
@inline(never)
20+
func deallocList() {
21+
first = nil
22+
}
23+
deallocList()

0 commit comments

Comments
 (0)