Skip to content

Commit 5ac839b

Browse files
hborlahborla
committed
[SILGen] Only hop to the callee isolation domain once when evaluating isolated
default arguments.
1 parent 0e71623 commit 5ac839b

File tree

3 files changed

+146
-23
lines changed

3 files changed

+146
-23
lines changed

‎lib/SILGen/SILGenApply.cpp

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2718,6 +2718,28 @@ class DelayedArgument {
27182718
return LV().Loc;
27192719
}
27202720

2721+
bool isDefaultArg() const {
2722+
return Kind == DefaultArgument;
2723+
}
2724+
2725+
SILLocation getDefaultArgLoc() const {
2726+
assert(isDefaultArg());
2727+
auto storage = Value.get<DefaultArgumentStorage>(Kind);
2728+
return storage.loc;
2729+
}
2730+
2731+
llvm::Optional<ActorIsolation> getIsolation() const {
2732+
if (!isDefaultArg())
2733+
return llvm::None;
2734+
2735+
auto storage = Value.get<DefaultArgumentStorage>(Kind);
2736+
if (!storage.implicitlyAsync)
2737+
return llvm::None;
2738+
2739+
auto callee = storage.defaultArgsOwner.getDecl();
2740+
return getActorIsolation(callee);
2741+
}
2742+
27212743
void emit(SILGenFunction &SGF, SmallVectorImpl<ManagedValue> &args,
27222744
size_t &argIndex) {
27232745
switch (Kind) {
@@ -2943,6 +2965,31 @@ static void emitDelayedArguments(SILGenFunction &SGF,
29432965
MutableArrayRef<SmallVector<ManagedValue, 4>> args) {
29442966
assert(!delayedArgs.empty());
29452967

2968+
// If any of the delayed arguments are isolated default arguments,
2969+
// argument evaluation happens in the following order:
2970+
//
2971+
// 1. Left-to-right evalution of explicit r-value arguments
2972+
// 2. Left-to-right evaluation of formal access arguments
2973+
// 3. Hop to the callee's isolation domain
2974+
// 4. Left-to-right evaluation of default arguments
2975+
2976+
// So, if any delayed arguments are isolated, all default arguments
2977+
// are collected during the first pass over the delayed arguments,
2978+
// and emitted separately after a hop to the callee's isolation domain.
2979+
2980+
llvm::Optional<ActorIsolation> defaultArgIsolation;
2981+
for (auto &arg : delayedArgs) {
2982+
if (auto isolation = arg.getIsolation()) {
2983+
defaultArgIsolation = isolation;
2984+
break;
2985+
}
2986+
}
2987+
2988+
SmallVector<std::tuple<
2989+
/*delayedArgIt*/decltype(delayedArgs)::iterator,
2990+
/*siteArgsIt*/decltype(args)::iterator,
2991+
/*index*/size_t>, 2> isolatedArgs;
2992+
29462993
SmallVector<std::pair<SILValue, SILLocation>, 4> emittedInoutArgs;
29472994
auto delayedNext = delayedArgs.begin();
29482995

@@ -2951,7 +2998,8 @@ static void emitDelayedArguments(SILGenFunction &SGF,
29512998
// wherever there's a delayed argument to insert.
29522999
//
29533000
// Note that this also begins the formal accesses in evaluation order.
2954-
for (auto &siteArgs : args) {
3001+
for (auto argsIt = args.begin(); argsIt != args.end(); ++argsIt) {
3002+
auto &siteArgs = *argsIt;
29553003
// NB: siteArgs.size() may change during iteration
29563004
for (size_t i = 0; i < siteArgs.size(); ) {
29573005
auto &siteArg = siteArgs[i];
@@ -2964,6 +3012,15 @@ static void emitDelayedArguments(SILGenFunction &SGF,
29643012
assert(delayedNext != delayedArgs.end());
29653013
auto &delayedArg = *delayedNext;
29663014

3015+
if (defaultArgIsolation && delayedArg.isDefaultArg()) {
3016+
isolatedArgs.push_back(std::make_tuple(delayedNext, argsIt, i));
3017+
if (++delayedNext == delayedArgs.end()) {
3018+
goto done;
3019+
} else {
3020+
continue;
3021+
}
3022+
}
3023+
29673024
// Emit the delayed argument and replace it in the arguments array.
29683025
delayedArg.emit(SGF, siteArgs, i);
29693026

@@ -2984,6 +3041,45 @@ static void emitDelayedArguments(SILGenFunction &SGF,
29843041

29853042
done:
29863043

3044+
if (defaultArgIsolation) {
3045+
assert(SGF.F.isAsync());
3046+
assert(!isolatedArgs.empty());
3047+
3048+
auto &firstArg = *std::get<0>(isolatedArgs[0]);
3049+
auto loc = firstArg.getDefaultArgLoc();
3050+
3051+
SILValue executor;
3052+
switch (*defaultArgIsolation) {
3053+
case ActorIsolation::GlobalActor:
3054+
case ActorIsolation::GlobalActorUnsafe:
3055+
executor = SGF.emitLoadGlobalActorExecutor(
3056+
defaultArgIsolation->getGlobalActor());
3057+
break;
3058+
3059+
case ActorIsolation::ActorInstance:
3060+
llvm_unreachable("default arg cannot be actor instance isolated");
3061+
3062+
case ActorIsolation::Unspecified:
3063+
case ActorIsolation::Nonisolated:
3064+
case ActorIsolation::NonisolatedUnsafe:
3065+
llvm_unreachable("Not isolated");
3066+
}
3067+
3068+
// Hop to the target isolation domain once to evaluate all
3069+
// default arguments.
3070+
SGF.emitHopToTargetExecutor(loc, executor);
3071+
3072+
size_t argsEmitted = 0;
3073+
for (auto &isolatedArg : isolatedArgs) {
3074+
auto &delayedArg = *std::get<0>(isolatedArg);
3075+
auto &siteArgs = *std::get<1>(isolatedArg);
3076+
auto argIndex = std::get<2>(isolatedArg) + argsEmitted;
3077+
auto origIndex = argIndex;
3078+
delayedArg.emit(SGF, siteArgs, argIndex);
3079+
argsEmitted += (argIndex - origIndex);
3080+
}
3081+
}
3082+
29873083
// Check to see if we have multiple inout arguments which obviously
29883084
// alias. Note that we could do this in a later SILDiagnostics pass
29893085
// as well: this would be stronger (more equivalences exposed) but

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2647,24 +2647,8 @@ SILGenFunction::emitApplyOfDefaultArgGenerator(SILLocation loc,
26472647
emitCaptures(loc, generator, CaptureEmission::ImmediateApplication,
26482648
captures);
26492649

2650-
// The default argument might require the callee's isolation. If so,
2651-
// make sure to emit an actor hop.
2652-
//
2653-
// FIXME: Instead of hopping back and forth for each individual isolated
2654-
// default argument, we should emit one hop for all default arguments if
2655-
// any of them are isolated, and immediately enter the function after.
2656-
llvm::Optional<ActorIsolation> implicitActorHopTarget = llvm::None;
2657-
if (implicitlyAsync) {
2658-
auto *param = getParameterAt(defaultArgsOwner.getDecl(), destIndex);
2659-
auto isolation = param->getInitializerIsolation();
2660-
if (isolation.isActorIsolated()) {
2661-
implicitActorHopTarget = isolation;
2662-
}
2663-
}
2664-
26652650
return emitApply(std::move(resultPtr), std::move(argScope), loc, fnRef, subs,
2666-
captures, calleeTypeInfo, ApplyOptions(), C,
2667-
implicitActorHopTarget);
2651+
captures, calleeTypeInfo, ApplyOptions(), C, llvm::None);
26682652
}
26692653

26702654
RValue SILGenFunction::emitApplyOfStoredPropertyInitializer(

‎test/Concurrency/isolated_default_argument_eval.swift

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,62 @@ func requiresMainActor() -> Int { 0 }
99
@MainActor
1010
func mainActorDefaultArg(value: Int = requiresMainActor()) {}
1111

12+
@MainActor
13+
func mainActorMultiDefaultArg(x: Int = requiresMainActor(),
14+
y: Int = 0,
15+
tuple: (Int, Int) = (requiresMainActor(), 2),
16+
z: Int = 0) {}
17+
1218
// CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval15mainActorCalleryyF
1319
@MainActor func mainActorCaller() {
1420
mainActorDefaultArg()
21+
mainActorMultiDefaultArg()
1522
}
1623

1724
// CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval22nonisolatedAsyncCalleryyYaF
1825
func nonisolatedAsyncCaller() async {
1926
// CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
20-
// CHECK: [[GETARG:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueySi_tFfA_
2127
// CHECK: hop_to_executor {{.*}} : $MainActor
22-
// CHECK-NEXT: apply [[GETARG]]()
23-
// CHECK-NEXT: end_borrow {{.*}} : $MainActor
24-
// CHECK-NEXT: destroy_value {{.*}} : $MainActor
25-
// CHECK-NEXT: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
28+
// CHECK: [[GET_VALUE:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueySi_tFfA_
29+
// CHECK-NEXT: apply [[GET_VALUE]]()
30+
// CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
2631
await mainActorDefaultArg()
32+
33+
// CHECK: hop_to_executor {{.*}} : $MainActor
34+
// CHECK: [[GET_X:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval24mainActorMultiDefaultArg1x1y5tuple1zySi_S2i_SitSitFfA_
35+
// CHECK-NEXT: apply [[GET_X]]()
36+
// CHECK-NOT: hop_to_executor
37+
// CHECK: [[GET_Y:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval24mainActorMultiDefaultArg1x1y5tuple1zySi_S2i_SitSitFfA0_
38+
// CHECK-NEXT: apply [[GET_Y]]()
39+
// CHECK-NOT: hop_to_executor
40+
// CHECK: [[GET_TUPLE:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval24mainActorMultiDefaultArg1x1y5tuple1zySi_S2i_SitSitFfA1_
41+
// CHECK-NEXT: apply [[GET_TUPLE]]()
42+
// CHECK-NOT: hop_to_executor
43+
// CHECK: [[GET_Z:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval24mainActorMultiDefaultArg1x1y5tuple1zySi_S2i_SitSitFfA2_
44+
// CHECK-NEXT: apply [[GET_Z]]()
45+
// CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
46+
await mainActorMultiDefaultArg()
47+
}
48+
49+
@MainActor
50+
func isolatedDefaultInoutMix(x: inout Int, y: Int, z: Int = requiresMainActor()) {}
51+
52+
var argValue: Int { 0 }
53+
54+
// CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval20passInoutWithDefaultyyYaF
55+
func passInoutWithDefault() async {
56+
// CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
57+
58+
var x = 0
59+
60+
// CHECK: [[GET_ARG_VALUE:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval8argValueSivg
61+
// CHECK-NEXT: [[ARG_VALUE:%[0-9]+]] = apply [[GET_ARG_VALUE]]()
62+
// CHECK-NEXT: [[INOUT_X:%[0-9]+]] = begin_access [modify]
63+
// CHECK: hop_to_executor {{.*}} : $MainActor
64+
// CHECK: [[GET_Z:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval0A15DefaultInoutMix1x1y1zySiz_S2itFfA1_
65+
// CHECK-NEXT: [[Z:%[0-9]+]] = apply [[GET_Z]]()
66+
// CHECK: [[FN:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval0A15DefaultInoutMix1x1y1zySiz_S2itF : $@convention(thin) (@inout Int, Int, Int) -> ()
67+
// CHECK: apply [[FN]]([[INOUT_X]], [[ARG_VALUE]], [[Z]])
68+
// CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
69+
await isolatedDefaultInoutMix(x: &x, y: argValue)
2770
}

0 commit comments

Comments
 (0)