Skip to content

Commit c5e8601

Browse files
authored
Merge pull request #74677 from gottesmm/release/6.0-rdar129400019
[6.0][transfer-non-sendable] Teach SILIsolationInfo how to handle "look through instructions" when finding actor instances.
2 parents 9d75e3f + 6c2c302 commit c5e8601

File tree

8 files changed

+99
-9
lines changed

8 files changed

+99
-9
lines changed

include/swift/SILOptimizer/Utils/SILIsolationInfo.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,16 @@ class ActorInstance {
5555
ActorInstance(SILValue value, Kind kind)
5656
: value(value, std::underlying_type<Kind>::type(kind)) {}
5757

58+
/// We want to look through certain instructions like end_init_ref that have
59+
/// the appropriate actor type but could disguise the actual underlying value
60+
/// that we want to represent our actor.
61+
static SILValue lookThroughInsts(SILValue value);
62+
5863
public:
5964
ActorInstance() : ActorInstance(SILValue(), Kind::Value) {}
6065

6166
static ActorInstance getForValue(SILValue value) {
67+
value = lookThroughInsts(value);
6268
return ActorInstance(value, Kind::Value);
6369
}
6470

@@ -411,6 +417,10 @@ class SILDynamicMergedIsolationInfo {
411417
void printForDiagnostics(llvm::raw_ostream &os) const {
412418
innerInfo.printForDiagnostics(os);
413419
}
420+
421+
SWIFT_DEBUG_DUMPER(dumpForDiagnostics()) {
422+
innerInfo.dumpForDiagnostics();
423+
}
414424
};
415425

416426
} // namespace swift

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
393393
// TODO: We really should be doing this based off of an Operand. Then
394394
// we would get the SILValue() for the first element. Today this can
395395
// only mess up isolation history.
396+
396397
return SILIsolationInfo::getActorInstanceIsolated(
397398
SILValue(), isolatedOp->get(), nomDecl);
398399
}
@@ -1099,6 +1100,30 @@ bool SILIsolationInfo::isNonSendableType(SILType type, SILFunction *fn) {
10991100
return !type.isSendable(fn);
11001101
}
11011102

1103+
//===----------------------------------------------------------------------===//
1104+
// MARK: ActorInstance
1105+
//===----------------------------------------------------------------------===//
1106+
1107+
SILValue ActorInstance::lookThroughInsts(SILValue value) {
1108+
if (!value)
1109+
return value;
1110+
1111+
while (auto *svi = dyn_cast<SingleValueInstruction>(value)) {
1112+
if (isa<EndInitLetRefInst>(svi) || isa<CopyValueInst>(svi) ||
1113+
isa<MoveValueInst>(svi) || isa<ExplicitCopyValueInst>(svi) ||
1114+
isa<BeginBorrowInst>(svi) ||
1115+
isa<CopyableToMoveOnlyWrapperValueInst>(svi) ||
1116+
isa<MoveOnlyWrapperToCopyableValueInst>(svi)) {
1117+
value = lookThroughInsts(svi->getOperand(0));
1118+
continue;
1119+
}
1120+
1121+
break;
1122+
}
1123+
1124+
return value;
1125+
}
1126+
11021127
//===----------------------------------------------------------------------===//
11031128
// MARK: SILDynamicMergedIsolationInfo
11041129
//===----------------------------------------------------------------------===//

lib/SILOptimizer/Utils/VariableNameUtils.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,8 @@ SILValue VariableNameInferrer::findDebugInfoProvidingValueHelper(
625625
isa<CopyableToMoveOnlyWrapperAddrInst>(searchValue) ||
626626
isa<MoveOnlyWrapperToCopyableAddrInst>(searchValue) ||
627627
isa<MoveOnlyWrapperToCopyableValueInst>(searchValue) ||
628-
isa<CopyableToMoveOnlyWrapperValueInst>(searchValue)) {
628+
isa<CopyableToMoveOnlyWrapperValueInst>(searchValue) ||
629+
isa<EndInitLetRefInst>(searchValue)) {
629630
searchValue = cast<SingleValueInstruction>(searchValue)->getOperand(0);
630631
continue;
631632
}

test/Concurrency/async_task_locals_basic_warnings_bug_isolation.swift

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,10 @@
22
// RUN: %target-swift-frontend -plugin-path %swift-plugin-dir -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking
33

44
// RUN: %target-swift-frontend -I %t -plugin-path %swift-plugin-dir -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify
5-
// RUN: %target-swift-frontend -I %t -plugin-path %swift-plugin-dir -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation
65

76
// REQUIRES: concurrency
87
// REQUIRES: asserts
98

