A begin_apply token may be used by operands that do not end the coroutine:
mark_dependence.
We need an API that gives us only the coroutine-ending uses. This blocks
~Escapable accessors.
end_borrow is considered coroutine-ending even though it does not actually
terminate the coroutine.
We cannot simply ask isLifetimeEnding, because end_apply and abort_apply do not
end any lifetime.
The main changes are:
*) Rewrite everything in swift. So far, parts of memory-behavior analysis were already implemented in swift. Now everything is done in swift and lives in `AliasAnalysis.swift`. This is a big code simplification.
*) Support many more instructions in the memory-behavior analysis - especially OSSA instructions, like `begin_borrow`, `end_borrow`, `store_borrow`, `load_borrow`. The computation of end_borrow effects is now much more precise. Also, partial_apply is now handled more precisely.
*) Simplify and reduce type-based alias analysis (TBAA). The complexity of the old TBAA comes from old days where the language and SIL didn't have strict aliasing and exclusivity rules (e.g. for inout arguments). Now TBAA is only needed for code using unsafe pointers. The new TBAA handles this - and not more. Note that TBAA for classes is already done in `AccessBase.isDistinct`.
*) Handle aliasing in `begin_access [modify]` scopes. We already supported truly immutable scopes like `begin_access [read]` or `ref_element_addr [immutable]`. For `begin_access [modify]` we know that there are no other reads or writes to the access-address within the scope.
*) Don't cache memory-behavior results. It turned out that the hit-miss rate was pretty bad (~ 1:7). The overhead of the cache lookup took as long as recomputing the memory behavior.
A guaranteed function argument is live for the duration of the function.
It should be safe to allow TempRValueOpt when it is the base of the copy source of a lexical alloc_stack.
Create two versions of the following functions:
isConsumedParameter
isGuaranteedParameter
SILParameterInfo::isConsumed
SILParameterInfo::isGuaranteed
SILArgumentConvention::isOwnedConvention
SILArgumentConvention::isGuaranteedConvention
These changes will be needed when we add a new convention for
non-trivial C++ types as the functions will return different answers
depending on whether they are called for the caller or the callee. This
commit doesn't change any functionality.
Although I don't plan to bring over new assertions wholesale
into the current qualification branch, it's entirely possible
that various minor changes in main will use the new assertions;
having this basic support in the release branch will simplify that.
(This is why I'm adding the includes as a separate pass from
rewriting the individual assertions)
In preparation for inserting mark_dependence instructions for lifetime
dependencies early, immediately after SILGen. That will simplify the
implementation of borrowed arguments.
Marking them unresolved is needed to make OSSA verification
conservative until lifetime dependence diagnostics runs.
The dependent 'value' may be marked 'nonescaping', which guarantees that the
lifetime dependence is statically enforceable. In this case, the compiler
must be able to follow all values forwarded from the dependent 'value', and
recognize all final (non-forwarded, non-escaping) use points. This implies
that `findPointerEscape` is false. A diagnostic pass checks that the
incoming SIL to verify that these use points are all initially within the
'base' lifetime. Regular 'mark_dependence' semantics ensure that
optimizations cannot violate the lifetime dependence after diagnostics.
To avoid introducing new copies--which is illegal for move-only values--
don't rewrite `load [take]`s and `copy_addr [take]`s as `load [copy]`s
and `copy_addr`s respectively and introduce new `destroy_addr`s after
them. Instead, get the effect of folding such a newly created
`destroy_addr` into the preceding rewritten `load [copy]` or
`copy_addr`. Get that effect by neither modifying the `copy_addr [take]`
or `load [take]` nor adding a subsequent `destroy_addr`.
An example for each kind (`load [take]` and `copy_addr [take]`):
```
// Input 1 (`load [take]`)
copy_addr [take] %src to [init] %tmp
%val = load [take] %src
// Old Output 1
%val = load [copy] %src
destroy_addr %src
// New Output 2
%val = load [take] %src
```
```
// Input 2 (`copy_addr [take]`)
copy_addr [take] %src to [init] %tmp
copy_addr [take] %src to [init] %dst
// Old Output 2
copy_addr %src to [init] %dst
destroy_addr %src
// New Output 2
copy_addr [take] %src to [init] %dst
```
rdar://107839979
In preparation for "folding" an "inserted destroy" into a load [copy] or
copy_addr, rename the variable that indicates whether the copyInst's
source must be deinitialized after its last "load".
Previously, whenever an alloc_stack [lexical] was seen, optimization
bailed conservatively.
In the fullness of time, we should optimize an alloc_stack [lexical] so
long as:
(1) the root of the source of the store/copy_addr is lexical
(2) the range in which the alloc_stack [lexical] contains the value
store'd/copy_addr'd into it (i.e. the range ending at
destroy_addr/etc) is contained within the live range of that lexical
root
Here, an incremental step in that direction is taken: if the base of the
accessed storage of the source of a copy_addr is a guaranteed function
argument, then we know
(1) the base is lexical--guaranteed function arguments are always
lexical
(2) the range in which the alloc_stack contains the value copy_addr'd in
is trivially contained within the range of that lexical root--the
live range of a guaranteed function argument is the whole function
Added TODOs for the full optimization.
When optimizing an enum `store` to an `alloc_stack`, require that all uses are in the same block.
Otherwise it could be a `switch_enum` of an optional where the none-case does not have a destroy of the enum value.
After transforming such an `alloc_stack`, the value would leak in the none-case block.
Fixes a OSSA verification error.
Previously, TempRValueElimination would peephole simple alloc_stacks,
even when they were lexical; here, they are left for Mem2Reg to properly
handle.
Previously, SemanticARCOpts would eliminate lexical begin_borrows,
incorrectly allowing the lifetime of the value borrowed by them to be
observably shortened. Here, those borrow scopes are not eliminated if
they are lexical.
Added an executable test that verifies that a local variable strongly
referencing a delegate object keeps that delegate alive through the call
to an object that weakly references the delegate and calls out to it.
Instead of caching alias results globally for the module, make AliasAnalysis a FunctionAnalysisBase which caches the alias results per function.
Why?
* So far the result caches could only grow. They were reset when they reached a certain size. This was not ideal. Now, they are invalidated whenever the function changes.
* It was not possible to actually invalidate an alias analysis result. This is required, for example in TempRValueOpt and TempLValueOpt (so far it was done manually with invalidateInstruction).
* Type based alias analysis results were also cached for the whole module, while it is actually dependent on the function, because it depends on the function's resilience expansion. This was a potential bug.
I also added a new PassManager API to directly get a function-base analysis:
getAnalysis(SILFunction *f)
The second change of this commit is the removal of the instruction-index indirection for the cache keys. Now the cache keys directly work on instruction pointers instead of instruction indices. This reduces the number of hash table lookups for a cache lookup from 3 to 1.
This indirection was needed to avoid dangling instruction pointers in the cache keys. But this is not needed anymore, because of the new delayed instruction deletion mechanism.
Without this when constructing an InstModCallback it is hard to distinguish
which closure is meant for which operation when passed to the constructor of
InstModCallback (if this was in Swift, we could use argument labels, but we do
not have such things in c++).
This new value type sort of formulation makes it unambiguous which callback is
used for what when constructing one of these.
Don't move an end_access over a (non-aliasing) end_access. This would destroy the proper nesting of accesses.
Also, add some comments, asserts and tests.
Try to move an end_access down to extend the access scope over all uses of the temporary.
For example:
%a = begin_access %src
copy_addr %a to [initialization] %temp : $*T
end_access %a
use %temp
We must not replace %temp with %a after the end_access. Instead we try to move the end_access after "use %temp".
This fixes generation of invalid SIL and/or the invalid removal of access checks.
I am trying to be more careful about this rather than letting someone make this
mistake in the future. I also added some comments and a DeadEndBlocks instance
to TempRValueElimination so that it gets full simplifications in OSSA.
Currently all of these places in the code base perform simplifyInstruction and
then a replaceAllSimplifiedUsesAndErase(...). This is a bad pattern since:
1. simplifyInstruction assumes its result will be passed to
replaceAllSimplifiedUsesAndErase. So by leaving these as separate things, we
allow for users to pointlessly make this mistake.
2. I am going to implement in a subsequent commit a utility that lifetime
extends interior pointer bases when replacing an address with an interior
pointer derived address. To do this efficiently, I want to reuse state I
compute during simplifyInstruction during the actual RAUW meaning that if the
two operations are split, that is difficult without extending the API. So by
removing this, I can make the transform and eliminate mistakes at the same
time.
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.
When instructions are changed within a pass in a way that affects subsequent alias queries in the same pass run,
their alias analysis information must be invalidated.
Otherwise it can result in miscompiles and/or invalid SIL.
rdar://71924430
Instead of reusing the existing destroy_addr (or load/copy_addr [take]) of the temporary, insert a new destroy_addr - at the correct location.
This fixes a memory lifetime failure in case the copy-source is modified (actually: re-initialized) after the last use of the temporary: E.g. (before copy elimination):
copy_addr [take] %src to [initialization] %temp
%x = load %temp // last use of %temp
store %y to [init] %src
destroy_addr %temp
The correct location for the destroy_addr is after the last use (after copy elimination):
%x = load %src
destroy_addr %src
store %y to [init] %src
rdar://problem/69757314
load [take] was not considered as a use and it was not detected if it's in a different basic block.
This fixes a miscompile in case there is a modification of the copy-source before a load [take].
rdar://problem/69757314
Consider the related end_access instructions as uses to correctly mark the end of the lifetime of the temporary.
This fixes a miscompile in case there is a modification of the copy-source between an begin_access and end_access.
... where it belongs.
This is mostly refactoring, but it also fixes a bug: we don't recurse into a begin_access in collectLoads.
If there is an apply in such a scope, the mayWrite-check wouldn't be done.
In checkNoSourceModification all instructions are visited, so the check is always done.
