Fixes a false alarm in case of recursive calls with different type parameters.
For example:
```
protocol P {
associatedtype E: P
}
func noRecursionMismatchingTypeArgs1<T: P>(_ t: T.Type) {
if T.self == Int.self {
return
}
noRecursionMismatchingTypeArgs1(T.E.self)
}
```
The reason why I am doing this is that I am going to be adding support for
preconcurrency imports to TransferNonSendable. That implies that we can have
preconcurrency import suppression in the SIL pipeline and thus that emitting the
diagnostic in Sema is too early.
To do this, I introduced a new module pass called
DiagnoseUnnecessaryPreconcurrencyImports that runs after the SILFunction pass
TransferNonSendable. The reason why I use a module pass is to ensure that
TransferNonSendable has run on all functions before we attempt to emit these
diagnostics. Then in that pass, we iterate over all of the modules functions and
construct a uniqued array of SourceFiles for these functions. Then we iterate
over the uniqued SourceFiles and use the already constructed Sema machinery to
emit the diagnostic using the source files.
rdar://126928265
I am doing this since region based isolation hit the same issue that the move
checker did. So it makes sense to refactor the functionality into its own pass
and move it into a helper pass that runs before both.
It is very conservative and only stubifies functions that the specialization
passes explicitly mark as this being ok to be done to.
Over time I am going to be using RegionAnalysis for a series of passes that all
use that same information since I am worried about RegionAnalysis computation
time. With that being said, we want to make sure to eliminate the memory that
RegionAnalysis uses once this series of passes have completed. What this commit
does is create a pass that explicitly invalidates region analysis and explicitly
places it in the pass pipeline after the series of passes. This will ensure that
even if we add an additional pass, there is a strong "rattlesnake" signal to the
new code author that the code needs to be placed before the region analysis
invalidation and will prevent mistakes such as having to recompute the region
analysis in that later pass or the later pass forgeting to invalidate the
analysis.
In the next commit, I am modifying the move only checker to ensure that we
always error when partially invalidating. While doing this I discovered that
these copies were getting in the way of emitting good diagnostics in that case.
If one has a type with a deinit, if one were to partially invalidate the value,
the checker will clean up the remaining parts of the value but not the actual
underlying value. This then would cause the deinit of the actual type to be
called.
rdar://101651138
Otherwise, sometimes when the object checker emits a diagnostic and cleans up
the IR, some of the cleaned up copies are copies that should have been handled
by the address checker. The end result is that the address checker does not emit
diagnostics for that IR. I found this problem was exascerbated when writing code
for escaping closures.
This commit also cleans up the passes in preparation for at a future time moving
some of the transformations into the utils folder.
The Swift Simplification pass can do more than the old MandatoryCombine pass: simplification of more instruction types and dead code elimination.
The result is a better -Onone performance while still keeping debug info consistent.
Currently following code patterns are simplified:
* `struct` -> `struct_extract`
* `enum` -> `unchecked_enum_data`
* `partial_apply` -> `apply`
* `br` to a 1:1 related block
* `cond_br` with a constant condition
* `isConcrete` and `is_same_metadata` builtins
More simplifications can be added in the future.
rdar://96708429
rdar://104562580
The reason that I am doing this is I discovered that we emit worse quality
diagnostics since if we emit an error while running the borrow to destructure
transform, we want to do a complete cleanup to be safe (e.x.: converting
copy_value -> explicit_copy_value). This causes the object checker to then not
emit any additional diagnostics for other variables that were not impacted by
the BorrowToDestructureTransform, reducing the quality of diagnostics in a
significant way that isn't needed.
This is a cleaner separation of concerns. The reason why I did not do this
originally is that I thought I would need to reuse this functionality in the
address checker, but this issue actually does not come up there since we project
the address and then load instead of load and then project.
This enables us to emit the appropriate error for consuming uses of fields and
also causes us to eliminate copies exposed by using fields of a move only type
in a non-consuming way.
rdar://103271138
NOTE: If one does not define a deinit on a move only type, if a destroy_value
lasts until IRGen time we will assert since I haven't implemented support in
IRGen for the destroy value witness for move only types. That being said, just
creating a deinit by adding to such a type:
```
deinit {}
```
is pretty low overhead for the experiments we want to use this for.
Some notes:
1. I added support for both loadable/address only types.
2. These tests are based off of porting the move only object tests for inout,
vars, mutating self, etc.
3. I did not include already written tests for address only types in this
specific merge since I need to change us to borrow move only var like types.
Without that, we get a lot of spurious error msgs and the burden of writing that
is not worth it. So instead in a forthcoming commit where I fix that issue in
SILGen, I will commit the corresponding address only tests for this work.
4. I did not include support for trivial types in this. I am going to do
object/address for that at the same time.
By using the keyword instead of the function, we actually get a much simpler
implementation since we avoid all of the machinery of SILGenApply. Given that we
are going down that path, I am removing the old builtin implementation since it
is dead code.
The reason why I am removing this now is that in a subsequent commit, I want to
move all of the ownership checking passes to run /before/ mandatory inlining. I
originally placed the passes after mandatory inlining since the function version
of the move keyword was transparent and needing to be inlined before we could
process it. Since we use the keyword now, that is no longer an issue.
This pass lowers moveonly-ness from the IR after we have finished move only
checking. The transform can be configured in two ways: such that it only handles
trivial types and such that it does trivial and non-trivial types. For ease of
use, I created two top level transforms (TrivialMoveOnlyTypeEliminator and
MoveOnlyTypeElimintor) that invoke the two behaviors by configuring the
underlying transform slightly differently.
For now, I am running first the trivial-only and then the all of the above
lowering. The trivial only pass will remain at this part of the pipeline
forever, but with time we are going to move the lower everything pass later into
the pipeline once I have audited the optimizer pipeline to just not perform any
work on move only types. That being said, currently we do not have this
guarantee and this patch at least improves the world and lets us codegen no
implicit copy code again.
The overall flow of the pass is:
1. We walk over the blocks summarizing the debug info instruction the blocks gen
as well as whether or not the block had an async funclet edge with in it.
2. We then perform a simple forward iterative optimistic dataflow using
intersection at merge points. At points where we find after merging that we have
a conflict and thus need to stop propagation, we insert a debug_value undef.
3. We then walk the CFG again visiting only blocks that we know had async
funclet edges. We then walk each said block from top to bottom starting with the
propagating gen information and updating as we go, dumping the current set of
debug_info we are tracking after each coroutine funclet boundary.
rdar://85020571
NOTE: debug_value [moved] appearing in the source code implies a _move was
used. So this will not effect current stable swift code.
This is just a first version of this that I am using to commit/bring up tests
for IRGen supporting a full dataflow version of this patch.
Big picture is that there is a bunch of work that is done in the LLVM level in
the coroutine splitter to work around communicating live variables in the
various coroutine func-lets. This logic is all done with debug.declare and we
would need to update that logic in the coroutine splitter to handle
debug.addr. Rather than do this, after some conversation, AdrianP and I realized
that we could get the same effect of a debug.declare by just redeclaring the
current live set of debug_value after each possible coroutine funclet start. To
do this in full generality, we need a full dataflow but just to bring this up we
initially perform a dominance propagation algorithm of the following sort:
1. We walk the CFG along successors. By doing this we guarantee that we visit
blocks after their dominators.
2. When we visit a block, we walk the block from start->end. During this walk:
a. We grab a new block state from the centralized block->blockState map. This
state is a [SILDebugVariable : DebugValueInst].
b. If we see a debug_value, we map blockState[debug_value.getDbgVar()] =
debug_value. This ensures that when we get to the bottom of the block, we
have pairs of SILDebugVariable + last debug_value on it.
c. If we see any coroutine funclet boundaries, we clone the current tracked
set of our block state and then walk up the dom tree dumping in each block
any debug_value with a SILDebugVariable that we have not already
dumped. This is maintained by using a visited set of SILDebugVariable for
each funclet boundary.
The end result is that at the beginning of each funclet we will basically
declare the debug info for an addr.
This is insufficient of course for moves that are in conditional control flow,
e.x.:
```
let x = Klass()
if boolValue {
await asyncCall()
let _ = _move(x)
}
```
but this at least lets me begin to write tests for this in lldb using straight
line code and work out the rest of the issues in CodeGen using those tests.
Merge the AddressLowering pass from its old development branch and update
it so we can begin incrementally enabling it under a flag.
This has been reimplemented for simplicity. There's no point in
looking at the old code.
Flow-isolation is a diagnostic SIL pass that finds
unsafe accesses to properties in initializers and
deinitializers that cannot gain isolation to otherwise
protect those accesses from concurrent modifications.
See SE-327 for more details about how and why it exists.
This commit includes changes and features like:
- The removal of the escaping-use restriction
- Flow-isolation that works properly with `defer` statements
- Flow-isolation with an emphasis on helpful diagnostics.
It also includes known issues like:
- Local / nonescaping functions are not analyzed by
flow-isolation, despite it being technically possible.
The main challenge in supporting it efficiently is that
such functions do not have a single exit-point, like
a `defer`. In particular, arbitrary functions can throw
so there are points where nonisolation should _not_ flow
out of the function at a call-site in the initializer, etc.
- The implementation of the flow-isolation pass is not
particularly memory efficient; it relies on BitDataflow
even though the particular flow problem is simple.
So, a more efficient implementation would be specialized for
this particular problem, etc.
There are also some changes to the Swift language itself: defer
will respect its context when deciding its property access kind.
Previously, a defer in an initializer would always access a stored
property through its accessor methods, instead of doing so directly
like its enclosing function might. This inconsistency is unfortunate,
so for Swift 6+ we make this consistent. For Swift 5, only a defer
in a function that is a member of the following kinds of types
will gain this consistency:
- an actor type
- any nominal type that is actor-isolated, excluding UnsafeGlobalActor.
These types are still rather new, so there is much less of a chance of
breaking expected behaviors around defer. In particular, the danger is
that users are relying on the behavior of defer triggering a property
observer within an init or deinit, when it would not be triggering it
without the defer.
To give a bit more information, currently the way the move function is
implemented is that:
1. SILGen emits a builtin "move" that is called within the function _move in the
stdlib.
2. Mandatory Inlining today if the final inlined type is address only, inlines
builtin "move" as mark_unresolved_move_addr. Otherwise, if the inlined type
is loadable, it performs a load [take] + move [diagnostic] + store [init].
3. In the diagnostic pipeline before any mem optimizations have run, we run the
move checker for addresses. This eliminates /all/ mark_unresolved_move_addr
as part of emitting diagnostics. In order to make this work, we perform a
small optimization before the checker runs that moves the
mark_unresolved_move_addr from being on temporary alloc_stacks to the true
base underlying address we are trying to move. This optimization is necessary
since _move is generic and often times SILGen will emit this temporary that
we do not want.
4. Then after we have run the guaranteed mem optimizations, we run the object
based move checker emitting diagnostics.
This PR changes the scheme above to the following:
1. SILGen emits a builtin "move" that is called within the function _move in the
stdlib.
2. Mandatory Inlining inlines builtin "move" as mark_unresolved_move_addr.
3. In the diagnostic pipeline before we have run any mem optimizations and
before we have run the actual move address checker, we massage the IR as we
do above but in a separate pass where in addition we try to match this pattern:
```
%temporary = alloc_stack $LoadableType
store %1 to [init] %temporary : $*LoadableType
mark_unresolved_move_addr %temporary to %otherAddr : $*LoadableType
destroy_addr %temporary : $*LoadableType
```
and transform it to:
```
%temporary = alloc_stack $LoadableType
%2 = move_value [allows_diagnostics] %1 : $*LoadableType
store %2 to [init] %temporary : $*LoadableType
destroy_addr %temporary : $*LoadableType
```
ensuring that the object move checker will handle this.
4. Then after we have run the guaranteed mem optimizations, we run the object
based move checker emitting diagnostics.
NOTE: This pass is disabled when -enable-experimental-lexical-lifetimes is
enabled.
When that flag is disabled, this removes the lexical flag from begin_borrow and
alloc_stack. This ensures that we can begin using begin_borrow [lexical] and
friends to emit diagnostics without impacting performance. I am going to be
preparing a subsequent patch that causes us to emit lexical lifetimes by
default. Due to this pass, I am not expecting any issues around perf.
This is just an initial prototype for people to play with. It is as always
behind the -enable-experimental-move-only flag.
NOTE: In this PR I implemented this only for 'local let' like things (local
lets/params). I did not implement in this PR support for local var and haven't
done anything with class ivars or globals.
rdar://83957028
NOTE: This is only available when the flag -enable-experimental-move-only. There
are no effects when the flag is disabled.
The way that this works is that it takes advantage of the following changes to
SILGen emission:
* When SILGen initializes a let with NoImplicitCopyAttribute, SILGen now emits
a begin_borrow [lexical] + copy + move_only. This is a pattern that we can check
and know that we are processing a move only value. When performing move
checking, we check move_only as a move only value and that it isn't consumed
multiple times.
* The first point works well for emitting all diagnostics except for
initializing an additional let var. To work around that I changed let
initialization to always bind to an owned value to a move of that owned
value. There is no semantic difference since that value is going to be consumed
by the binding operation anyways so we effectively just move the cleanup from
the original value we wanted to bind to the move. We still then actually borrow
the new let value with a begin_borrow [lexical] for the new let value. This
ensures that an initialization of a let value appears to be a consuming use to
the move only value checker while ensuring that the value has a proper
begin_borrow [lexical].
Some notes on functionality:
1. This attribute can only be applied to local 'let'.
2. "print" due to how we call it today with a vararg array is treated as a
consuming use (unfortunately).
3. I have not added the builtin copy operator yet, but I recently added a _move
skeleton attribute so one can end the lifetimes of these values early.
4. This supports all types that are not address only types (similar to
_move). To support full on address only types we need opaque values.
rdar://83957088
The PerformanceDiagnostics pass issues performance diagnostics for functions which are annotated with performance annotations, like @_noLocks, @_noAllocation.
This is done recursively for all functions which are called from performance-annotated functions.
rdar://83882635
The comment in LowerHopToActor explains the design here.
We want SILGen to emit hops to actors, ignoring executors,
because it's easier to fully optimize in a world where deriving
an executor is a non-trivial operation. But we also want something
prior to IRGen to lower the executor derivation because there are
useful static optimizations we can do, such as doing the derivation
exactly once on a dominance path and strength-reducing the derivation
(e.g. exploiting static knowledge that an actor is a default actor).
There are probably phase-ordering problems with doing this so late,
but hopefully they're restricted to situations like actors that
share an executor. We'll want to optimize that eventually, but
in the meantime, this unblocks the executor work.
The DiagnoseLifetimeIssuesPass pass prints a warning if an object is stored to a weak property (or is weakly captured) and destroyed before the property (or captured reference) is ever used again.
This can happen if the programmer relies on the lexical scope to keep an object alive, but copy-propagation can shrink the object's lifetime to its last use.
For example:
func test() {
let k = Klass()
// k is deallocated immediately after the closure capture (a store_weak).
functionWithClosure({ [weak k] in
// crash!
k!.foo()
})
}
Unfortunately this pass can only catch simple cases, but it's better than nothing.
rdar://73910632
* Redundant hop_to_executor elimination: if a hop_to_executor is dominated by another hop_to_executor with the same operand, it is eliminated:
hop_to_executor %a
... // no suspension points
hop_to_executor %a // can be eliminated
* Dead hop_to_executor elimination: if a hop_to_executor is not followed by any code which requires to run on its actor's executor, it is eliminated:
hop_to_executor %a
... // no instruction which require to run on %a
return
rdar://problem/70304809
to check for improperly nested '@_semantic' functions.
Add a missing @_semantics("array.init") in ArraySlice found by the
diagnostic.
Distinguish between array.init and array.init.empty.
Categorize the types of semantic functions by how they affect the
inliner and pass pipeline, and centralize this logic in
PerformanceInlinerUtils. The ultimate goal is to prevent inlining of
"Fundamental" @_semantics calls and @_effects calls until the late
pipeline where we can safely discard semantics. However, that requires
significant pipeline changes.
In the meantime, this change prevents the situation from getting worse
and makes the intention clear. However, it has no significant effect
on the pass pipeline and inliner.
The pass is already not being run during normal compilation scenarios today
since it bails on OSSA except in certain bit-rot situations where a test wasn't
updated and so was inadvertently invoking the pass. I discovered these while
originally just trying to eliminate the pass from the diagnostic pipeline. The
reason why I am doing this in one larger change is that I found there were a
bunch of sil tests inadvertently relying on guaranteed arc opts to eliminate
copy traffic. So, if I just removed this and did this in two steps, I would
basically be unoptimizing then re-optimizing the tests.
Some notes:
1. The new guaranteed arc opts is based off of SemanticARCOpts and runs only on
ossa. Specifically, in this new pass, we just perform simple
canonicalizations that do not involve any significant analysis. Some
examples: a copy_value all of whose uses are destroys. This will do what the
original pass did and more without more compile time. I did a conservative
first approximation, but we can probably tune this a bit.
2. the reason why I am doing this now is that I was trying to eliminate the
enable-ownership-stripping-after-serialization flag and discovered that the
test opaque_value_mandatory implicitly depends on this since sil-opt by
default was the only place left in the compiler with that option set to false
by default. So I am eliminating that dependency before I land the larger
change.
This simplifies the handling of the subdirectories in the SIL and
SILOptimizer paths. Create individual libraries as object libraries
which allows the analysis of the source changes to be limited in scope.
Because these are object libraries, this has 0 overhead compared to the
previous implementation. However, string operations over the filenames
are avoided. The cost for this is that any new sub-library needs to be
added into the list rather than added with the special local function.
The differentiation transform does the following:
- Canonicalizes differentiability witnesses by filling in missing derivative
function entries.
- Canonicalizes `differentiable_function` instructions by filling in missing
derivative function operands.
- If necessary, performs automatic differentiation: generating derivative
functions for original functions.
- When encountering non-differentiability code, produces a diagnostic and
errors out.
Partially resolves TF-1211: add the main canonicalization loop.
To incrementally stage changes, derivative functions are currently created
with empty bodies that fatal error with a nice message.
Derivative emitters will be upstreamed separately.
