[6.2][concurrency] Make optimize hop to executor more conservative for 6.2 around caller isolation inheriting functions. #83084
+696
−59
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci test |
…n write more concurrency tests in SIL. Specifically, we write a string out like: sil [isolation "$REPRESENTATION OF ISOLATION"] @function : $@convention(thin) ... The idea is that by using a string, we avoid parsing issues of the isolation and have flexibility. I left in the way we put isolation into the comment above functions so I did not break any tests that rely on it. I also made it so that we only accept this with sil tests that pass in the flag "sil-print-function-isolation-info". I am going to for the next release put in a full real implementation of this that allows for actor isolation to become a true first class citizen in SIL. But for now this at least lets us write tests in the short term. Since this is temporary and behind a flag, I did not add support for serialization since this is just for writing textual SIL tests. (cherry picked from commit ee3027c) Conflicts: lib/SIL/Parser/ParseSIL.cpp
… 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 (cherry picked from commit b394242)
gottesmm
force-pushed
the
release/6.2-rdar155905383
branch
from
b8cb428 to
1821499
Compare
|
@swift-ci test |
gottesmm
changed the title
ktoso
approved these changes
Jul 16, 2025
rjmccall
requested changes
Jul 16, 2025
|
Oh, and you'll need to make sure that SILGen thunks actually emit hops in the right places when converting to/from caller-isolated types, because I'm pretty sure the current emission logic manually optimizes this (because of course it doesn't normally need to establish isolation just to do bridging and reabstraction). |
|
@swift-ci test |
1 similar comment
|
@swift-ci test |
…nding) handle hopping correctly. Specifically: 1. When we convert a function to nonisolated(nonsending), we need to make sure that in the thunk we hop upon return since nonisolated(nonsending) functions are assumed to preserve the caller's isolation. 2. When we convert a function from nonisolated(nonsending), we need to make sure that in the thunk we hop onto the actor that we are passing in as the isolated parameter of the nonisolated(nonsending) function. This ensures that the nonisolated(nonsending) function can assume that it is already on its isolated parameter's actor at function entry. rdar://155905383
gottesmm
force-pushed
the
release/6.2-rdar155905383
branch
from
6f9312f to
82b0ec3
Compare
|
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation: Stops elimination of hop_to_executor in a few cases.
Specifically for 6.2, we are making optimize hop to executor more conservative around caller isolation inheriting functions. This means that we are:
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.
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:
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.
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.
Scope: Narrow. Only causes the optimizer to be conservative in a few cases
Issues: rdar://155905383
Original PRs: #83083
Risk: Very low. Just makes optimizer more conservative.
Testing: CI
Reviewers: