Skip to content

Commit 12164c9

Browse files
gottesmmgottesmm
authored
Merge pull request #35974 from gottesmm/interior_pointer_improvements
[ownership] Small code improvements around interior pointers
2 parents c193757 + 479b5b9 commit 12164c9

File tree

4 files changed

+113
-20
lines changed

4 files changed

+113
-20
lines changed

‎lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -648,16 +648,15 @@ bool BorrowedValue::visitInteriorPointerOperandHelper(
648648
if (isa<DebugValueInst>(user) || isa<SuperMethodInst>(user) ||
649649
isa<ClassMethodInst>(user) || isa<CopyValueInst>(user) ||
650650
isa<EndBorrowInst>(user) || isa<ApplyInst>(user) ||
651-
isa<StoreBorrowInst>(user) || isa<StoreInst>(user) ||
652-
isa<PartialApplyInst>(user) || isa<UnmanagedRetainValueInst>(user) ||
651+
isa<StoreInst>(user) || isa<PartialApplyInst>(user) ||
652+
isa<UnmanagedRetainValueInst>(user) ||
653653
isa<UnmanagedReleaseValueInst>(user) ||
654654
isa<UnmanagedAutoreleaseValueInst>(user)) {
655655
continue;
656656
}
657657

658658
// These are interior pointers that have not had support yet added for them.
659-
if (isa<OpenExistentialBoxInst>(user) ||
660-
isa<ProjectExistentialBoxInst>(user)) {
659+
if (isa<ProjectExistentialBoxInst>(user)) {
661660
continue;
662661
}
663662

@@ -708,13 +707,12 @@ bool InteriorPointerOperand::findTransitiveUsesForAddress(
708707
// and do not need to check transitive uses of.
709708
if (isa<LoadInst>(user) || isa<CopyAddrInst>(user) ||
710709
isIncidentalUse(user) || isa<StoreInst>(user) ||
711-
isa<StoreBorrowInst>(user) || isa<PartialApplyInst>(user) ||
712-
isa<DestroyAddrInst>(user) || isa<AssignInst>(user) ||
713-
isa<AddressToPointerInst>(user) || isa<YieldInst>(user) ||
714-
isa<LoadUnownedInst>(user) || isa<StoreUnownedInst>(user) ||
715-
isa<EndApplyInst>(user) || isa<LoadWeakInst>(user) ||
716-
isa<StoreWeakInst>(user) || isa<AssignByWrapperInst>(user) ||
717-
isa<BeginUnpairedAccessInst>(user) ||
710+
isa<PartialApplyInst>(user) || isa<DestroyAddrInst>(user) ||
711+
isa<AssignInst>(user) || isa<AddressToPointerInst>(user) ||
712+
isa<YieldInst>(user) || isa<LoadUnownedInst>(user) ||
713+
isa<StoreUnownedInst>(user) || isa<EndApplyInst>(user) ||
714+
isa<LoadWeakInst>(user) || isa<StoreWeakInst>(user) ||
715+
isa<AssignByWrapperInst>(user) || isa<BeginUnpairedAccessInst>(user) ||
718716
isa<EndUnpairedAccessInst>(user) || isa<WitnessMethodInst>(user) ||
719717
isa<SwitchEnumAddrInst>(user) || isa<CheckedCastAddrBranchInst>(user) ||
720718
isa<SelectEnumAddrInst>(user) || isa<InjectEnumAddrInst>(user)) {
@@ -727,7 +725,7 @@ bool InteriorPointerOperand::findTransitiveUsesForAddress(
727725
isa<OpenExistentialAddrInst>(user) ||
728726
isa<InitExistentialAddrInst>(user) || isa<InitEnumDataAddrInst>(user) ||
729727
isa<BeginAccessInst>(user) || isa<TailAddrInst>(user) ||
730-
isa<IndexAddrInst>(user) ||
728+
isa<IndexAddrInst>(user) || isa<StoreBorrowInst>(user) ||
731729
isa<UnconditionalCheckedCastAddrInst>(user) ||
732730
isa<UncheckedAddrCastInst>(user)) {
733731
for (SILValue r : user->getResults()) {

‎lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,10 @@ bool OwnershipModelEliminatorVisitor::visitStoreBorrowInst(
255255
StoreOwnershipQualifier::Unqualified);
256256
});
257257

258-
// Then remove the qualified store.
258+
// Then remove the qualified store after RAUWing si with its dest. This
259+
// ensures that any uses of the interior pointer result of the store_borrow
260+
// are rewritten to be on the dest point.
261+
si->replaceAllUsesWith(si->getDest());
259262
eraseInstruction(si);
260263
return true;
261264
}

‎test/SIL/ownership-verifier/interior_pointer.sil

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
1-
// RUN: %target-sil-opt -sil-ownership-verifier-enable-testing -ownership-verifier-textual-error-dumper -enable-sil-verify-all=0 %s -o /dev/null 2>&1 | %FileCheck %s
1+
// RUN: %target-sil-opt -module-name Swift -sil-ownership-verifier-enable-testing -ownership-verifier-textual-error-dumper -enable-sil-verify-all=0 %s -o /dev/null 2>&1 | %FileCheck %s
22
// REQUIRES: asserts
33

4-
import Swift
5-
64
sil_stage canonical
75

6+
import Builtin
7+
88
class Klass {}
99

1010
class KlassUser {
1111
var field: Klass
1212
}
1313

14+
protocol Error {
15+
var _code: Int { get }
16+
}
17+
18+
sil @use_builtinnativeobject_inguaranteed : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> ()
19+
1420
// CHECK-LABEL: Function: 'simple_error_ref_element_addr'
1521
// CHECK-NEXT: Found outside of lifetime use?!
1622
// CHECK-NEXT: Value: %1 = begin_borrow %0 : $KlassUser // users: %3, %2
@@ -43,16 +49,21 @@ bb0(%0 : @owned $KlassUser):
4349
return %3 : $Klass
4450
}
4551

52+
enum FakeOptional<T> {
53+
case none
54+
case some(T)
55+
}
56+
4657
class OptionalBox<T> {
47-
var t: T?
58+
var t: FakeOptional<T>
4859
}
4960

5061
// CHECK-NOT: Function: 'inject_enum_addr_test'
5162
sil [ossa] @inject_enum_addr_test : $@convention(thin) <T> (@owned OptionalBox<T>) -> () {
5263
bb0(%0 : @owned $OptionalBox<T>):
5364
%1 = begin_borrow %0 : $OptionalBox<T>
5465
%2 = ref_element_addr %1 : $OptionalBox<T>, #OptionalBox.t
55-
inject_enum_addr %2 : $*Optional<T>, #Optional.none!enumelt
66+
inject_enum_addr %2 : $*FakeOptional<T>, #FakeOptional.none!enumelt
5667
end_borrow %1 : $OptionalBox<T>
5768
destroy_value %0 : $OptionalBox<T>
5869
%3 = tuple ()
@@ -64,7 +75,7 @@ sil [ossa] @init_enum_data_addr_test : $@convention(thin) <T> (@owned OptionalBo
6475
bb0(%0 : @owned $OptionalBox<T>, %1 : $*T):
6576
%2 = begin_borrow %0 : $OptionalBox<T>
6677
%3 = ref_element_addr %2 : $OptionalBox<T>, #OptionalBox.t
67-
%4 = init_enum_data_addr %3 : $*Optional<T>, #Optional.some!enumelt
78+
%4 = init_enum_data_addr %3 : $*FakeOptional<T>, #FakeOptional.some!enumelt
6879
copy_addr %1 to [initialization] %4 : $*T
6980
end_borrow %2 : $OptionalBox<T>
7081
destroy_value %0 : $OptionalBox<T>
@@ -76,6 +87,10 @@ class Box<T> {
7687
var t: T
7788
}
7889

90+
struct Int {
91+
var _value: Builtin.Int64
92+
}
93+
7994
// CHECK-NOT: Function: 'unconditional_cast_test'
8095
sil [ossa] @unconditional_cast_test : $@convention(thin) <T> (@owned Box<T>, @in Int) -> () {
8196
bb0(%0 : @owned $Box<T>, %1 : $*Int):
@@ -86,4 +101,60 @@ bb0(%0 : @owned $Box<T>, %1 : $*Int):
86101
destroy_value %0 : $Box<T>
87102
%4 = tuple ()
88103
return %4 : $()
89-
}
104+
}
105+
106+
// CHECK-LABEL: Error#: 0. Begin Error in Function: 'store_borrow_result_used_outside_of_borrow_lifetime'
107+
// CHECK-NEXT: Found outside of lifetime use?!
108+
// CHECK-NEXT: Value: %1 = begin_borrow %0 : $Builtin.NativeObject // users: %4, %3
109+
// CHECK-NEXT: Consuming User: end_borrow %1 : $Builtin.NativeObject // id: %4
110+
// CHECK-NEXT: Non Consuming User: %7 = apply %6(%3) : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> ()
111+
// CHECK-NEXT: Block: bb0
112+
sil [ossa] @store_borrow_result_used_outside_of_borrow_lifetime : $@convention(thin) (@owned Builtin.NativeObject) -> () {
113+
bb0(%0 : @owned $Builtin.NativeObject):
114+
%0a = begin_borrow %0 : $Builtin.NativeObject
115+
%1 = alloc_stack $Builtin.NativeObject
116+
%result = store_borrow %0a to %1 : $*Builtin.NativeObject
117+
end_borrow %0a : $Builtin.NativeObject
118+
destroy_value %0 : $Builtin.NativeObject
119+
%func = function_ref @use_builtinnativeobject_inguaranteed : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> ()
120+
apply %func(%result) : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> ()
121+
dealloc_stack %1 : $*Builtin.NativeObject
122+
%9999 = tuple()
123+
return %9999 : $()
124+
}
125+
126+
// CHECK-LABEL: Error#: 0. Begin Error in Function: 'recursive_interior_pointer_error'
127+
// CHECK-NEXT: Found outside of lifetime use?!
128+
// CHECK-NEXT: Value: %2 = begin_borrow %0 : $KlassUser // users: %5, %3
129+
// CHECK-NEXT: Consuming User: end_borrow %2 : $KlassUser // id: %5
130+
// CHECK-NEXT: Non Consuming User: %7 = load [copy] %4 : $*Klass // user: %8
131+
// CHECK-NEXT: Block: bb0
132+
// CHECK: Error#: 0. End Error in Function: 'recursive_interior_pointer_error'
133+
sil [ossa] @recursive_interior_pointer_error : $@convention(thin) (@owned KlassUser, @guaranteed Klass) -> @owned Klass {
134+
bb0(%0 : @owned $KlassUser, %0a : @guaranteed $Klass):
135+
%1 = begin_borrow %0 : $KlassUser
136+
%2 = ref_tail_addr %1 : $KlassUser, $Klass
137+
%result = store_borrow %0a to %2 : $*Klass
138+
end_borrow %1 : $KlassUser
139+
destroy_value %0 : $KlassUser
140+
%3 = load [copy] %result : $*Klass
141+
return %3 : $Klass
142+
}
143+
144+
// CHECK-LABEL: Error#: 0. Begin Error in Function: 'open_existential_box_interior_pointer_error'
145+
// CHECK-NEXT: Found outside of lifetime use?!
146+
// CHECK-NEXT: Value: %1 = begin_borrow %0 : $Error // users: %3, %2
147+
// CHECK-NEXT: Consuming User: end_borrow %1 : $Error // id: %3
148+
// CHECK-NEXT: Non Consuming User: %6 = apply %5<@opened("01234567-89AB-CDEF-0123-000000000000") Error>(%2) : $@convention(witness_method: Error) <τ_0_0 where τ_0_0 : Error> (@in_guaranteed τ_0_0) -> Int // type-defs: %2; user: %7
149+
// CHECK-NEXT: Block: bb0
150+
// CHECK: Error#: 0. End Error in Function: 'open_existential_box_interior_pointer_error'
151+
sil [ossa] @open_existential_box_interior_pointer_error : $@convention(thin) (@owned Error) -> Int {
152+
bb0(%0 : @owned $Error):
153+
%1 = begin_borrow %0 : $Error
154+
%2 = open_existential_box %1 : $Error to $*@opened("01234567-89AB-CDEF-0123-000000000000") Error
155+
end_borrow %1 : $Error
156+
destroy_value %0 : $Error
157+
%m = witness_method $@opened("01234567-89AB-CDEF-0123-000000000000") Error, #Error._code!getter, %2 : $*@opened("01234567-89AB-CDEF-0123-000000000000") Error : $@convention(witness_method: Error) <Self: Error> (@in_guaranteed Self) -> Int
158+
%result = apply %m<@opened("01234567-89AB-CDEF-0123-000000000000") Error>(%2) : $@convention(witness_method: Error) <Self: Error> (@in_guaranteed Self) -> Int
159+
return %result : $Int
160+
}

‎test/SILOptimizer/ownership_model_eliminator.sil

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ sil_stage raw
55
import Builtin
66

77
sil @use_native_object : $@convention(thin) (@owned Builtin.NativeObject) -> ()
8+
sil @use_native_object_inguaranteed : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> ()
89
sil @use_int32 : $@convention(thin) (Builtin.Int32) -> ()
910

1011
enum Either<T, R> {
@@ -358,3 +359,23 @@ bb0(%0a : $PairOfInt):
358359
%0b = unchecked_value_cast %0a : $PairOfInt to $Builtin.Int64
359360
return %0b : $Builtin.Int64
360361
}
362+
363+
// Make sure we RAUW the store_borrow's result with its dest while lowering.
364+
//
365+
// CHECK-LABEL: sil @lower_store_borrow_result_correctly : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
366+
// CHECK: bb0([[ARG:%.*]] :
367+
// CHECK: [[STACK:%.*]] = alloc_stack
368+
// CHECK: store [[ARG]] to [[STACK]]
369+
// CHECK: apply {{%.*}}([[STACK]])
370+
// CHECK: } // end sil function 'lower_store_borrow_result_correctly'
371+
sil [ossa] @lower_store_borrow_result_correctly : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
372+
bb0(%0 : @guaranteed $Builtin.NativeObject):
373+
// This is materializing %0 into memory to be passed as an in_guaranteed arg.
374+
%1 = alloc_stack $Builtin.NativeObject
375+
%result = store_borrow %0 to %1 : $*Builtin.NativeObject
376+
%f = function_ref @use_native_object_inguaranteed : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> ()
377+
apply %f(%result) : $@convention(thin) (@in_guaranteed Builtin.NativeObject) -> ()
378+
dealloc_stack %1 : $*Builtin.NativeObject
379+
%9999 = tuple()
380+
return %9999 : $()
381+
}

0 commit comments

Comments
 (0)