Skip to content

Commit 0051966

Browse files
gottesmmgottesmm
authored
Merge pull request swiftlang#76076 from gottesmm/rdar134623227
[region-isolation] Treat sending indirect_results as disconnected even if it is a return value of an actor isolated function.
2 parents e6f9642 + 3e27bfc commit 0051966

File tree

8 files changed

+167
-22
lines changed

8 files changed

+167
-22
lines changed

‎include/swift/SIL/ApplySite.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,10 @@ class ApplySite {
534534
/// result argument to the apply site.
535535
bool isIndirectResultOperand(const Operand &op) const;
536536

537+
/// Returns true if \p op is an operand that is passed as an indirect error
538+
/// result.
539+
bool isIndirectErrorResultOperand(const Operand &op) const;
540+
537541
ApplyOptions getApplyOptions() const {
538542
switch (ApplySiteKind(getInstruction()->getKind())) {
539543
case ApplySiteKind::ApplyInst:
@@ -597,6 +601,14 @@ class ApplySite {
597601
return getSubstCalleeConv().getParamInfoForSILArg(calleeArgIndex);
598602
}
599603

604+
bool isSending(const Operand &oper) const {
605+
if (isIndirectErrorResultOperand(oper))
606+
return false;
607+
if (isIndirectResultOperand(oper))
608+
return getSubstCalleeType()->hasSendingResult();
609+
return getArgumentParameterInfo(oper).hasOption(SILParameterInfo::Sending);
610+
}
611+
600612
static ApplySite getFromOpaqueValue(void *p) { return ApplySite(p); }
601613

602614
friend bool operator==(ApplySite lhs, ApplySite rhs) {
@@ -955,6 +967,13 @@ inline bool ApplySite::isIndirectResultOperand(const Operand &op) const {
955967
return fas.isIndirectResultOperand(op);
956968
}
957969

970+
inline bool ApplySite::isIndirectErrorResultOperand(const Operand &op) const {
971+
auto fas = asFullApplySite();
972+
if (!fas)
973+
return false;
974+
return fas.isIndirectErrorResultOperand(op);
975+
}
976+
958977
} // namespace swift
959978

960979
#endif

‎include/swift/SIL/SILArgument.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,9 +424,7 @@ class SILFunctionArgument : public SILArgument {
424424
sharedUInt32().SILFunctionArgument.lifetimeAnnotation = newValue;
425425
}
426426

427-
bool isSending() const {
428-
return getKnownParameterInfo().hasOption(SILParameterInfo::Sending);
429-
}
427+
bool isSending() const;
430428

431429
Lifetime getLifetime() const {
432430
return getType()

‎lib/SIL/IR/SILArgument.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,3 +433,11 @@ bool SILFunctionArgument::isSelf() const {
433433
return getFunction()->hasSelfParam() &&
434434
getParent()->getArguments().back() == this;
435435
}
436+
437+
bool SILFunctionArgument::isSending() const {
438+
if (isIndirectErrorResult())
439+
return false;
440+
if (isIndirectResult())
441+
return getFunction()->getLoweredFunctionType()->hasSendingResult();
442+
return getKnownParameterInfo().hasOption(SILParameterInfo::Sending);
443+
}

‎lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,7 +1123,9 @@ void InferredCallerArgumentTypeInfo::init(const Operand *op) {
11231123

11241124
namespace {
11251125

1126-
constexpr const char *SEP_STR = "╾──────────────────────────────╼\n";
1126+
constexpr StringLiteral SEP_STR = "╾──────────────────────────────╼\n";
1127+
constexpr StringLiteral PER_FUNCTION_SEP_STR =
1128+
"╾++++++++++++++++++++++++++++++╼\n";
11271129

11281130
} // namespace
11291131

@@ -1434,9 +1436,11 @@ class PartitionOpTranslator {
14341436

14351437
void gatherFlowInsensitiveInformationBeforeDataflow() {
14361438
REGIONBASEDISOLATION_LOG(llvm::dbgs()
1437-
<< ">>> Performing pre-dataflow scan to gather "
1439+
<< SEP_STR
1440+
<< "Performing pre-dataflow scan to gather "
14381441
"flow insensitive information "
1439-
<< function->getName() << ":\n");
1442+
<< function->getName() << ":\n"
1443+
<< SEP_STR);
14401444

14411445
for (auto &block : *function) {
14421446
for (auto &inst : block) {
@@ -1488,6 +1492,18 @@ class PartitionOpTranslator {
14881492
: function(function), functionArgPartition(), builder(),
14891493
partialApplyReachabilityDataflow(function, pofi), valueMap(valueMap) {
14901494
builder.translator = this;
1495+
1496+
REGIONBASEDISOLATION_LOG(
1497+
llvm::dbgs()
1498+
<< PER_FUNCTION_SEP_STR
1499+
<< "Beginning processing: " << function->getName() << '\n'
1500+
<< "Demangled: "
1501+
<< Demangle::demangleSymbolAsString(
1502+
function->getName(),
1503+
Demangle::DemangleOptions::SimplifiedUIDemangleOptions())
1504+
<< '\n'
1505+
<< PER_FUNCTION_SEP_STR);
1506+
14911507
gatherFlowInsensitiveInformationBeforeDataflow();
14921508

14931509
REGIONBASEDISOLATION_LOG(llvm::dbgs() << "Initializing Function Args:\n");
@@ -1960,6 +1976,7 @@ class PartitionOpTranslator {
19601976
// For non-self parameters, gather all of the transferring parameters and
19611977
// gather our non-transferring parameters.
19621978
SmallVector<Operand *, 8> nonTransferringParameters;
1979+
SmallVector<Operand *, 8> sendingIndirectResults;
19631980
if (fas.getNumArguments()) {
19641981
// NOTE: We want to process indirect parameters as if they are
19651982
// parameters... so we process them in nonTransferringParameters.
@@ -1968,12 +1985,16 @@ class PartitionOpTranslator {
19681985
if (fas.isCalleeOperand(op))
19691986
continue;
19701987

1971-
if (!fas.getArgumentConvention(op).isIndirectOutParameter() &&
1972-
fas.getArgumentParameterInfo(op).hasOption(
1973-
SILParameterInfo::Sending)) {
1988+
if (fas.isSending(op)) {
1989+
if (fas.isIndirectResultOperand(op)) {
1990+
sendingIndirectResults.push_back(&op);
1991+
continue;
1992+
}
1993+
19741994
if (auto value = tryToTrackValue(op.get())) {
19751995
builder.addRequire(value->getRepresentative().getValue());
19761996
builder.addTransfer(value->getRepresentative().getValue(), &op);
1997+
continue;
19771998
}
19781999
} else {
19792000
nonTransferringParameters.push_back(&op);
@@ -2025,11 +2046,20 @@ class PartitionOpTranslator {
20252046
// override isolation, then perform assign fresh.
20262047
ArrayRef<SILValue> empty;
20272048
translateSILMultiAssign(empty, nonTransferringParameters, {});
2049+
2050+
// Sending direct results.
20282051
for (SILValue result : applyResults) {
20292052
if (auto value = tryToTrackValue(result)) {
20302053
builder.addAssignFresh(value->getRepresentative().getValue());
20312054
}
20322055
}
2056+
2057+
// Sending indirect results.
2058+
for (Operand *op : sendingIndirectResults) {
2059+
if (auto value = tryToTrackValue(op->get())) {
2060+
builder.addAssignFresh(value->getRepresentative().getValue());
2061+
}
2062+
}
20332063
}
20342064

20352065
void translateSILApply(SILInstruction *inst) {
@@ -2057,8 +2087,9 @@ class PartitionOpTranslator {
20572087
return translateIsolationCrossingSILApply(cast);
20582088
if (auto cast = dyn_cast<BeginApplyInst>(inst))
20592089
return translateIsolationCrossingSILApply(cast);
2060-
if (auto cast = dyn_cast<TryApplyInst>(inst))
2090+
if (auto cast = dyn_cast<TryApplyInst>(inst)) {
20612091
return translateIsolationCrossingSILApply(cast);
2092+
}
20622093

20632094
llvm_unreachable("Only ApplyInst, BeginApplyInst, and TryApplyInst should "
20642095
"cross isolation domains");

‎lib/SILOptimizer/Utils/SILIsolationInfo.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -832,11 +832,19 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
832832

833833
auto *fArg = cast<SILFunctionArgument>(arg);
834834

835-
// Transferring is always disconnected.
835+
// Sending is always disconnected.
836+
if (fArg->isSending())
837+
return SILIsolationInfo::getDisconnected(false /*nonisolated(unsafe)*/);
838+
839+
// If we have a closure capture that is not an indirect result or indirect
840+
// result error, we want to treat it as sending so that we properly handle
841+
// async lets.
842+
//
843+
// This pattern should only come up with async lets. See comment in
844+
// isTransferrableFunctionArgument.
836845
if (!fArg->isIndirectResult() && !fArg->isIndirectErrorResult() &&
837-
((fArg->isClosureCapture() &&
838-
fArg->getFunction()->getLoweredFunctionType()->isSendable()) ||
839-
fArg->isSending()))
846+
fArg->isClosureCapture() &&
847+
fArg->getFunction()->getLoweredFunctionType()->isSendable())
840848
return SILIsolationInfo::getDisconnected(false /*nonisolated(unsafe)*/);
841849

842850
// Before we do anything further, see if we have an isolated parameter. This

‎test/Concurrency/sending_continuation.swift

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,10 @@ func withCheckedContinuation_3() async {
6060
// expected-note @-2 {{'x' used after being passed as a 'sending' parameter}}
6161
useValue(x) // expected-note {{access can happen concurrently}}
6262
}
63+
64+
// Since x is returned as sending from withCheckedContinuation, we can send it
65+
// further.
6366
await useValueAsync(x)
64-
// expected-error @-1 {{sending 'x' risks causing data races}}
65-
// expected-note @-2 {{sending main actor-isolated 'x' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and main actor-isolated uses}}
6667
}
6768

6869
func withCheckedContinuation_3a() async {
@@ -88,9 +89,10 @@ func withCheckedContinuation_4() async {
8889
// expected-note @-2 {{main actor-isolated 'y' is passed as a 'sending' parameter}}
8990
useValue(y)
9091
}
92+
93+
// Since withCheckedContinuation returns value as sending, we can call
94+
// useValueAsync since it is disconnected.
9195
await useValueAsync(x)
92-
// expected-error @-1 {{sending 'x' risks causing data races}}
93-
// expected-note @-2 {{sending main actor-isolated 'x' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and main actor-isolated uses}}
9496
}
9597

9698
func withCheckedContinuation_4a() async {
@@ -189,9 +191,10 @@ func withUnsafeContinuation_3() async {
189191
// expected-note @-2 {{'x' used after being passed as a 'sending' parameter}}
190192
useValue(x) // expected-note {{access can happen concurrently}}
191193
}
194+
195+
// withUnsafeContinuation returns x as sending, so we can pass it to a
196+
// nonisolated function.
192197
await useValueAsync(x)
193-
// expected-error @-1 {{sending 'x' risks causing data races}}
194-
// expected-note @-2 {{sending main actor-isolated 'x' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and main actor-isolated uses}}
195198
}
196199

197200
func withUnsafeContinuation_3a() async {
@@ -215,9 +218,10 @@ func withUnsafeContinuation_4() async {
215218
// expected-note @-2 {{main actor-isolated 'y' is passed as a 'sending' parameter}}
216219
useValue(y)
217220
}
221+
222+
// Since withUnsafeContinuation returns x as sending, we can use it in a
223+
// nonisolated function.
218224
await useValueAsync(x)
219-
// expected-error @-1 {{sending 'x' risks causing data races}}
220-
// expected-note @-2 {{sending main actor-isolated 'x' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and main actor-isolated uses}}
221225
}
222226

223227
func withUnsafeContinuation_4a() async {

‎test/Concurrency/silisolationinfo_inference.sil

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -819,3 +819,60 @@ bb0(%0 : @guaranteed $MyActor):
819819
%9999 = tuple ()
820820
return %9999 : $()
821821
}
822+
823+
// CHECK-LABEL: begin running test 1 of 1 on sending_indirect_result_isnt_actor_isolated: sil_isolation_info_inference with: @trace[0]
824+
// CHECK-NEXT: Input Value: %0 = argument of bb0 : $*Optional<τ_0_0> // user: %2
825+
// CHECK-NEXT: Isolation: disconnected
826+
// CHECK-NEXT: end running test 1 of 1 on sending_indirect_result_isnt_actor_isolated: sil_isolation_info_inference with: @trace[0]
827+
sil [ossa] @sending_indirect_result_isnt_actor_isolated : $@convention(thin) @async <τ_0_0> (@sil_isolated @guaranteed MyActor) -> (@sil_sending @out Optional<τ_0_0>) {
828+
bb0(%0 : $*Optional<τ_0_0>, %1 : @guaranteed $MyActor):
829+
specify_test "sil_isolation_info_inference @trace[0]"
830+
debug_value [trace] %0 : $*Optional<τ_0_0>
831+
inject_enum_addr %0 : $*Optional<τ_0_0>, #Optional.none!enumelt
832+
%9999 = tuple ()
833+
return %9999 : $()
834+
}
835+
836+
sil @myactor_sending_result_sync : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> @sil_sending @out τ_0_0
837+
838+
// CHECK: begin running test 1 of 1 on sending_indirect_result_isnt_actor_isolated_apply: sil_isolation_info_inference with: @trace[0]
839+
// CHECK-NEXT: Input Value: %3 = alloc_stack $T // users: %6, %5, %4
840+
// CHECK-NEXT: Isolation: disconnected
841+
// CHECK-NEXT: end running test 1 of 1 on sending_indirect_result_isnt_actor_isolated_apply: sil_isolation_info_inference with: @trace[0]
842+
sil [ossa] @sending_indirect_result_isnt_actor_isolated_apply : $@convention(method) @async <T> (@sil_isolated @guaranteed MyActor) -> @sil_sending @out T {
843+
bb0(%0 : $*T, %1 : @guaranteed $MyActor):
844+
specify_test "sil_isolation_info_inference @trace[0]"
845+
%4 = function_ref @myactor_sending_result_sync : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> @sil_sending @out τ_0_0
846+
%2 = alloc_stack $T
847+
debug_value [trace] %2 : $*T
848+
%5 = apply %4<T>(%2, %1) : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> @sil_sending @out τ_0_0
849+
copy_addr [take] %2 to [init] %0 : $*T
850+
dealloc_stack %2 : $*T
851+
%6 = tuple ()
852+
return %6 : $()
853+
}
854+
855+
sil @myactor_sending_result_sync_throws : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> (@sil_sending @out τ_0_0, @error any Error)
856+
857+
// CHECK-LABEL: begin running test 1 of 1 on sending_indirect_result_isnt_actor_isolated_try_apply: sil_isolation_info_inference with: @trace[0]
858+
// CHECK-NEXT: Input Value: %3 = alloc_stack $T
859+
// CHECK-NEXT: Isolation: disconnected
860+
// CHECK-NEXT: end running test 1 of 1 on sending_indirect_result_isnt_actor_isolated_try_apply: sil_isolation_info_inference with: @trace[0]
861+
sil [ossa] @sending_indirect_result_isnt_actor_isolated_try_apply : $@convention(method) @async <T> (@sil_isolated @guaranteed MyActor) -> (@sil_sending @out T, @error any Error) {
862+
bb0(%0 : $*T, %1 : @guaranteed $MyActor):
863+
specify_test "sil_isolation_info_inference @trace[0]"
864+
%5 = function_ref @myactor_sending_result_sync_throws : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> (@sil_sending @out τ_0_0, @error any Error)
865+
%2 = alloc_stack $*T
866+
try_apply %5<T>(%2, %1) : $@convention(method) <τ_0_0> (@sil_isolated @guaranteed MyActor) -> (@sil_sending @out τ_0_0, @error any Error), normal bb1, error bb2
867+
868+
bb1(%7 : $()):
869+
copy_addr [take] %2 to [init] %0 : $*T
870+
debug_value [trace] %2 : $*T
871+
dealloc_stack %2 : $*T
872+
%8 = tuple ()
873+
return %8 : $()
874+
875+
bb2(%10 : @owned $any Error):
876+
dealloc_stack %2 : $*T
877+
throw %10 : $any Error
878+
}

‎test/Concurrency/transfernonsendable_sending_results.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ class GenericNonSendableKlass<T> {
9090

9191
func sendParameter<T>(_ t: sending T) {}
9292

93+
actor MyActor {
94+
private var ns = NonSendableKlass()
95+
}
96+
9397
/////////////////
9498
// MARK: Tests //
9599
/////////////////
@@ -295,3 +299,19 @@ func indirectSendingOptionalClassField<T>(_ t: GenericNonSendableKlass<T>) -> se
295299
// expected-note @-1 {{returning a task-isolated 'Optional<T>' value risks causing races since the caller assumes the value can be safely sent to other isolation domains}}
296300
// expected-note @-2 {{'Optional<T>' is a non-Sendable type}}
297301
}
302+
303+
func useBlock<T>(block: () throws -> T) throws -> sending T {
304+
fatalError()
305+
}
306+
307+
extension MyActor {
308+
// This shouldn't emit any errors. We used to error on returning result.
309+
public func withContext<T>(_ block: sending () throws -> T) async throws -> sending T {
310+
let value: T = try useBlock {
311+
_ = ns
312+
return try block()
313+
}
314+
315+
return value
316+
}
317+
}

0 commit comments

Comments
 (0)