Skip to content

Commit a0911ef

Browse files
gottesmmgottesmm
committed
[concurrency] Make optimize hop to executor more conservative for 6.2 around caller isolation inheriting functions.
Specifically for 6.2, we are making optimize hop to executor more conservative around caller isolation inheriting functions. This means that we are: 1. No longer treating calls to caller isolation inheriting functions as having a hop in their prologue. In terms of this pass, it means that when determining dead hop to executors, we no longer think that a caller isolation inheriting function means that an earlier hop to executor is not required. 2. Treating returns from caller isolation inheriting callees as requiring a hop. The reason why we are doing this is that we can no longer assume that our caller will hop after we return. Post 6.2, there are three main changes we are going to make: * Forward Dataflow Caller isolation inheriting functions will no longer be treated as suspension points meaning that we will be able to propagate hops over them and can assume that we know the actor that we are on when we enter the function. Practically this means that trees of calls that involve just nonisolated(nonsending) async functions will avoid /all/ hop to executor calls since we will be able to eliminate all of them since the dataflow will just propagate forward from the entrance that we are already on the actor. * Backwards Dataflow A caller isolation inheriting call site will still cause preceding hop_to_executor functions to be live. This is because we need to ensure that we are on the caller isolation inheriting actor before we hit the call site. If we are already on that actor, the hop will be eliminated by the forward pass. But if the hop has not been eliminated, then the hop must be needed to return us to the appropriate actor. We will also keep the behavior that returns from a caller isolation inheriting function are considered to keep hop to executors alive. If we were able to propagate to a hop to executor before the return inst with the forward dataflow, then we know that we are guaranteed to still be on the relevant actor. If the hop to executor is still there, then we need it to ensure that our caller can treat the caller isolation inheriting function as a non-suspension point. rdar://155905383
1 parent a28d065 commit a0911ef

File tree

3 files changed

+191
-1
lines changed

3 files changed

+191
-1
lines changed

‎include/swift/SIL/ApplySite.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -894,6 +894,21 @@ class FullApplySite : public ApplySite {
894894
getNumIndirectSILErrorResults();
895895
}
896896

897+
std::optional<ActorIsolation> getActorIsolation() const {
898+
if (auto isolation = getIsolationCrossing();
899+
isolation && isolation->getCalleeIsolation())
900+
return isolation->getCalleeIsolation();
901+
auto *calleeFunction = getCalleeFunction();
902+
if (!calleeFunction)
903+
return {};
904+
return calleeFunction->getActorIsolation();
905+
}
906+
907+
bool isCallerIsolationInheriting() const {
908+
auto isolation = getActorIsolation();
909+
return isolation && isolation->isCallerIsolationInheriting();
910+
}
911+
897912
static FullApplySite getFromOpaqueValue(void *p) { return FullApplySite(p); }
898913