10-
// FIXME: rdar://125078448 is resolved
11-
// XFAIL: *
12-
139
actor Test {
1410

1511
@TaskLocal static var local: Int?

test/Concurrency/silisolationinfo_inference.sil

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ sil @constructNonSendableKlassIndirect : $@convention(thin) () -> @out NonSendab
3232
sil @constructNonSendableKlassIndirectAsync : $@convention(thin) @async () -> @out NonSendableKlass
3333
sil @useUnmanagedNonSendableKlass : $@convention(thin) (@guaranteed @sil_unmanaged NonSendableKlass) -> ()
3434

35+
sil @useActor : $@convention(thin) (@sil_isolated @guaranteed MyActor) -> @owned NonSendableKlass
3536

3637
///////////////////////
3738
// MARK: Basic Tests //
@@ -699,3 +700,24 @@ bb7:
699700
%9999 = tuple ()
700701
return %9999 : $()
701702
}
703+
704+
// CHECK-LABEL: begin running test 1 of 1 on lookthrough_test: sil-isolation-info-inference with: @trace[0]
705+
// CHECK-NEXT: Input Value: %7 = apply %6(%5) : $@convention(thin) (@sil_isolated @guaranteed MyActor) -> @owned NonSendableKlass
706+
// CHECK-NEXT: Isolation: 'argument'-isolated
707+
// CHECK-NEXT: end running test 1 of 1 on lookthrough_test: sil-isolation-info-inference with: @trace[0]
708+
sil [ossa] @lookthrough_test : $@convention(thin) (@owned MyActor) -> @owned NonSendableKlass {
709+
bb0(%0 : @owned $MyActor):
710+
specify_test "sil-isolation-info-inference @trace[0]"
711+
debug_value %0 : $MyActor, let, name "argument"
712+
%1 = end_init_let_ref %0 : $MyActor
713+
%1a = copy_value %1 : $MyActor
714+
%1b = move_value %1a : $MyActor
715+
%1c = begin_borrow %1b : $MyActor
716+
%2 = function_ref @useActor : $@convention(thin) (@sil_isolated @guaranteed MyActor) -> @owned NonSendableKlass
717+
%3 = apply %2(%1c) : $@convention(thin) (@sil_isolated @guaranteed MyActor) -> @owned NonSendableKlass
718+
debug_value [trace] %3 : $NonSendableKlass
719+
end_borrow %1c : $MyActor
720+
destroy_value %1 : $MyActor
721+
destroy_value %1b : $MyActor
722+
return %3 : $NonSendableKlass
723+
}

test/Concurrency/transfernonsendable_initializers.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,13 @@ actor ActorWithSynchronousNonIsolatedInit {
5050
}
5151
}
5252

53+
init(ns: NonSendableKlass) async {
54+
self.k = NonSendableKlass()
55+
self.isolatedHelper(ns)
56+
}
57+
5358
nonisolated func helper(_ newK: NonSendableKlass) {}
59+
func isolatedHelper(_ newK: NonSendableKlass) {}
5460
}
5561

5662
func initActorWithSyncNonIsolatedInit() {

test/Concurrency/transfernonsendable_ownership.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,19 @@ func testInOutError(_ x: inout Klass) async {
105105
// expected-note @-1 {{sending global actor 'CustomActor'-isolated 'x' to main actor-isolated global function 'transferToMain'}}
106106
_ = consume x // expected-note {{consumed here}}
107107
} // expected-note {{used here}}
108+
109+
actor ActorTestCase {
110+
let k = Klass()
111+
112+
// TODO: This crashes the compiler since we attempt to hop_to_executor to the
113+
// value that is materialized onto disk.
114+
//
115+
// consuming func test() async {
116+
// await transferToMain(k)
117+
// }
118+
119+
borrowing func test2() async {
120+
await transferToMain(k) // expected-warning {{sending 'self.k' risks causing data races}}
121+
// expected-note @-1 {{sending 'self'-isolated 'self.k' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and 'self'-isolated uses}}
122+
}
123+
}

test/SILOptimizer/variable_name_inference.sil

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -866,10 +866,10 @@ bb0:
866866
}
867867

868868
// CHECK-LABEL: begin running test 1 of 1 on begin_borrow_var_decl_3: variable-name-inference with: @trace[0]
869-
// CHECK-LABEL: Input Value: %1 = apply %0() : $@convention(thin) () -> @owned Klass
870-
// CHECK-LABEL: Name: 'unknown'
871-
// CHECK-LABEL: Root: 'unknown'
872-
// CHECK-LABEL: end running test 1 of 1 on begin_borrow_var_decl_3: variable-name-inference with: @trace[0]
869+
// CHECK: Input Value: %1 = apply %0() : $@convention(thin) () -> @owned Klass
870+
// CHECK: Name: 'unknown'
871+
// CHECK: Root: 'unknown'
872+
// CHECK: end running test 1 of 1 on begin_borrow_var_decl_3: variable-name-inference with: @trace[0]
873873
sil [ossa] @begin_borrow_var_decl_3 : $@convention(thin) () -> () {
874874
bb0:
875875
specify_test "variable-name-inference @trace[0]"
@@ -883,3 +883,17 @@ bb0:
883883
%9999 = tuple ()
884884
return %9999 : $()
885885
}
886+
887+
// CHECK-LABEL: begin running test 1 of 1 on infer_through_end_init_let_ref: variable-name-inference with: @trace[0]
888+
// CHECK: Input Value: %2 = end_init_let_ref %0 : $Klass
889+
// CHECK: Name: 'self'
890+
// CHECK: Root: %0 = argument of bb0 : $Klass
891+
// CHECK: end running test 1 of 1 on infer_through_end_init_let_ref: variable-name-inference with: @trace[0]
892+
sil [ossa] @infer_through_end_init_let_ref : $@convention(thin) (@owned Klass) -> @owned Klass {
893+
bb0(%0 : @owned $Klass):
894+
specify_test "variable-name-inference @trace[0]"
895+
debug_value %0 : $Klass, let, name "self", argno 2
896+
%1 = end_init_let_ref %0 : $Klass
897+
debug_value [trace] %1 : $Klass
898+
return %1 : $Klass
899+
}

0 commit comments

Comments
 (0)