Skip to content

Commit 09151bd

Browse files
gottesmmgottesmm
committed
[region-isolation] Add lookthrough support for a bunch of instructions.
Specifically: * MoveValueInst * MarkUnresolvedNonCopyableValueInst * MarkUnresolvedReferenceBindingInst * CopyableToMoveOnlyWrapperValueInst * MoveOnlyWrapperToCopyableValueInst * MoveOnlyWrapperToCopyableBoxInst * MoveOnlyWrapperToCopyableAddrInst * CopyableToMoveOnlyWrapperAddrInst * MarkUninitializedInst
1 parent 13d48d2 commit 09151bd

File tree

4 files changed

+207
-17
lines changed

4 files changed

+207
-17
lines changed

‎include/swift/SIL/MemAccessUtils.h

+2
Original file line numberDiff line numberDiff line change
@@ -1639,6 +1639,8 @@ inline bool isAccessStorageIdentityCast(SingleValueInstruction *svi) {
16391639
case SILInstructionKind::MarkUnresolvedReferenceBindingInst:
16401640
case SILInstructionKind::MarkDependenceInst:
16411641
case SILInstructionKind::CopyValueInst:
1642+
case SILInstructionKind::BeginBorrowInst:
1643+
case SILInstructionKind::MoveOnlyWrapperToCopyableBoxInst:
16421644
return true;
16431645
}
16441646
}

‎lib/SIL/Utils/MemAccessUtils.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -775,11 +775,11 @@ LLVM_ATTRIBUTE_USED void AccessBase::dump() const { print(llvm::dbgs()); }
775775

776776
bool swift::isIdentityPreservingRefCast(SingleValueInstruction *svi) {
777777
// Ignore both copies and other identity and ownership preserving casts
778-
return isa<CopyValueInst>(svi) || isa<BeginBorrowInst>(svi)
779-
|| isa<EndInitLetRefInst>(svi)
780-
|| isa<BeginDeallocRefInst>(svi)
781-
|| isa<EndCOWMutationInst>(svi)
782-
|| isIdentityAndOwnershipPreservingRefCast(svi);
778+
return isa<CopyValueInst>(svi) || isa<BeginBorrowInst>(svi) ||
779+
isa<EndInitLetRefInst>(svi) || isa<BeginDeallocRefInst>(svi) ||
780+
isa<EndCOWMutationInst>(svi) ||
781+
isa<MarkUnresolvedReferenceBindingInst>(svi) ||
782+
isIdentityAndOwnershipPreservingRefCast(svi);
783783
}
784784

785785
// On some platforms, casting from a metatype to a reference type dynamically

‎lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

+15-11
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,12 @@ static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
199199

200200
temp = getUnderlyingObject(temp);
201201