899914
static bool classof(const SILInstruction *inst) {

‎lib/SILOptimizer/Mandatory/OptimizeHopToExecutor.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,16 @@ void OptimizeHopToExecutor::solveDataflowBackward() {
193193
/// Returns true if \p inst is a suspension point or an async call.
194194
static bool isSuspensionPoint(SILInstruction *inst) {
195195
if (auto applySite = FullApplySite::isa(inst)) {
196+
// NOTE: For 6.2, we consider nonisolated(nonsending) to be a suspension
197+
// point, when it really isn't. We do this so that we have a truly
198+
// conservative change that does not change output.
196199
if (applySite.isAsync())
197200
return true;
198201
return false;
199202
}
200203
if (isa<AwaitAsyncContinuationInst>(inst))
201204
return true;
205+
202206
return false;
203207
}
204208

@@ -311,6 +315,40 @@ void OptimizeHopToExecutor::updateNeedExecutor(int &needExecutor,
311315
needExecutor = BlockState::NoExecutorNeeded;
312316
return;
313317
}
318+
319+
// For 6.2 to be conservative, if we are calling a function with
320+
// caller_isolation_inheriting isolation, treat the callsite as if the
321+
// callsite is an instruction that needs an executor.
322+
//
323+
// DISCUSSION: The reason why we are doing this is that in 6.2, we are going
324+
// to continue treating caller isolation inheriting functions as a suspension
325+
// point for the purpose of eliminating redundant hop to executor to not make
326+
// this optimization more aggressive. Post 6.2, we will stop treating caller
327+
// isolation inheriting functions as suspension points, meaning this code can
328+
// be deleted.
329+
if (auto fas = FullApplySite::isa(inst);
330+
fas && fas.isAsync() && fas.isCallerIsolationInheriting()) {
331+
needExecutor = BlockState::ExecutorNeeded;
332+
return;
333+
}
334+
335+
// For 6.2, if we are in a caller isolation inheriting function, we need to
336+
// treat its return as an executor needing function before
337+
// isSuspensionPoint.
338+
//
339+
// DISCUSSION: We need to do this here since for 6.2, a caller isolation
340+
// inheriting function is going to be considered a suspension point to be
341+
// conservative and make this optimization strictly more conservative. Post
342+
// 6.2, since caller isolation inheriting functions will no longer be
343+
// considered suspension points, we will be able to sink this code into needs
344+
// executor.
345+
if (auto isolation = inst->getFunction()->getActorIsolation();
346+
isolation && isolation->isCallerIsolationInheriting() &&
347+
isa<ReturnInst>(inst)) {
348+
needExecutor = BlockState::ExecutorNeeded;
349+
return;
350+
}
351+
314352
if (isSuspensionPoint(inst)) {
315353
needExecutor = BlockState::NoExecutorNeeded;
316354
return;

‎test/SILOptimizer/optimize_hop_to_executor.sil

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
// RUN: %target-sil-opt -enable-sil-verify-all %s -optimize-hop-to-executor | %FileCheck %s
1+
// RUN: %target-sil-opt -sil-print-function-isolation-info -enable-sil-verify-all %s -optimize-hop-to-executor | %FileCheck %s
22

33
// REQUIRES: concurrency
4+
// REQUIRES: asserts
45

56
sil_stage canonical
67

@@ -13,6 +14,7 @@ actor MyActor {
1314

1415
sil [ossa] @requiredToRunOnActor : $@convention(method) (@guaranteed MyActor) -> ()
1516
sil [ossa] @anotherAsyncFunction : $@convention(thin) @async () -> ()
17+
sil [ossa] @syncFunction : $@convention(thin) () -> ()
1618

1719
// CHECK-LABEL: sil [ossa] @simpleRedundantActor : $@convention(method) @async (@guaranteed MyActor) -> () {
1820
// CHECK: bb0(%0 : @guaranteed $MyActor):
@@ -302,3 +304,138 @@ bb0(%0 : @guaranteed $MyActor):
302304
%r = tuple ()
303305
return %r : $()
304306
}
307+
308+
// CHECK-LABEL: sil [ossa] @simpleDCEAsync : $@convention(thin) @async (@guaranteed MyActor) -> () {
309+
// CHECK-NOT: hop_to_executor
310+
// CHECK: } // end sil function 'simpleDCEAsync'
311+
sil [ossa] @simpleDCEAsync : $@convention(thin) @async (@guaranteed MyActor) -> () {
312+
bb0(%0 : @guaranteed $MyActor):
313+
hop_to_executor %0
314+
%f = function_ref @anotherAsyncFunction : $@convention(thin) @async () -> ()
315+
apply %f() : $@convention(thin) @async () -> ()
316+
%9999 = tuple ()
317+
return %9999 : $()
318+
}
319+
320+
// CHECK-LABEL: sil [ossa] @simpleCallerIsolationInheritingStopsDCE : $@convention(thin) @async (@guaranteed MyActor) -> () {
321+
// CHECK: hop_to_executor
322+
// CHECK: } // end sil function 'simpleCallerIsolationInheritingStopsDCE'
323+
sil [ossa] @simpleCallerIsolationInheritingStopsDCE : $@convention(thin) @async (@guaranteed MyActor) -> () {
324+
bb0(%0 : @guaranteed $MyActor):
325+
hop_to_executor %0
326+
%f = function_ref @anotherAsyncFunction : $@convention(thin) @async () -> ()
327+
apply [callee_isolation=caller_isolation_inheriting] [caller_isolation=caller_isolation_inheriting] %f() : $@convention(thin) @async () -> ()
328+
%9999 = tuple ()
329+
return %9999 : $()
330+
}
331+
332+
// We should eliminate none of the hop_to_executor here since
333+
// caller_isolation_inheriting @async apply sites do not cross isolation
334+
// boundaries.
335+
//
336+
// CHECK-LABEL: sil [ossa] @simpleCallerIsolationInheritingStopsDCE2 : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
337+
// CHECK: hop_to_executor
338+
// CHECK: hop_to_executor
339+
// CHECK: hop_to_executor
340+
// CHECK: } // end sil function 'simpleCallerIsolationInheritingStopsDCE2'
341+
sil [ossa] @simpleCallerIsolationInheritingStopsDCE2 : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
342+
bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor, %2 : @guaranteed $MyActor):
343+
hop_to_executor %0
344+
%f = function_ref @anotherAsyncFunction : $@convention(thin) @async () -> ()
345+
apply [callee_isolation=caller_isolation_inheriting] [caller_isolation=caller_isolation_inheriting] %f() : $@convention(thin) @async () -> ()
346+
hop_to_executor %1
347+
%f2 = function_ref @syncFunction : $@convention(thin) () -> ()
348+
apply %f2() : $@convention(thin) () -> ()
349+
apply [callee_isolation=caller_isolation_inheriting] [caller_isolation=caller_isolation_inheriting] %f() : $@convention(thin) @async () -> ()
350+
hop_to_executor %2
351+
apply %f2() : $@convention(thin) () -> ()
352+
apply [callee_isolation=caller_isolation_inheriting] [caller_isolation=caller_isolation_inheriting] %f() : $@convention(thin) @async () -> ()
353+
%9999 = tuple ()
354+
return %9999 : $()
355+
}
356+
357+
// CHECK-LABEL: sil [ossa] @simpleWithoutCallerIsolationInheritingHaveDCE : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
358+
// CHECK: bb0([[ARG1:%.*]] : @guaranteed $MyActor, [[ARG2:%.*]] : @guaranteed $MyActor, [[ARG3:%.*]] : @guaranteed $MyActor):
359+
// CHECK-NEXT: // function_ref anotherAsyncFunction
360+
// CHECK-NEXT: [[FUNC:%.*]] = function_ref @anotherAsyncFunction : $@convention(thin) @async () -> ()
361+
// CHECK-NEXT: apply [[FUNC]]() : $@convention(thin) @async () -> ()
362+
// CHECK-NEXT: hop_to_executor [[ARG2]]
363+
// CHECK-NEXT: // function_ref syncFunction
364+
// CHECK-NEXT: [[FUNC2:%.*]] = function_ref @syncFunction : $@convention(thin) () -> ()
365+
// CHECK-NEXT: apply [[FUNC2]]() : $@convention(thin) () -> ()
366+
// CHECK-NEXT: apply [[FUNC]]() : $@convention(thin) @async () -> ()
367+
// CHECK-NEXT: hop_to_executor [[ARG3]]
368+
// CHECK-NEXT: apply [[FUNC2]]() : $@convention(thin) () -> ()
369+
// CHECK-NEXT: apply [[FUNC]]() : $@convention(thin) @async () -> ()
370+
// CHECK-NEXT: apply [[FUNC]]() : $@convention(thin) @async () -> ()
371+
// CHECK: } // end sil function 'simpleWithoutCallerIsolationInheritingHaveDCE'
372+
sil [ossa] @simpleWithoutCallerIsolationInheritingHaveDCE : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
373+
bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor, %2 : @guaranteed $MyActor):
374+
hop_to_executor %0
375+
%f = function_ref @anotherAsyncFunction : $@convention(thin) @async () -> ()
376+
apply %f() : $@convention(thin) @async () -> ()
377+
hop_to_executor %1
378+
%f2 = function_ref @syncFunction : $@convention(thin) () -> ()
379+
apply %f2() : $@convention(thin) () -> ()
380+
apply %f() : $@convention(thin) @async () -> ()
381+
hop_to_executor %2
382+
apply %f2() : $@convention(thin) () -> ()
383+
apply %f() : $@convention(thin) @async () -> ()
384+
hop_to_executor %2
385+
apply %f() : $@convention(thin) @async () -> ()
386+
%9999 = tuple ()
387+
return %9999 : $()
388+
}
389+
390+
// We do not allow for these to be propagated yet through caller isolation
391+
// inheriting, so we should have all of the hop_to_executor.
392+
//
393+
// CHECK-LABEL: sil [ossa] @callerIsolationInheritingStopsAllowsPropagating : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
394+
// CHECK: bb0(
395+
// CHECK: hop_to_executor
396+
// CHECK: hop_to_executor
397+
// CHECK: hop_to_executor
398+
// CHECK: } // end sil function 'callerIsolationInheritingStopsAllowsPropagating'
399+
sil [ossa] @callerIsolationInheritingStopsAllowsPropagating : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
400+
bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor, %2 : @guaranteed $MyActor):
401+
hop_to_executor %0
402+
%f = function_ref @anotherAsyncFunction : $@convention(thin) @async () -> ()
403+
apply [callee_isolation=caller_isolation_inheriting] [caller_isolation=caller_isolation_inheriting] %f() : $@convention(thin) @async () -> ()
404+
hop_to_executor %0
405+
%f2 = function_ref @syncFunction : $@convention(thin) () -> ()
406+
apply %f2() : $@convention(thin) () -> ()
407+
apply [callee_isolation=caller_isolation_inheriting] [caller_isolation=caller_isolation_inheriting] %f() : $@convention(thin) @async () -> ()
408+
hop_to_executor %0
409+
apply %f2() : $@convention(thin) () -> ()
410+
apply [callee_isolation=caller_isolation_inheriting] [caller_isolation=caller_isolation_inheriting] %f() : $@convention(thin) @async () -> ()
411+
%9999 = tuple ()
412+
return %9999 : $()
413+
}
414+
415+
// Since we are caller isolation inheriting, make sure that we leave around the
416+
// hop_to_executor due to ehre elase. We do eliminate one of the hop_to_executor
417+
// though.
418+
//
419+
// CHECK: sil [isolation "caller_isolation_inheriting"] [ossa] @callerIsolationInheritingStopsReturnDeadEnd : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
420+
// CHECK: bb0(
421+
// CHECK-NEXT: hop_to_executor
422+
// CHECK-NEXT: tuple
423+
// CHECK-NEXT: return
424+
// CHECK: } // end sil function 'callerIsolationInheritingStopsReturnDeadEnd'
425+
sil [isolation "caller_isolation_inheriting"] [ossa] @callerIsolationInheritingStopsReturnDeadEnd : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
426+
bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor, %2 : @guaranteed $MyActor):
427+
hop_to_executor %0
428+
hop_to_executor %0
429+
%9999 = tuple ()
430+
return %9999 : $()
431+
}
432+
433+
// CHECK-LABEL: sil [ossa] @noIsolationStillRemoves : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
434+
// CHECK-NOT: hop_to_executor
435+
// CHECK: } // end sil function 'noIsolationStillRemoves'
436+
sil [ossa] @noIsolationStillRemoves : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
437+
bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor, %2 : @guaranteed $MyActor):
438+
hop_to_executor %0
439+
%9999 = tuple ()
440+
return %9999 : $()
441+
}

0 commit comments

Comments
 (0)