Skip to content

Commit 5011584

Browse files
committed
[region-isolation] Handle (project_box (load %boxAddr)) correctly.
I thought that we would never actually hit this test case, but since we do not have opaque values this pattern is treated like a projection in certain cases. I updated the code as appropriate and added a test.
1 parent 6200a3b commit 5011584

File tree

2 files changed

+100
-2
lines changed

2 files changed

+100
-2
lines changed

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,17 @@ struct UseDefChainVisitor
126126
// do not want to treat this as a merge.
127127
if (auto p = Projection(inst)) {
128128
switch (p.getKind()) {
129+
// Currently if we load and then project_box from a memory location,
130+
// we treat that as a projection. This follows the semantics/notes in
131+
// getAccessProjectionOperand.
132+
case ProjectionKind::Box:
133+
return cast<ProjectBoxInst>(inst)->getOperand();
129134
case ProjectionKind::Upcast:
130135
case ProjectionKind::RefCast:
131136
case ProjectionKind::BlockStorageCast:
132137
case ProjectionKind::BitwiseCast:
133-
case ProjectionKind::TailElems:
134-
case ProjectionKind::Box:
135138
case ProjectionKind::Class:
139+
case ProjectionKind::TailElems:
136140
llvm_unreachable("Shouldn't see this here");
137141
case ProjectionKind::Index:
138142
// Index is always a merge.
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// RUN: %target-sil-opt -transfer-non-sendable -enable-experimental-feature RegionBasedIsolation -strict-concurrency=complete %s -o /dev/null -verify
2+
3+
// REQUIRES: concurrency
4+
// REQUIRES: asserts
5+
6+
// PLEASE READ THIS!
7+
//
8+
// This test is specifically meant for small test cases that come from bugs that
9+
// do not categorize by a specific category. If this gets too big, please split
10+
// sections of tests out if possible.
11+
12+
sil_stage raw
13+
14+
import Swift
15+
import Builtin
16+
17+
////////////////////////
18+
// MARK: Declarations //
19+
////////////////////////
20+
21+
class NonSendableKlass {}
22+
23+
sil @transferNonSendableKlass : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
24+
sil @useNonSendableKlass : $@convention(thin) (@guaranteed NonSendableKlass) -> ()
25+
sil @constructNonSendableKlass : $@convention(thin) () -> @owned NonSendableKlass
26+
sil @useUnmanagedNonSendableKlass : $@convention(thin) (@guaranteed @sil_unmanaged NonSendableKlass) -> ()
27+
28+
final class SendableKlass : Sendable {}
29+
30+
sil @transferSendableKlass : $@convention(thin) @async (@guaranteed SendableKlass) -> ()
31+
sil @constructSendableKlass : $@convention(thin) () -> @owned SendableKlass
32+
33+
final class KlassContainingKlasses {
34+
let nsImmutable : NonSendableKlass
35+
var nsMutable : NonSendableKlass
36+
let sImmutable : SendableKlass
37+
var sMutable : SendableKlass
38+
}
39+
40+
sil @transferKlassContainingKlasses : $@convention(thin) @async (@guaranteed KlassContainingKlasses) -> ()
41+
sil @useKlassContainingKlasses : $@convention(thin) (@guaranteed KlassContainingKlasses) -> ()
42+
sil @constructKlassContainingKlasses : $@convention(thin) () -> @owned KlassContainingKlasses
43+
44+
@_moveOnly
45+
struct NonSendableMoveOnlyStruct {
46+
var ns: NonSendableKlass
47+
48+
deinit
49+
}
50+
51+
sil @constructMoveOnlyStruct : $@convention(thin) () -> @owned NonSendableMoveOnlyStruct
52+
sil @transferMoveOnlyStruct : $@convention(thin) @async (@guaranteed NonSendableMoveOnlyStruct) -> ()
53+
54+
struct NonSendableStruct {
55+
var ns: NonSendableKlass
56+
}
57+
58+
sil @constructStruct : $@convention(thin) () -> @owned NonSendableStruct
59+
sil @transferStruct : $@convention(thin) @async (@guaranteed NonSendableStruct) -> ()
60+
61+
sil @transferRawPointer : $@convention(thin) @async (Builtin.RawPointer) -> ()
62+
sil @useRawPointer : $@convention(thin) (Builtin.RawPointer) -> ()
63+
sil @initRawPointer : $@convention(thin) () -> Builtin.RawPointer
64+
65+
sil @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
66+
sil @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
67+
sil @initIndirect : $@convention(thin) <T> () -> @out T
68+
sil @initIndirectTransferring : $@convention(thin) @async <T> () -> @out T
69+
sil @initIndirectTransferringError : $@convention(thin) @async <T> () -> (@out Optional<T>, @error any Error)
70+
71+
enum FakeOptional<T> {
72+
case none
73+
case some(T)
74+
}
75+
76+
/////////////////
77+
// MARK: Tests //
78+
/////////////////
79+
80+
// This goes through the access projection code differently from normal
81+
// project_box since we do not see the alloc_box.
82+
sil [ossa] @project_box_loadable_test_case : $@convention(thin) @async (@in { var NonSendableKlass }) -> () {
83+
bb0(%0 : $*{ var NonSendableKlass }):
84+
%1 = load [take] %0 : $*{ var NonSendableKlass }
85+
%3 = project_box %1 : ${ var NonSendableKlass }, 0
86+
87+
%f = function_ref @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
88+
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %f<NonSendableKlass>(%3) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
89+
// 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}}
90+
destroy_value %1 : ${ var NonSendableKlass }
91+
92+
%9999 = tuple ()
93+
return %9999 : $()
94+
}

0 commit comments

Comments
 (0)