202-
if (auto *cvi = dyn_cast<ExplicitCopyValueInst>(temp)) {
203-
temp = cvi->getOperand();
202+
if (auto *svi = dyn_cast<SingleValueInstruction>(temp)) {
203+
if (isa<ExplicitCopyValueInst, CopyableToMoveOnlyWrapperValueInst,
204+
MoveOnlyWrapperToCopyableValueInst,
205+
MoveOnlyWrapperToCopyableBoxInst>(svi)) {
206+
temp = svi->getOperand(0);
207+
}
204208
}
205209

206210
if (auto *dsi = dyn_cast_or_null<DestructureStructInst>(
@@ -2325,6 +2329,15 @@ CONSTANT_TRANSLATION(OpenExistentialAddrInst, LookThrough)
23252329
CONSTANT_TRANSLATION(UncheckedRefCastInst, LookThrough)
23262330
CONSTANT_TRANSLATION(UncheckedTakeEnumDataAddrInst, LookThrough)
23272331
CONSTANT_TRANSLATION(UpcastInst, LookThrough)
2332+
CONSTANT_TRANSLATION(MoveValueInst, LookThrough)
2333+
CONSTANT_TRANSLATION(MarkUnresolvedNonCopyableValueInst, LookThrough)
2334+
CONSTANT_TRANSLATION(MarkUnresolvedReferenceBindingInst, LookThrough)
2335+
CONSTANT_TRANSLATION(CopyableToMoveOnlyWrapperValueInst, LookThrough)
2336+
CONSTANT_TRANSLATION(MoveOnlyWrapperToCopyableValueInst, LookThrough)
2337+
CONSTANT_TRANSLATION(MoveOnlyWrapperToCopyableBoxInst, LookThrough)
2338+
CONSTANT_TRANSLATION(MoveOnlyWrapperToCopyableAddrInst, LookThrough)
2339+
CONSTANT_TRANSLATION(CopyableToMoveOnlyWrapperAddrInst, LookThrough)
2340+
CONSTANT_TRANSLATION(MarkUninitializedInst, LookThrough)
23282341
// We identify destructured results with their operand's region.
23292342
CONSTANT_TRANSLATION(DestructureTupleInst, LookThrough)
23302343
CONSTANT_TRANSLATION(DestructureStructInst, LookThrough)
@@ -2394,19 +2407,10 @@ CONSTANT_TRANSLATION(UnownedCopyValueInst, Unhandled)
23942407
CONSTANT_TRANSLATION(WeakCopyValueInst, Unhandled)
23952408
CONSTANT_TRANSLATION(StrongCopyWeakValueInst, Unhandled)
23962409
CONSTANT_TRANSLATION(StrongCopyUnmanagedValueInst, Unhandled)
2397-
CONSTANT_TRANSLATION(MoveValueInst, Unhandled)
23982410
CONSTANT_TRANSLATION(DropDeinitInst, Unhandled)
2399-
CONSTANT_TRANSLATION(MarkUnresolvedNonCopyableValueInst, Unhandled)
2400-
CONSTANT_TRANSLATION(MarkUnresolvedReferenceBindingInst, Unhandled)
2401-
CONSTANT_TRANSLATION(CopyableToMoveOnlyWrapperValueInst, Unhandled)
2402-
CONSTANT_TRANSLATION(MoveOnlyWrapperToCopyableValueInst, Unhandled)
2403-
CONSTANT_TRANSLATION(MoveOnlyWrapperToCopyableBoxInst, Unhandled)
2404-
CONSTANT_TRANSLATION(MoveOnlyWrapperToCopyableAddrInst, Unhandled)
2405-
CONSTANT_TRANSLATION(CopyableToMoveOnlyWrapperAddrInst, Unhandled)
24062411
CONSTANT_TRANSLATION(MarkUnresolvedMoveAddrInst, Unhandled)
24072412
CONSTANT_TRANSLATION(IsUniqueInst, Unhandled)
24082413
CONSTANT_TRANSLATION(LoadUnownedInst, Unhandled)
2409-
CONSTANT_TRANSLATION(MarkUninitializedInst, Unhandled)
24102414
CONSTANT_TRANSLATION(ProjectExistentialBoxInst, Unhandled)
24112415
CONSTANT_TRANSLATION(ValueMetatypeInst, Unhandled)
24122416
CONSTANT_TRANSLATION(ExistentialMetatypeInst, Unhandled)

‎test/Concurrency/sendnonsendable_basic.sil

+185-1
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,24 @@ import Swift
66

77
class NonSendableKlass {}
88

9+
@_moveOnly
10+
struct NonSendableMoveOnlyStruct {
11+
var ns: NonSendableKlass
12+
}
13+
14+
struct NonSendableStruct {
15+
var ns: NonSendableKlass
16+
}
17+
918
sil @transferKlass : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
10-
sil @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
1119
sil @useKlass : $@convention(thin) (@guaranteed NonSendableKlass) -> ()
1220
sil @constructKlass : $@convention(thin) () -> @owned NonSendableKlass
1321

22+
sil @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
23+
sil @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
24+
25+
sil @constructMoveOnlyStruct : $@convention(thin) () -> @owned NonSendableMoveOnlyStruct
26+
1427
enum FakeOptional<T> {
1528
case none
1629
case some(T)
@@ -105,3 +118,174 @@ bb0:
105118
%9999 = tuple ()
106119
return %9999 : $()
107120
}
121+
122+
sil [ossa] @move_value_test : $@convention(thin) @async () -> () {
123+
bb0:
124+
%0 = function_ref @constructKlass : $@convention(thin) () -> @owned NonSendableKlass
125+
%1 = apply %0() : $@convention(thin) () -> @owned NonSendableKlass
126+
%2 = function_ref @transferKlass : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
127+
%1a = move_value %1 : $NonSendableKlass
128+
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %2(%1a) : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
129+
// expected-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass' from nonisolated context to global actor '<null>'-isolated context at this call site could yield a race with accesses later in this function}}
130+
%3 = function_ref @useKlass : $@convention(thin) (@guaranteed NonSendableKlass) -> ()
131+
apply %3(%1a) : $@convention(thin) (@guaranteed NonSendableKlass) -> ()
132+
// expected-note @-1 {{access here could race}}
133+
destroy_value %1a : $NonSendableKlass
134+
%9999 = tuple ()
135+
return %9999 : $()
136+
}
137+
138+
sil [ossa] @mark_unresolved_noncopyable_value_test : $@convention(thin) @async () -> () {
139+
bb0:
140+
%0 = function_ref @constructMoveOnlyStruct : $@convention(thin) () -> @owned NonSendableMoveOnlyStruct
141+
%1 = apply %0() : $@convention(thin) () -> @owned NonSendableMoveOnlyStruct
142+
%box = alloc_box ${ var NonSendableMoveOnlyStruct }
143+
%project = project_box %box : ${ var NonSendableMoveOnlyStruct }, 0
144+
%unresolved = mark_unresolved_non_copyable_value [consumable_and_assignable] %project : $*NonSendableMoveOnlyStruct
145+
store %1 to [init] %unresolved : $*NonSendableMoveOnlyStruct
146+
147+
%4 = function_ref @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
148+
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %4<NonSendableMoveOnlyStruct>(%unresolved) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
149+
// expected-warning @-1 {{passing argument of non-sendable type 'NonSendableMoveOnlyStruct' from nonisolated context to global actor '<null>'-isolated context at this call site could yield a race with accesses later in this function}}
150+
%5 = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
151+
apply %5<NonSendableMoveOnlyStruct>(%unresolved) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
152+
// expected-note @-1 {{access here could race}}
153+
destroy_value %box : ${ var NonSendableMoveOnlyStruct }
154+
%9999 = tuple ()
155+
return %9999 : $()
156+
}
157+
158+
sil [ossa] @mark_unresolved_reference_binding_test : $@convention(thin) @async () -> () {
159+
bb0:
160+
%0 = function_ref @constructKlass : $@convention(thin) () -> @owned NonSendableKlass
161+
%1 = apply %0() : $@convention(thin) () -> @owned NonSendableKlass
162+
%box = alloc_box ${ var NonSendableKlass }
163+
%binding = mark_unresolved_reference_binding [inout] %box : ${ var NonSendableKlass }
164+
%project = project_box %binding : ${ var NonSendableKlass }, 0
165+
store %1 to [init] %project : $*NonSendableKlass
166+
167+
%4 = function_ref @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
168+
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %4<NonSendableKlass>(%project) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
169+
// expected-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass' from nonisolated context to global actor '<null>'-isolated context at this call site could yield a race with accesses later in this function}}
170+
%5 = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
171+
apply %5<NonSendableKlass>(%project) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
172+
// expected-note @-1 {{access here could race}}
173+
destroy_value %binding : ${ var NonSendableKlass }
174+
%9999 = tuple ()
175+
return %9999 : $()
176+
}
177+
178+
sil [ossa] @copyable_to_moveonly_wrapper_value_and_back_test : $@convention(thin) @async () -> () {
179+
bb0:
180+
%0 = function_ref @constructKlass : $@convention(thin) () -> @owned NonSendableKlass
181+
%1 = apply %0() : $@convention(thin) () -> @owned NonSendableKlass
182+
%2 = function_ref @transferKlass : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
183+
%0a = copyable_to_moveonlywrapper [owned] %1 : $NonSendableKlass
184+
185+
%0b = begin_borrow %0a : $@moveOnly NonSendableKlass
186+
%0c = moveonlywrapper_to_copyable [guaranteed] %0b : $@moveOnly NonSendableKlass
187+
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %2(%0c) : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
188+
// expected-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass' from nonisolated context to global actor '<null>'-isolated context at this call site could yield a race with accesses later in this function}}
189+
end_borrow %0b : $@moveOnly NonSendableKlass
190+
%3 = function_ref @useKlass : $@convention(thin) (@guaranteed NonSendableKlass) -> ()
191+
%0bb = begin_borrow %0a : $@moveOnly NonSendableKlass
192+
%0d = moveonlywrapper_to_copyable [guaranteed] %0bb : $@moveOnly NonSendableKlass
193+
apply %3(%0d) : $@convention(thin) (@guaranteed NonSendableKlass) -> ()
194+
// expected-note @-1 {{access here could race}}
195+
end_borrow %0bb : $@moveOnly NonSendableKlass
196+
destroy_value %0a : $@moveOnly NonSendableKlass
197+
%9999 = tuple ()
198+
return %9999 : $()
199+
}
200+
201+
sil [ossa] @test_moveonlywrapper_to_copyable_addr : $@convention(thin) @async () -> () {
202+
bb0:
203+
%1 = function_ref @constructKlass : $@convention(thin) () -> @owned NonSendableKlass
204+
%2 = apply %1() : $@convention(thin) () -> @owned NonSendableKlass
205+
%box = alloc_box ${ var @moveOnly NonSendableKlass }
206+
%bb = begin_borrow [var_decl] %box : ${ var @moveOnly NonSendableKlass }
207+
%project = project_box %bb : ${ var @moveOnly NonSendableKlass }, 0
208+
%unwrappedProject = moveonlywrapper_to_copyable_addr %project : $*@moveOnly NonSendableKlass
209+
store %2 to [init] %unwrappedProject : $*NonSendableKlass
210+
211+
%4 = function_ref @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
212+
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %4<NonSendableKlass>(%unwrappedProject) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
213+
// expected-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass' from nonisolated context to global actor '<null>'-isolated context at this call site could yield a race with accesses later in this function}}
214+
215+
%5 = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
216+
apply %5<NonSendableKlass>(%unwrappedProject) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
217+
// expected-note @-1 {{access here could race}}
218+
219+
end_borrow %bb : ${ var @moveOnly NonSendableKlass }
220+
destroy_value %box : ${ var @moveOnly NonSendableKlass }
221+
%9999 = tuple ()
222+
return %9999 : $()
223+
}
224+
225+
sil @partial_apply_box : $@convention(thin) (@guaranteed { var NonSendableKlass }) -> ()
226+
sil @transfer_partial_apply : $@convention(thin) @async (@guaranteed @callee_owned () -> ()) -> ()
227+
228+
sil [ossa] @test_moveonlywrapper_to_copyable_box : $@convention(thin) @async () -> () {
229+
bb0:
230+
%1 = function_ref @constructKlass : $@convention(thin) () -> @owned NonSendableKlass
231+
%2 = apply %1() : $@convention(thin) () -> @owned NonSendableKlass
232+
%box = alloc_box ${ var @moveOnly NonSendableKlass }
233+
%bb = begin_borrow %box : ${ var @moveOnly NonSendableKlass }
234+
%project = project_box %bb : ${ var @moveOnly NonSendableKlass }, 0
235+
%unwrappedProject = moveonlywrapper_to_copyable_addr %project : $*@moveOnly NonSendableKlass
236+
store %2 to [init] %unwrappedProject : $*NonSendableKlass
237+
end_borrow %bb : ${ var @moveOnly NonSendableKlass }
238+
239+
%unwrappedBox = moveonlywrapper_to_copyable_box %box : ${ var @moveOnly NonSendableKlass }
240+
241+
%f2 = function_ref @partial_apply_box : $@convention(thin) (@guaranteed { var NonSendableKlass }) -> ()
242+
%copiedUnwrappedBox = copy_value %unwrappedBox : ${ var NonSendableKlass }
243+
%pa = partial_apply %f2(%copiedUnwrappedBox) : $@convention(thin) (@guaranteed { var NonSendableKlass }) -> ()
244+
%4 = function_ref @transfer_partial_apply : $@convention(thin) @async (@guaranteed @callee_owned () -> ()) -> ()
245+
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %4(%pa) : $@convention(thin) @async (@guaranteed @callee_owned () -> ()) -> ()
246+
// expected-warning @-1 {{passing argument of non-sendable type '@callee_owned () -> ()' from nonisolated context to global actor '<null>'-isolated context at this call site could yield a race with accesses later in this function}}
247+
%bb2 = begin_borrow %unwrappedBox : ${ var NonSendableKlass }
248+
%p2 = project_box %bb2 : ${ var NonSendableKlass }, 0
249+
%5 = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
250+
apply %5<NonSendableKlass>(%p2) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
251+
// expected-note @-1 {{access here could race}}
252+
end_borrow %bb2 : ${ var NonSendableKlass }
253+
254+
destroy_value %pa : $@callee_owned () -> ()
255+
destroy_value %unwrappedBox : ${ var NonSendableKlass }
256+
%9999 = tuple ()
257+
return %9999 : $()
258+
}
259+
260+
sil [ossa] @mark_uninitialized_test : $@convention(method) @async (@owned NonSendableStruct) -> () {
261+
bb0(%0 : @owned $NonSendableStruct):
262+
%1 = alloc_stack $NonSendableStruct, var, name "self"
263+
%2 = mark_uninitialized [rootself] %1 : $*NonSendableStruct
264+
%7 = begin_access [modify] [static] %2 : $*NonSendableStruct
265+
store %0 to [init] %7 : $*NonSendableStruct
266+
end_access %7 : $*NonSendableStruct
267+
268+
%4 = function_ref @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
269+
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %4<NonSendableStruct>(%2) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
270+
// expected-warning @-1 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
271+
272+
%5 = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
273+
apply %5<NonSendableStruct>(%2) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
274+
275+
destroy_addr %1 : $*NonSendableStruct
276+
dealloc_stack %1 : $*NonSendableStruct
277+
278+
%9999 = tuple ()
279+
return %9999 : $()
280+
}
281+
282+
sil [ossa] @copyable_to_moveonlywrapper_addr_test : $@convention(thin) @async (@in_guaranteed NonSendableStruct) -> () {
283+
bb0(%0 : $*NonSendableStruct):
284+
%1 = copyable_to_moveonlywrapper_addr %0 : $*NonSendableStruct
285+
%2 = moveonlywrapper_to_copyable_addr %1 : $*@moveOnly NonSendableStruct
286+
%4 = function_ref @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
287+
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %4<NonSendableStruct>(%2) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
288+
// expected-warning @-1 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
289+
%9999 = tuple ()
290+
return %9999 : $()
291+
}

0 commit comments

Comments
 (0)