Skip to content

Commit 29e3069

Browse files
gottesmmgottesmm
committed
[region-isolation] Teach variable name utils how to handle more tuple patterns.
Just slicing off from a larger patch.
1 parent 8b899b8 commit 29e3069

File tree

4 files changed

+197
-19
lines changed

4 files changed

+197
-19
lines changed

‎lib/SILOptimizer/Utils/VariableNameUtils.cpp

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,57 @@ static SILValue findRootValueForTupleTempAllocation(AllocationInst *allocInst,
133133

134134
if (auto *si = dyn_cast<StoreInst>(&inst)) {
135135
if (si->getOwnershipQualifier() != StoreOwnershipQualifier::Assign) {
136+
// Check if we are updating the entire tuple value.
137+
if (si->getDest() == allocInst) {
138+
// If we already found a root address (meaning we were processing
139+
// tuple_elt_addr), bail. We have some sort of unhandled mix of
140+
// copy_addr and store.
141+
if (foundRootAddress)
142+
return SILValue();
143+
144+
// If we already found a destructure, return SILValue(). We are
145+
// initializing twice.
146+
if (foundDestructure)
147+
return SILValue();
148+
149+
// We are looking for a pattern where we construct a tuple from
150+
// destructured parts.
151+
if (auto *ti = dyn_cast<TupleInst>(si->getSrc())) {
152+
for (auto p : llvm::enumerate(ti->getOperandValues())) {
153+
SILValue value = lookThroughOwnershipInsts(p.value());
154+
if (auto *dti = dyn_cast_or_null<DestructureTupleInst>(
155+
value->getDefiningInstruction())) {
156+
// We should always go through the same dti.
157+
if (foundDestructure && foundDestructure != dti)
158+
return SILValue();
159+
if (!foundDestructure)
160+
foundDestructure = dti;
161+
162+
// If we have a mixmatch of indices, we cannot look through.
163+
if (p.index() != dti->getIndexOfResult(value))
164+
return SILValue();
165+
if (tupleValues[p.index()])
166+
return SILValue();
167+
tupleValues[p.index()] = value;
168+
169+
// If we have completely covered the tuple, break.
170+
--numEltsLeft;
171+
if (!numEltsLeft)
172+
break;
173+
}
174+
}
175+
176+
// If we haven't completely covered the tuple, return SILValue(). We
177+
// should completely cover the tuple.
178+
if (numEltsLeft)
179+
return SILValue();
180+
181+
// Otherwise, break since we are done.
182+
break;
183+
}
184+
}
185+
186+
// If we store to a tuple_element_addr, update for a single value.
136187
if (auto *tei = dyn_cast<TupleElementAddrInst>(si->getDest())) {
137188
if (tei->getOperand() == allocInst) {
138189
unsigned i = tei->getFieldIndex();

‎test/Concurrency/transfernonsendable.swift

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
////////////////////////
1616

1717
/// Classes are always non-sendable, so this is non-sendable
18-
class NonSendableKlass { // expected-complete-note 35{{}}
18+
class NonSendableKlass { // expected-complete-note 36{{}}
1919
// expected-typechecker-only-note @-1 4{{}}
2020
// expected-tns-note @-2 2{{}}
2121
var field: NonSendableKlass? = nil
@@ -1275,26 +1275,29 @@ func varNonSendableNonTrivialLetStructFieldClosureFlowSensitive7() async {
12751275

12761276
func varSendableTrivialLetTupleFieldTest() async {
12771277
let test = (0, SendableKlass(), NonSendableKlass())
1278-
await transferToMain(test) // expected-tns-warning {{transferring value of non-Sendable type '(Int, SendableKlass, NonSendableKlass)' from nonisolated context to main actor-isolated context; later accesses could race}}
1279-
// expected-complete-warning @-1 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
1278+
await transferToMain(test) // expected-tns-warning {{transferring 'test' may cause a race}}
1279+
// expected-tns-note @-1 {{'test' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
1280+
// expected-complete-warning @-2 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
12801281
let z = test.0
12811282
useValue(z)
12821283
useValue(test) // expected-tns-note {{access here could race}}
12831284
}
12841285

12851286
func varSendableNonTrivialLetTupleFieldTest() async {
12861287
let test = (0, SendableKlass(), NonSendableKlass())
1287-
await transferToMain(test) // expected-tns-warning {{transferring value of non-Sendable type '(Int, SendableKlass, NonSendableKlass)' from nonisolated context to main actor-isolated context; later accesses could race}}
1288-
// expected-complete-warning @-1 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
1288+
await transferToMain(test) // expected-tns-warning {{transferring 'test' may cause a race}}
1289+
// expected-tns-note @-1 {{'test' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
1290+
// expected-complete-warning @-2 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
12891291
let z = test.1
12901292
useValue(z)
12911293
useValue(test) // expected-tns-note {{access here could race}}
12921294
}
12931295

12941296
func varNonSendableNonTrivialLetTupleFieldTest() async {
12951297
let test = (0, SendableKlass(), NonSendableKlass())
1296-
await transferToMain(test) // expected-tns-warning {{transferring value of non-Sendable type '(Int, SendableKlass, NonSendableKlass)' from nonisolated context to main actor-isolated context; later accesses could race}}
1297-
// expected-complete-warning @-1 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
1298+
await transferToMain(test) // expected-tns-warning {{transferring 'test' may cause a race}}
1299+
// expected-tns-note @-1 {{'test' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
1300+
// expected-complete-warning @-2 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
12981301
let z = test.2
12991302
// The SIL emitted for the assignment of the tuple is just a jumble of
13001303
// instructions that are not semantically significant. The result is that we
@@ -1306,26 +1309,39 @@ func varNonSendableNonTrivialLetTupleFieldTest() async {
13061309
func varSendableTrivialVarTupleFieldTest() async {
13071310
var test = (0, SendableKlass(), NonSendableKlass())
13081311
test = (0, SendableKlass(), NonSendableKlass())
1309-
await transferToMain(test) // expected-tns-warning {{transferring value of non-Sendable type '(Int, SendableKlass, NonSendableKlass)' from nonisolated context to main actor-isolated context; later accesses could race}}
1310-
// expected-complete-warning @-1 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
1312+
await transferToMain(test) // expected-tns-warning {{transferring 'test' may cause a race}}
1313+
// expected-tns-note @-1 {{'test' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
1314+
// expected-complete-warning @-2 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
1315+
_ = test.0
1316+
useValue(test) // expected-tns-note {{access here could race}}
1317+
}
1318+
1319+
func varSendableTrivialVarTupleFieldTest2() async {
1320+
var test = (0, SendableKlass(), NonSendableKlass())
1321+
test = (0, SendableKlass(), NonSendableKlass())
1322+
await transferToMain(test.2) // expected-tns-warning {{transferring 'test.2' may cause a race}}
1323+
// expected-tns-note @-1 {{'test.2' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
1324+
// expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
13111325
_ = test.0
13121326
useValue(test) // expected-tns-note {{access here could race}}
13131327
}
13141328

13151329
func varSendableNonTrivialVarTupleFieldTest() async {
13161330
var test = (0, SendableKlass(), NonSendableKlass())
13171331
test = (0, SendableKlass(), NonSendableKlass())
1318-
await transferToMain(test) // expected-tns-warning {{transferring value of non-Sendable type '(Int, SendableKlass, NonSendableKlass)' from nonisolated context to main actor-isolated context; later accesses could race}}
1319-
// expected-complete-warning @-1 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
1332+
await transferToMain(test) // expected-tns-warning {{transferring 'test' may cause a race}}
1333+
// expected-tns-note @-1 {{'test' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
1334+
// expected-complete-warning @-2 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
13201335
_ = test.1
13211336
useValue(test) // expected-tns-note {{access here could race}}
13221337
}
13231338

13241339
func varNonSendableNonTrivialVarTupleFieldTest() async {
13251340
var test = (0, SendableKlass(), NonSendableKlass())
13261341
test = (0, SendableKlass(), NonSendableKlass())
1327-
await transferToMain(test) // expected-tns-warning {{transferring value of non-Sendable type '(Int, SendableKlass, NonSendableKlass)' from nonisolated context to main actor-isolated context; later accesses could race}}
1328-
// expected-complete-warning @-1 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
1342+
await transferToMain(test) // expected-tns-warning {{transferring 'test' may cause a race}}
1343+
// expected-tns-note @-1 {{'test' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
1344+
// expected-complete-warning @-2 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
13291345
let z = test.2 // expected-tns-note {{access here could race}}
13301346
useValue(z)
13311347
useValue(test)

‎test/Concurrency/transfernonsendable_global_actor.swift

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,10 @@ private struct StructContainingValue { // expected-complete-note 2{{}}
130130
var x = (NonSendableLinkedList<Int>(), NonSendableLinkedList<Int>())
131131
x = (NonSendableLinkedList<Int>(), NonSendableLinkedList<Int>())
132132

133-
await transferToNonIsolated(x) // expected-tns-warning {{transferring value of non-Sendable type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' from global actor 'GlobalActor'-isolated context to nonisolated context; later accesses could race}}
134-
// expected-complete-warning @-1 {{passing argument of non-sendable type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' outside of global actor 'GlobalActor'-isolated context may introduce data races}}
133+
await transferToNonIsolated(x) // expected-tns-warning {{transferring 'x' may cause a race}}
134+
// expected-tns-note @-1 {{'x' is transferred from global actor 'GlobalActor'-isolated caller to nonisolated callee. Later uses in caller could race with potential uses in callee}}
135135
// expected-complete-warning @-2 {{passing argument of non-sendable type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' outside of global actor 'GlobalActor'-isolated context may introduce data races}}
136+
// expected-complete-warning @-3 {{passing argument of non-sendable type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' outside of global actor 'GlobalActor'-isolated context may introduce data races}}
136137

137138
useValue(x) // expected-tns-note {{access here could race}}
138139
}
@@ -142,10 +143,10 @@ private struct StructContainingValue { // expected-complete-note 2{{}}
142143

143144
x.1 = firstList
144145

145-
// TODO: This should be a global actor isolated error, not a task-isolated error.
146-
await transferToNonIsolated(x) // expected-tns-warning {{global actor 'GlobalActor'-isolated value of type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' transferred to nonisolated context}}
147-
// expected-complete-warning @-1 {{passing argument of non-sendable type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' outside of global actor 'GlobalActor'-isolated context may introduce data races}}
146+
await transferToNonIsolated(x) // expected-tns-warning {{transferring 'x' may cause a race}}
147+
// expected-tns-note @-1 {{transferring global actor 'GlobalActor'-isolated 'x' to nonisolated callee could cause races between nonisolated and global actor 'GlobalActor'-isolated uses}}
148148
// expected-complete-warning @-2 {{passing argument of non-sendable type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' outside of global actor 'GlobalActor'-isolated context may introduce data races}}
149+
// expected-complete-warning @-3 {{passing argument of non-sendable type '(NonSendableLinkedList<Int>, NonSendableLinkedList<Int>)' outside of global actor 'GlobalActor'-isolated context may introduce data races}}
149150

150151
useValue(x)
151152
}

‎test/SILOptimizer/variable_name_inference.sil

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,11 +616,121 @@ bb0(%0 : @noImplicitCopy @guaranteed $Klass):
616616
%8 = moveonlywrapper_to_copyable [guaranteed] %7 : $@moveOnly Klass
617617
%9 = alloc_stack $Klass
618618
%10 = store_borrow %8 to %9 : $*Klass
619-
debug_value [trace] %10 : $*Klass
619+
debug_value [trace] %10 : $*Klass
620620
end_borrow %10 : $*Klass
621621
dealloc_stack %9 : $*Klass
622622
end_borrow %7 : $@moveOnly Klass
623623
destroy_value %3 : $@moveOnly Klass
624624
%25 = tuple ()
625625
return %25 : $()
626626
}
627+
628+
// CHECK-LABEL: begin running test 1 of 1 on store_borrow_tuple_test: variable-name-inference with: @trace[0]
629+
// CHECK: Input Value: %25 = store_borrow %23 to %24 : $*Klass
630+
// CHECK: Name: 'z'
631+
// CHECK: Root: %20 = move_value [lexical] [var_decl] %19 : $Klass
632+
// CHECK: end running test 1 of 1 on store_borrow_tuple_test: variable-name-inference with: @trace[0]
633+
sil hidden [ossa] @store_borrow_tuple_test : $@convention(thin) @async () -> () {
634+
bb0:
635+
specify_test "variable-name-inference @trace[0]"
636+
%2 = integer_literal $Builtin.IntLiteral, 0
637+
%7 = function_ref @getKlass : $@convention(thin) () -> @owned Klass
638+
%8 = apply %7() : $@convention(thin) () -> @owned Klass
639+
%9 = tuple (%2 : $Builtin.IntLiteral, %8 : $Klass)
640+
%10 = move_value [lexical] [var_decl] %9 : $(Builtin.IntLiteral, Klass)
641+
debug_value %10 : $(Builtin.IntLiteral, Klass), let, name "test"
642+
%12 = begin_borrow %10 : $(Builtin.IntLiteral, Klass)
643+
(%13, %14) = destructure_tuple %12 : $(Builtin.IntLiteral, Klass)
644+
%15 = copy_value %14 : $Klass
645+
%16 = tuple (%13 : $Builtin.IntLiteral, %15 : $Klass)
646+
%17 = alloc_stack $(Builtin.IntLiteral, Klass)
647+
store %16 to [init] %17 : $*(Builtin.IntLiteral, Klass)
648+
// original location of use.
649+
destroy_addr %17 : $*(Builtin.IntLiteral, Klass)
650+
dealloc_stack %17 : $*(Builtin.IntLiteral, Klass)
651+
end_borrow %12 : $(Builtin.IntLiteral, Klass)
652+
%25 = begin_borrow %10 : $(Builtin.IntLiteral, Klass)
653+
%26 = copy_value %25 : $(Builtin.IntLiteral, Klass)
654+
(%27, %28) = destructure_tuple %26 : $(Builtin.IntLiteral, Klass)
655+
%29 = move_value [lexical] [var_decl] %28 : $Klass
656+
debug_value %29 : $Klass, let, name "z"
657+
end_borrow %25 : $(Builtin.IntLiteral, Klass)
658+
%32 = begin_borrow %29 : $Klass
659+
%33 = alloc_stack $Klass
660+
%34 = store_borrow %32 to %33 : $*Klass
661+
debug_value [trace] %34 : $*Klass
662+
%35 = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
663+
%36 = apply %35<Klass>(%34) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
664+
end_borrow %34 : $*Klass
665+
dealloc_stack %33 : $*Klass
666+
end_borrow %32 : $Klass
667+
%40 = begin_borrow %10 : $(Builtin.IntLiteral, Klass)
668+
(%41, %42) = destructure_tuple %40 : $(Builtin.IntLiteral, Klass)
669+
%43 = copy_value %42 : $Klass
670+
%44 = tuple (%41 : $Builtin.IntLiteral, %43 : $Klass)
671+
%45 = alloc_stack $(Builtin.IntLiteral, Klass)
672+
store %44 to [init] %45 : $*(Builtin.IntLiteral, Klass)
673+
%48 = apply %35<(Builtin.IntLiteral, Klass)>(%45) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
674+
destroy_addr %45 : $*(Builtin.IntLiteral, Klass)
675+
dealloc_stack %45 : $*(Builtin.IntLiteral, Klass)
676+
end_borrow %40 : $(Builtin.IntLiteral, Klass)
677+
destroy_value %29 : $Klass
678+
destroy_value %10 : $(Builtin.IntLiteral, Klass)
679+
%54 = tuple ()
680+
return %54 : $()
681+
}
682+
683+
// CHECK-LABEL: begin running test 1 of 1 on store_borrow_tuple_test_2: variable-name-inference with: @trace[0]
684+
// CHECK: Input Value: %11 = alloc_stack $(Builtin.IntLiteral, Klass)
685+
// CHECK: Name: 'test'
686+
// CHECK: Root: %4 = move_value [lexical] [var_decl] %3 : $(Builtin.IntLiteral, Klass)
687+
// CHECK: end running test 1 of 1 on store_borrow_tuple_test_2: variable-name-inference with: @trace[0]
688+
sil hidden [ossa] @store_borrow_tuple_test_2 : $@convention(thin) @async () -> () {
689+
bb0:
690+
specify_test "variable-name-inference @trace[0]"
691+
%2 = integer_literal $Builtin.IntLiteral, 0
692+
%7 = function_ref @getKlass : $@convention(thin) () -> @owned Klass
693+
%8 = apply %7() : $@convention(thin) () -> @owned Klass
694+
%9 = tuple (%2 : $Builtin.IntLiteral, %8 : $Klass)
695+
%10 = move_value [lexical] [var_decl] %9 : $(Builtin.IntLiteral, Klass)
696+
debug_value %10 : $(Builtin.IntLiteral, Klass), let, name "test"
697+
%12 = begin_borrow %10 : $(Builtin.IntLiteral, Klass)
698+
(%13, %14) = destructure_tuple %12 : $(Builtin.IntLiteral, Klass)
699+
%15 = copy_value %14 : $Klass
700+
%16 = tuple (%13 : $Builtin.IntLiteral, %15 : $Klass)
701+
%17 = alloc_stack $(Builtin.IntLiteral, Klass)
702+
store %16 to [init] %17 : $*(Builtin.IntLiteral, Klass)
703+
debug_value [trace] %17 : $*(Builtin.IntLiteral, Klass)
704+
// original location of use.
705+
destroy_addr %17 : $*(Builtin.IntLiteral, Klass)
706+
dealloc_stack %17 : $*(Builtin.IntLiteral, Klass)
707+
end_borrow %12 : $(Builtin.IntLiteral, Klass)
708+
%25 = begin_borrow %10 : $(Builtin.IntLiteral, Klass)
709+
%26 = copy_value %25 : $(Builtin.IntLiteral, Klass)
710+
(%27, %28) = destructure_tuple %26 : $(Builtin.IntLiteral, Klass)
711+
%29 = move_value [lexical] [var_decl] %28 : $Klass
712+
debug_value %29 : $Klass, let, name "z"
713+
end_borrow %25 : $(Builtin.IntLiteral, Klass)
714+
%32 = begin_borrow %29 : $Klass
715+
%33 = alloc_stack $Klass
716+
%34 = store_borrow %32 to %33 : $*Klass
717+
%35 = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
718+
%36 = apply %35<Klass>(%34) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
719+
end_borrow %34 : $*Klass
720+
dealloc_stack %33 : $*Klass
721+
end_borrow %32 : $Klass
722+
%40 = begin_borrow %10 : $(Builtin.IntLiteral, Klass)
723+
(%41, %42) = destructure_tuple %40 : $(Builtin.IntLiteral, Klass)
724+
%43 = copy_value %42 : $Klass
725+
%44 = tuple (%41 : $Builtin.IntLiteral, %43 : $Klass)
726+
%45 = alloc_stack $(Builtin.IntLiteral, Klass)
727+
store %44 to [init] %45 : $*(Builtin.IntLiteral, Klass)
728+
%48 = apply %35<(Builtin.IntLiteral, Klass)>(%45) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
729+
destroy_addr %45 : $*(Builtin.IntLiteral, Klass)
730+
dealloc_stack %45 : $*(Builtin.IntLiteral, Klass)
731+
end_borrow %40 : $(Builtin.IntLiteral, Klass)
732+
destroy_value %29 : $Klass
733+
destroy_value %10 : $(Builtin.IntLiteral, Klass)
734+
%54 = tuple ()
735+
return %54 : $()
736+
}

0 commit comments

Comments
 (0)