-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[concurrency] Change #isolated to mask out the TBI bits of the witness pointer of the implicit isolated any Actor pointer so we can do optimizations on TBI supporting platforms in the future. #83346
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
base: main
Are you sure you want to change the base?
[concurrency] Change #isolated to mask out the TBI bits of the witness pointer of the implicit isolated any Actor pointer so we can do optimizations on TBI supporting platforms in the future. #83346
Conversation
@swift-ci smoke test |
isolatedMV = ManagedValue::forBorrowedRValue(isolatedArg); | ||
} else { | ||
isolatedMV = ManagedValue::forUnmanagedOwnedValue(isolatedArg); | ||
auto isolatedMV = ManagedValue::forBorrowedRValue(isolatedArg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we never see unowned here, or is it handled is part of this call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will never be unowned. We always know that it will be @guaranteed. I checked.
@@ -670,6 +670,9 @@ namespace swift { | |||
bool RestrictNonProductionExperimentalFeatures = false; | |||
#endif | |||
|
|||
/// Set to true if we support AArch64TBI. | |||
bool HasAArch64TBI = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have expected this in the IR gen options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I didn't put it into IRGen options is that I am using it in SILGen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DougGregor I was thinking about this. Really the whole feature should be in IRGen and just be a flag on an apply. That being said, I want to do that later on main.
1fa6f01
to
f770390
Compare
@swift-ci smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICS looking good 👍
…pointer of the implicit isolated any Actor pointer so we can do optimizations on TBI supporting platforms in the future. The specific bits that we mask are the bottom two bits of the top nibble of the TBI. This will let us perform an optimization in the future where a caller can set bits in the TBI byte to signal information dynamically to a nonisolated(nonsending) callee. The intent is that this will let us signal to a nonisolated(nonsending) callee that the caller isn't performing any isolated work before it hops (or potentially a caller of the caller) hops meaning that the nonisolated(nonsending) function can avoid a tail hop that restores isolation after calling some sort of actor isolated call. E.x.: ```swift nonisolated(nonsending) func callsActor(_ a: MyActor) async { await a.doSomething() } @Concurrent func runOnBackground() async {} actor A { func doSomething() async { ... } func caller() async { await callsActor(self) await runOnBackgroud() } } ``` In the above case, the hop back in callsActor is not needed dynamically in caller since we are going to immediately await ontot he background. But we do not know that. An optimization can be written that can recognize this case and dynamically tell via the tbi bits of the protocol witness table of the any Actor call that the hop to executor call can avoid the hop. Since this only applies to the implicit isolated any Actor parameter which is only used for the purpose of hopping, it is safe to do this since when we hop we just call the Actor unowned executor dispatch thunk which directly accesses the witness table by loading from that pointer... meaning that anything set in the TBI bit is irrelevent. The only other way to grab this implicit bit is to use `#isolation` and that is where this commit comes into play. Since we can access the actor via `#isolation`, we need to ensure that we mask out the TBI bits so that the rest of the runtime/compiled code never see the TBI bits. This is because we do not want the fact that we are using these TBI bits to ever escape into other code (e.x.: consider things that may cast). So we have basically created a hermetically sealed world where we can pass information from callee to caller using the protocol witness TBI bits. I also provided a similar implementation for platforms without TBI that use the tagged pointer bits of the protocol witness function. This is behind an experimental feature flag called NonisolatedNonsendingDynamicHopElim that is only available in asserts builds. rdar://156525771
…ding) by default enabled. Just adding more code coverage.
f770390
to
f589761
Compare
Most of the changes involve converting Builtin.Executor -> any Actor. Also in the case of concurrency-builtins.swift, we just had to import _Concurrency.
f589761
to
dcecf88
Compare
@swift-ci smoke test |
ManagedValue swift::Lowering::clearImplicitIsolatedActorBits( | ||
SILGenFunction &SGF, SILLocation loc, ManagedValue isolatedMV) { | ||
auto &ctx = SGF.getASTContext(); | ||
if (!ctx.LangOpts.hasFeature(Feature::NonisolatedNonsendingDynamicHopElim) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part shouldn't be behind the feature flag, right? It's the bit stripping we will always need to do.
SILGenFunction &SGF, SILLocation loc, ManagedValue isolatedMV) { | ||
auto &ctx = SGF.getASTContext(); | ||
if (!ctx.LangOpts.hasFeature(Feature::NonisolatedNonsendingDynamicHopElim) && | ||
!ctx.LangOpts.HasAArch64TBI) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we checking for TBI here? It seems like we effectively need to distinguish between places where TBI will strip the bits for us (e.g., we're dereferencing the pointer) vs those where we treat the pointer as a value and we need to strip the bits ourselves (e.g., when building a runtime data structure involving this witness table).
Specifically, this will let us perform an optimization in the future where a caller can set bits in the TBI byte to signal information dynamically to a nonisolated(nonsending) callee. The intent is that this will let us signal to a nonisolated(nonsending) callee that the caller isn't performing any isolated work before it hops (or potentially a caller of the caller) hops meaning that the nonisolated(nonsending) function can avoid a tail hop that restores isolation after calling some sort of actor isolated call.
E.x.:
In the above case, the hop back in callsActor is not needed dynamically in caller since we are going to immediately await ontot he background. But we do not know that. An optimization can be written that can recognize this case and dynamically tell via the tbi bits of the protocol witness table of the any Actor call that the hop to executor call can avoid the hop.
Since this only applies to the implicit isolated any Actor parameter which is only used for the purpose of hopping, it is safe to do this since when we hop we just call the Actor unowned executor dispatch thunk which directly accesses the witness table by loading from that pointer... meaning that anything set in the TBI bit is irrelevent.
The only other way to grab this implicit bit is to use
#isolation
and that is where this commit comes into play. Since we can access the actor via#isolation
, we need to ensure that we mask out the TBI bits so that the rest of the runtime/compiled code never see the TBI bits. This is because we do not want the fact that we are using these TBI bits to ever escape into other code (e.x.: consider things that may cast).So we have basically created a hermetically sealed world where we can pass information from callee to caller using the protocol witness TBI bits.
rdar://156525771