Immediately after the hop_to_executor in an async, distributed
actor init, we need to notify the transport that the actor is ready.
This patch does not yet account for more complex cases. In particular,
we will need a mechanism to prevent multiple calls to actorReady,
which can happen if a loop appears in the init:
distributed actor Dactor {
var x: Int
init(tport: ActorTransport, count: Int) async {
var i = count
repeat {
self.x = count
// hop is injected here
i -= 1
} while i > 0
}
}
Those instructions should not be "visible" for debugging and for diagnostic messages.
Fixes a wrong "unreachable code" warning.
https://bugs.swift.org/browse/SR-14873
rdar://80237870
Because `self` can be used unrestricted after it is
fully-initialized in an async actor init, we perform
a hop_to_executor(self) immediately after `self`
is fully-initialized on all paths within the
initializer.
If we did not, then code could race with the
initializer if it, e.g., spawns a task from
the initializer and mutates actor state that
the initializer then reads. By hopping to the
executor, we prevent that task from running
until _after_ the initializer completes.
A uniqueness rule for 'self' is enforced throughout
the following types of actor initializers:
1. synchronous
2. global-actor isolated
This means that 'self' cannot be used for anything,
even after 'self' is fully initialized, other than
for access to stored properties. Method calls are
considered escaping uses of 'self', since it is passed
implicitly to the method of computed property when invoked.
Also, an actor's convenience init now treats self as
nonisolated.
This provides a way to perform follow-up work after
self is fully initialized, when creating the
actor from a synchronous context.
Resolves rdar://78790369
* [Distributed] Initial distributed checking
* [Distributed] initial types shapes and conform to DistributedActor
* [Distributed] Require Codable params and return types
* [Distributed] initial synthesis of fields and constructors
* [Distributed] Field and initializer synthesis
* [Distributed] Codable requirement on distributed funcs; also handle <T: Codable>
* [Distributed] handle generic type params which are Codable in dist func
[Distributed] conformsToProtocol after all
* [Distributed] Implement remote flag on actors
* Implement remote flag on actors
* add test
* actor initializer that sets remote flag
[Distributed] conformances getting there
* [Distributed] dont require async throws; cleanup compile tests
* [Distributed] do not synthesize default implicit init, only our special ones
* [Distributed] properly synth inits and properties; mark actorTransport as _distributedActorIndependent
Also:
- do not synthesize default init() initializer for dist actor
* [Distributed] init(transport:) designated and typechecking
* [Distributed] dist actor initializers MUST delegate to local-init
* [Distributed] check if any ctors in delegation call init(transport:)
* [Distributed] check init(transport:) delegation through many inits; ban invoking init(resolve:using:) explicitly
* [Distributed] disable IRGen test for now
* [Distributed] Rebase cleanups
* [Concurrent] transport and address are concurrent value
* [Distributed] introduce -enable-experimental-distributed flag
* rebase adjustments again
* rebase again...
* [Distributed] distributed functions are implicitly async+throws outside the actor
* [Distributed] implicitly throwing and async distributed funcs
* remove printlns
* add more checks to implicit function test
* [Distributed] resolve initializer now marks the isRemote actor flag
* [Distributed] distributedActor_destroy invoked instead, rather than before normal
* [Distributed] Generate distributed thunk for actors
* [distributed] typechecking for _remote_ functions existing, add tests for remote funcs
* adding one XFAIL'ed task & actor lifetime test
The `executor_deinit1` test fails 100% of the time
(from what I've seen) so I thought we could track
and see when/if someone happens to fix this bug.
Also, added extra coverage for #36298 via `executor_deinit2`
* Fix a memory issue with actors in the runtime system, by @phausler
* add new test that now passes because of patch by @phausler
See previous commit in this PR.
Test is based on one from rdar://74281361
* fix all tests that require the _remote_ function stubs
* Do not infer @actorIndependent onto `let` decls
* REVERT_ME: remove some tests that hacky workarounds will fail
* another flaky test, help build toolchain
* [Distributed] experimental distributed implies experimental concurrency
* [Distributed] Allow distributed function that are not marked async or throws
* [Distributed] make attrs SIMPLE to get serialization generated
* [Distributed] ActorAddress must be Hashable
* [Distributed] Implement transport.actorReady call in local init
* cleanup after rebase
* [Distributed] add availability attributes to all distributed actor code
* cleanup - this fixed some things
* fixing up
* fixing up
* [Distributed] introduce new Distributed module
* [Distributed] diagnose when missing 'import _Distributed'
* [Distributed] make all tests import the module
* more docs on address
* [Distributed] fixup merge issues
* cleanup: remove unnecessary code for now SIMPLE attribute
* fix: fix getActorIsolationOfContext
* [Distributed] cmake: depend on _concurrency module
* fixing tests...
* Revert "another flaky test, help build toolchain"
This reverts commit 83ae6654dd.
* remove xfail
* clenup some IR and SIL tests
* cleanup
* [Distributed] fix cmake test and ScanDependencies/can_import_with_map.swift
* [Distributed] fix flags/build tests
* cleanup: use isDistributed wherever possible
* [Distributed] don't import Dispatch in tests
* dont link distributed in stdlib unittest
* trying always append distributed module
* cleanups
* [Distributed] move all tests to Distributed/ directory
* [lit] try to fix lit test discovery
* [Distributed] update tests after diagnostics for implicit async changed
* [Distributed] Disable remote func tests on Windows for now
* Review cleanups
* [Distributed] fix typo, fixes Concurrency/actor_isolation_objc.swift
* [Distributed] attributes are DistributedOnly (only)
* cleanup
* [Distributed] cleanup: rely on DistributedOnly for guarding the keyword
* Update include/swift/AST/ActorIsolation.h
Co-authored-by: Doug Gregor <dgregor@apple.com>
* introduce isAnyThunk, minor cleanup
* wip
* [Distributed] move some type checking to TypeCheckDistributed.cpp
* [TypeCheckAttr] remove extra debug info
* [Distributed/AutoDiff] fix SILDeclRef creation which caused AutoDiff issue
* cleanups
* [lit] remove json import from lit test suite, not needed after all
* [Distributed] distributed functions only in DistributedActor protocols
* [Distributed] fix flag overlap & build setting
* [Distributed] Simplify noteIsolatedActorMember to not take bool distributed param
* [Distributed] make __isRemote not public
* [Distributed] Fix availability and remove actor class tests
* [actorIndependent] do not apply actorIndependent implicitly to values where it would be illegal to apply
* [Distributed] disable tests until issue fixed
Co-authored-by: Dario Rexin <drexin@apple.com>
Co-authored-by: Kavon Farvardin <kfarvardin@apple.com>
Co-authored-by: Doug Gregor <dgregor@apple.com>
* Revert "[Distributed] disable tests until issue fixed"
This reverts commit 0a04278920.
* Revert "[Distributed] Initial `distributed` actors and functions and new module (#37109)"
This reverts commit 814ede0cf3.
* [Distributed] Initial distributed checking
* [Distributed] initial types shapes and conform to DistributedActor
* [Distributed] Require Codable params and return types
* [Distributed] initial synthesis of fields and constructors
* [Distributed] Field and initializer synthesis
* [Distributed] Codable requirement on distributed funcs; also handle <T: Codable>
* [Distributed] handle generic type params which are Codable in dist func
[Distributed] conformsToProtocol after all
* [Distributed] Implement remote flag on actors
* Implement remote flag on actors
* add test
* actor initializer that sets remote flag
[Distributed] conformances getting there
* [Distributed] dont require async throws; cleanup compile tests
* [Distributed] do not synthesize default implicit init, only our special ones
* [Distributed] properly synth inits and properties; mark actorTransport as _distributedActorIndependent
Also:
- do not synthesize default init() initializer for dist actor
* [Distributed] init(transport:) designated and typechecking
* [Distributed] dist actor initializers MUST delegate to local-init
* [Distributed] check if any ctors in delegation call init(transport:)
* [Distributed] check init(transport:) delegation through many inits; ban invoking init(resolve:using:) explicitly
* [Distributed] disable IRGen test for now
* [Distributed] Rebase cleanups
* [Concurrent] transport and address are concurrent value
* [Distributed] introduce -enable-experimental-distributed flag
* rebase adjustments again
* rebase again...
* [Distributed] distributed functions are implicitly async+throws outside the actor
* [Distributed] implicitly throwing and async distributed funcs
* remove printlns
* add more checks to implicit function test
* [Distributed] resolve initializer now marks the isRemote actor flag
* [Distributed] distributedActor_destroy invoked instead, rather than before normal
* [Distributed] Generate distributed thunk for actors
* [distributed] typechecking for _remote_ functions existing, add tests for remote funcs
* adding one XFAIL'ed task & actor lifetime test
The `executor_deinit1` test fails 100% of the time
(from what I've seen) so I thought we could track
and see when/if someone happens to fix this bug.
Also, added extra coverage for #36298 via `executor_deinit2`
* Fix a memory issue with actors in the runtime system, by @phausler
* add new test that now passes because of patch by @phausler
See previous commit in this PR.
Test is based on one from rdar://74281361
* fix all tests that require the _remote_ function stubs
* Do not infer @actorIndependent onto `let` decls
* REVERT_ME: remove some tests that hacky workarounds will fail
* another flaky test, help build toolchain
* [Distributed] experimental distributed implies experimental concurrency
* [Distributed] Allow distributed function that are not marked async or throws
* [Distributed] make attrs SIMPLE to get serialization generated
* [Distributed] ActorAddress must be Hashable
* [Distributed] Implement transport.actorReady call in local init
* cleanup after rebase
* [Distributed] add availability attributes to all distributed actor code
* cleanup - this fixed some things
* fixing up
* fixing up
* [Distributed] introduce new Distributed module
* [Distributed] diagnose when missing 'import _Distributed'
* [Distributed] make all tests import the module
* more docs on address
* [Distributed] fixup merge issues
* cleanup: remove unnecessary code for now SIMPLE attribute
* fix: fix getActorIsolationOfContext
* [Distributed] cmake: depend on _concurrency module
* fixing tests...
* Revert "another flaky test, help build toolchain"
This reverts commit 83ae6654dd.
* remove xfail
* clenup some IR and SIL tests
* cleanup
* [Distributed] fix cmake test and ScanDependencies/can_import_with_map.swift
* [Distributed] fix flags/build tests
* cleanup: use isDistributed wherever possible
* [Distributed] don't import Dispatch in tests
* dont link distributed in stdlib unittest
* trying always append distributed module
* cleanups
* [Distributed] move all tests to Distributed/ directory
* [lit] try to fix lit test discovery
* [Distributed] update tests after diagnostics for implicit async changed
* [Distributed] Disable remote func tests on Windows for now
* Review cleanups
* [Distributed] fix typo, fixes Concurrency/actor_isolation_objc.swift
* [Distributed] attributes are DistributedOnly (only)
* cleanup
* [Distributed] cleanup: rely on DistributedOnly for guarding the keyword
* Update include/swift/AST/ActorIsolation.h
Co-authored-by: Doug Gregor <dgregor@apple.com>
* introduce isAnyThunk, minor cleanup
* wip
* [Distributed] move some type checking to TypeCheckDistributed.cpp
* [TypeCheckAttr] remove extra debug info
* [Distributed/AutoDiff] fix SILDeclRef creation which caused AutoDiff issue
* cleanups
* [lit] remove json import from lit test suite, not needed after all
* [Distributed] distributed functions only in DistributedActor protocols
* [Distributed] fix flag overlap & build setting
* [Distributed] Simplify noteIsolatedActorMember to not take bool distributed param
* [Distributed] make __isRemote not public
Co-authored-by: Dario Rexin <drexin@apple.com>
Co-authored-by: Kavon Farvardin <kfarvardin@apple.com>
Co-authored-by: Doug Gregor <dgregor@apple.com>
* Refactoring: replace "Destination" and the ownership qualifier by a single "Mode". This represents much better the mode how the instruction is to be lowered. NFC
* Make assign_by_wrapper printable and parseable.
* Fix lowering of the assign modes for indirect results of the init-closure: The indirect result was initialized and not assigned to. The fix is to insert a destroy_addr before calling the init closure. This fixes a memory lifetime error and/or a memory leak. Found by inspection.
* Fix an iterator-invalidation crash in RawSILInstLowering
* Add tests for lowering assign_by_wrapper.
Specifically before this PR, if a caller did not customize a specific callback
of InstModCallbacks, we would store a static default std::function into
InstModCallbacks. This means that we always would have an indirect jump. That is
unfortunate since this code is often called in loops.
In this PR, I eliminate this problem by:
1. I made all of the actual callback std::function in InstModCallback private
and gave them a "Func" postfix (e.x.: deleteInst -> deleteInstFunc).
2. I created public methods with the old callback names to actually call the
callbacks. This ensured that as long as we are not escaping callbacks from
InstModCallback, this PR would not result in the need for any source changes
since we are changing a call of a std::function field to a call to a method.
3. I changed all of the places that were escaping inst mod's callbacks to take
an InstModCallback. We shouldn't be doing that anyway.
4. I changed the default value of each callback in InstModCallbacks to be a
nullptr and changed the public helper methods to check if a callback is
null. If the callback is not null, it is called, otherwise the getter falls
back to an inline default implementation of the operation.
All together this means that the cost of a plain InstModCallback is reduced and
one pays an indirect function cost price as one customizes it further which is
better scalability.
P.S. as a little extra thing, I added a madeChange field onto the
InstModCallback. Now that we have the helpers calling the callbacks, I can
easily insert instrumentation like this, allowing for users to pass in
InstModCallback and see if anything was RAUWed without needing to specify a
callback.
LifetimeChecker::shouldEmitError in order to avoid incorrectly
populating EmittedErrorLocs when no error is emitted.
This fixes a few corner cases where DI would error out or crash
while assigning to a wrapped property with a nonmutating setter.
It would be more abstractly correct if this got DI support so
that we destroy the member if the constructor terminates
abnormally, but we can get to that later.
In a constructor, SILGen lowers type(of: self) to a value_metatype
instruction, but 'self' might not have been initialized yet if
this is a convenience initializer, so we replace it with a use of
the 'self' metatype argument.
However getDynamicSelfMetadata() asserts if 'self' is a non-class
type. Since we know we're inside of a proper method here and not a
closure that captured 'self', we can use getSelfArgument() instead.
Fixes https://bugs.swift.org/browse/SR-12665 / rdar://problem/62481587.
DI had trouble with this pattern:
%s = struct_element_addr ...
%t0 = tuple_element_addr %s, 0
%t1 = tuple_element_addr %s, 1
%f = function_ref ...
apply %f(%t0, %t1)
This is because the NonLoadUses map only stored a single use of a
tuple element per instruction, preventing instructions such as
'apply' from being able to use multiple tuple elements.
In other cases where this comes up, such as with 'assign' and
'copy_addr', DI scalarizes the tuple operation by projecting
each component, however clearly this can't be easily done with
an 'apply'.
Instead, we can get DI out of the business of scalarization, at
least for instructions which are known to perform an unconditional
initialization.
We do this by changing the NonLoadUses map to store a vector of
DIMemoryUse IDs instead of a single value.
In a designated initializer of a non-root class, 'self' becomes
fully initialized after the 'super.init' call, at which point
escaping uses of 'self' become valid, and releases of 'self' are
lowered to a 'strong_release' instruction, which runs the
deinitializer.
In a root class, 'self' becomes fully initialized after all stored
properties have been initialized, at which point escaping uses of
'self' become valid.
However, DI would still lower a conditional destroy of 'self' by
destroying any initialized stored properties and freeing the object
with 'dealloc_partial_ref'. This is incorrect, because 'self' may
have escaped.
In the non-conditional destroy case, we correctly lowered the
release to a 'strong_release' if all stored properties are known
to be initialized.
Fix DI to handle the conditional destroy case by first checking if all
bits in the control variable are set, and releasing the instance with
'strong_release' if so. The 'dealloc_partial_ref' is only emitted
if not at least one stored property was not initialized.
This ensures that we don't deallocate an instance that may have
escaped.
Fixes <https://bugs.swift.org/browse/SR-13439>, <rdar://problem/67746791>,
<https://bugs.swift.org/browse/SR-13355>, <rdar://problem/67361228>.
While we don't need to destroy these fields, we still need to know
when the class is _semantically_ fully initialized, since this will
determine if we're going to release the class, running the deinitializer,
or if we're going to deallocate the memory for the instance directly.
Previously we would just forward the cleanup and create a normal "destroy"
cleanup resulting in early deallocation and use after frees along error paths.
As part of this I also had to tweak how DI recognizes self init writebacks to
not use SILLocations. This is approach is more robust (since we aren't relying
on SourceLocs/have verifiers to make sure we don't violate SIL) and also avoids
issues from the write back store not necessarily have the same SILLocation.
<rdar://problem/59830255>
This is achieved in 3 steps:
1. CSApply detects assignments to property wrappers inside constructors, and produces an `inout` expr instead of a `load`, which it normally would because nonmutating setters take the `self` by-value. This is necessary becasue the assign_by_wrapper instruction expects an address type for its $1 operand.
2. SILGenLValue now emits the assign_by_wrapper pattern for such setters, ignoring the fact that they capture `self` by value. It also introduces an additional load instruction for the setter patrial_apply because the setter signature still expects a value and we now have an address (because of (1)).
3. DefiniteInitialization specifically ignores load instructions used to produce a `self` value for a setter referenced on assign_by_wrapper because it will be deleted by lowering anyway.
Resolves rdar://problem/60600911 and rdar://problem/52280477
This commit eliminates the need for mark uninitialized fixup by updating the
compiler so that we now emit:
```
%0 = alloc_box
%1 = mark_uninitialized %0
%2 = project_box %1
...
destroy_value %1
```
Instead of:
```
%0 = alloc_box
%1 = project_box %0
%2 = mark_uninitialized %1
...
destroy_value %0
```
Now that the first type of code is generated, I can change project_box to only
take guaranteed arguments. This will ensure that the OSSA ARC optimizer can
eliminate copies of boxes without needing to understand the usage of the
project_box.
This is going to let me change getUninitializedValue() to special case alloc_box
so that we return the project_box (the address we want to analyze) rather than
the mark_uninitialized (which is on the box).
Specifically isDefiniteInitFinished is always set to false and
TreatAddressToPointerAsInout is set to true when ever DI is run, so this patch
just constant propagates those values and then DCEs as appropriate.
and eliminate dead code. This is meant to be a replacement for the utility:
recursivelyDeleteTriviallyDeadInstructions. The new utility performs more aggresive
dead-code elimination for ownership SIL.
This patch also migrates most non-force-delete uses of
recursivelyDeleteTriviallyDeadInstructions to the new utility.
and migrates one force-delete use of recursivelyDeleteTriviallyDeadInstructions
(in IRGenPrepare) to use the new utility.
The XXOptUtils.h convention is already established and parallels
the SIL/XXUtils convention.
New:
- InstOptUtils.h
- CFGOptUtils.h
- BasicBlockOptUtils.h
- ValueLifetime.h
Removed:
- Local.h
- Two conflicting CFG.h files
This reorganization is helpful before I introduce more
utilities for block cloning similar to SinkAddressProjections.
Move the control flow utilies out of Local.h, which was an
unreadable, unprincipled mess. Rename it to InstOptUtils.h, and
confine it to small APIs for working with individual instructions.
These are the optimizer's additions to /SIL/InstUtils.h.
Rename CFG.h to CFGOptUtils.h and remove the one in /Analysis. Now
there is only SIL/CFG.h, resolving the naming conflict within the
swift project (this has always been a problem for source tools). Limit
this header to low-level APIs for working with branches and CFG edges.
Add BasicBlockOptUtils.h for block level transforms (it makes me sad
that I can't use BBOptUtils.h, but SIL already has
BasicBlockUtils.h). These are larger APIs for cloning or removing
whole blocks.
This flag is set by DefinitInitialization if the lifetime of the stored value is controlled dynamically.
If the flag is set, it's not (easily) possibly to statically calculate the lifetime of the stored value.
With the advent of dynamic_function_ref the actual callee of such a ref
my vary. Optimizations should not assume to know the content of a
function referenced by dynamic_function_ref. Introduce
getReferencedFunctionOrNull which will return null for such function
refs. And getInitialReferencedFunction to return the referenced
function.
Use as appropriate.
rdar://50959798
