Skip to content

[concurrency] Make optimize hop to executor more conservative for 6.2 around caller isolation inheriting functions. #83083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

gottesmm
Copy link
Contributor

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

…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.
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm force-pushed the pr-e0e1571656b80b6c2d45f1a0cad842fbf6ae4800 branch from a0911ef to 28f3c94 Compare July 15, 2025 23:57
@gottesmm gottesmm requested a review from ktoso as a code owner July 15, 2025 23:57
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

… 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
@gottesmm gottesmm force-pushed the pr-e0e1571656b80b6c2d45f1a0cad842fbf6ae4800 branch from 28f3c94 to b394242 Compare July 16, 2025 00:04
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@@ -3630,6 +3635,15 @@ void SILFunction::print(SILPrintContext &PrintCtx) const {
OS << "[available " << availability.getVersionString() << "] ";
}

// This is here only for testing purposes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it just for "testing"? Seems like would be useful to just keep around anyway right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why I am only doing it for testing is that I am going to revamp this post 6.2. I don't want people to rely on it.

Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! And looking forward to the followups we can do post-6.2 :-)

@gottesmm gottesmm merged commit e18c68a into swiftlang:main Jul 16, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants