OSSA lifetime canonicalization can take a very long time in certain
cases in which there are large basic blocks. to mitigate this, add logic
to skip walking the liveness boundary for extending liveness to dead
ends when there aren't any dead ends in the function.
Updates `DeadEndBlocks` with a new `isEmpty` method and cache to
determine if there are any dead-end blocks in a given function.
When the utility is used by the ConsumeOperatorCopyableValuesChecker,
the checker guarantees that the lifetime can end at the consumes, that
there are no uses after those consumes. In that circumstance, the
utility maintains liveness to those consumes and as far as possible
without introducing a copy everywhere else.
The lack of complete lifetimes has forced the utility to extend liveness
of values to dead-ends. That extension, however, is in tension with the
use that the checker is putting the utility to. If there is a dead-end
after a consume, liveness must not be maintained to that dead-end.
rdar://147586673
The logic here got confused over time. This simplifies the logic and ensures
that we do not send a value if it is in the same isolation domain as the callee.
The one interesting side effect of this is that in a few tests, due to the logic
being confused, we were emitting use-after-send errors for global actor isolated
values that were passed to a function that was global actor isolated to the same
actor and then used later locally. The error was sending 'X'-isolated a to
'X'-isolated function causes race against nonisolated local uses. In truth, this
error is misleading and the only error that we should be emitting in such a case
is the error about moving an isolated value into a non-isolated context (which
we already emit).
rdar://132932382
Otherwise, we can be inconsistent with isolations returned by other parts of the
code. Previously we were just treating it always as self + nom decl, which is
clearly wrong if a type is not self (e.x.: if it is an isolated parameter).
rdar://135459885
I am doing this so I can mark requires as being on a mutable non-Sendable base
from a Sendable value.
I also took this as an opportunity to compress the size of PartitionOp to be 24
bytes instead of 40 bytes.
Previously, when we saw any Sendable type and attempted to look up an underlying
tracked value, we just bailed. This caused an issue in situations like the
following where we need to emit an error:
```swift
func test() {
var x = 5
Task.detached { x += 1 }
print(x)
}
```
The problem with the above example is that despite value in x being Sendable,
'x' is actually in a non-Sendable box. We are passing that non-Sendable box into
the detached task by reference causing a race against the read from the
non-Sendable box later in the function. In SE-0414, this is explicitly banned in
the section called "Accessing Sendable fields of non-Sendable types after weak
transferring". In this example, the box is the non-Sendable type and the value
stored in the box is the Sendable field.
To properly represent this, we need to change how the underlying object part of
our layering returns underlying objects and vends TrackableValues to the actual
analysis for addresses. NOTE: We leave the current behavior alone for SIL
objects.
By doing this, in situations like the above, despite have a Sendable value (the
integer), we are able to ensure that we require that the non-Sendable box
containing the integer is not used after we have sent it into the other Task
despite us not actually using the box directly.
Below I describe the representation change in more detail and describe the
various cases here. In this commit, I only change the representation and do not
actually use the new base information. I do that in the next commit to make this
change easier for others to read and review. I made sure that change was NFC by
leaving RegionAnalysis.cpp:727 returning an optional.none if the value found was
a Sendable value.
----
The way we modify the representation is that we instead of just returning a
single TrackedValue return a pair of tracked values, one for the base and one
for the "value". We return this pair in what is labeled a
"TrackableValueLookupResult":
```c++
struct TrackableValueLookupResult {
TrackableValue value;
std::optional<TrackableValue> base;
TrackableValueLookupResult(TrackableValue value)
: value(value), base() {}
TrackableValueLookupResult(TrackableValue value, TrackableValue base)
: value(value), base(base) {}
};
```
In the case where we are accessing a projection path out of a non-Sendable type
that contains all non-Sendable fields, we do not do anything different than we
did previously. We just walk up from use->def until we find the access path base
which we use as the representative of the leaf of the chain and return
TrackableValueLookupResult(access path base).
In the case where we are accessing a Sendable leaf type projected from a
non-Sendable base, we store the leaf type as our value and return the actual
non-Sendable base in TrackableValueLookupResult. Importantly this ensures that
even though our Sendable value will be ignored by the rest of the analysis, the
rest of the analysis will ensure that our base is required if our base is a var
that had been escaped into a closure by reference.
In the case where we are accessing a non-Sendable leaf type projected from a
Sendable type (which we may have continued to be projected subsequently out of
additional Sendable types or a non-Sendable type), we make the last type on the
projection path before the Sendable type, the value of the leaf type. We return
the eventual access path base as our underlying value base. The logic here is
that since we are dealing with access paths, our access path can only consist of
projections into a recursive value type (e.x.: struct/tuple/enum... never a
class). The minute that we hit a pointer or a class, we will no longer be along
the access path since we will be traversing a non-contiguous piece of
memory (consider a class vs the class's storage) and the traversal from use->def
will stop. Thus, we know that there are only two ways we can get a field in that
value type to be Sendable and have a non-Sendable field:
1. The struct can be @unchecked Sendable. In such a case, we want to treat the
leaf field as part of its own disconnected region.
2. The struct can be global actor isolated. In such a case, we want to treat the
leaf field as part of the global actor's region rather than whatever actor.
The reason why we return the eventual access path base as our tracked value base
is that we want to ensure that if the var value had been escaped by reference,
we can require that the var not be sent since we are going to attempt to access
state from the var in order to get the global actor guarded struct that we are
going to attempt to extract our non-Sendable leaf value out of.
Addressable parameters must remain indirect.
Incidentally also fixes an obvious latent bug in which all specialization was
disabled if any metatypes could not be specialized.
Fixes rdar://145687827 (Crash of inline-stored Span properties with optimizations)
The body of a function has to be re-analyzed for every call
site of the function, which is very expensive and if the
body is not changed would produce the same result.
This takes about ~10% from swift-syntax overall build time
in release configuration.
Regardless of consumes of copies, if the original lexical value is not
consumed on a dead-end path, it must remain live to that dead-end.
rdar://145226757
From talking with @dgregor, it became clear that this comment was easily
interpreted as saying that AssignFresh always introduced a disconnected value...
which is not the case. Instead, AssignFresh just introduces a new value that
could have any form of isolation. The actual isolation of the value is assigned
via tryToTrackValue and eventually SILIsolationInfo::get().
CSE uses OSSA rauw which creates copies and copies that are created to optimize
across borrow scopes are unoptimizable. This PR avoids this situation for now.
executing unknown code
This means we have to claw back some performance by recognizing harmless
releases.
Such as releases on types we known don't call a deinit with unknown
side-effects.
rdar://143497196
rdar://143141695
It's sufficient just to have a struct with a kind and a value. There
aren't any cases where the payload's original type benefits from being
statically preserved--they're only ever obtained as `SILValue`s. Keep
the type safety by way of constructors.
The reason why I am doing this is that in certain cases the AST captures indices
will never actually line up with partial apply capture indices since we seem to
"smush" together closures and locally defined functions.
NOTE: The reason for the really small amount of test changes is that this change
does not change the actual output by design. The only cases I had to change were
a case where we began to emit a better diagnostic and also where I added code
coverage around _ and let _ since those require ignored_use to be implemented so
that they would be diagnosed (previously we just did not emit anything so we
couldn't emit the diagnostic at the SIL level).
rdar://142661388
Don't use previously found owned concrete values in ossa. They will consumed by
forwarding operations like init_existential_ref. Instead create an unconditional cast
of the opened existential to concrete type and use that to create a concrete apply.
This is needed after running the SSAUpdater for an existing OSSA value, because the updater can
insert unnecessary phis in the middle of the original liverange which breaks up the original
liverange into smaller ones:
```
%1 = def_of_owned_value
%2 = begin_borrow %1
...
br bb2(%1)
bb2(%3 : @owned $T): // inserted by SSAUpdater
...
end_borrow %2 // use after end-of-lifetime!
destroy_value %3
```
It's not needed to run this utility if SSAUpdater is used to create a _new_ OSSA liverange.
In preparation for only recording the defs once, replace the
GraphNodeWorklist of defs with a SetVector. Preserve the current
visitation order by creating a worklist of indices to be visited.
Add a type which distinguishes among the types of defs that are pushed
onto the "def-use worklist". Note that it's not possible to rely on the
kind of value because the root may itself be a copy_value. For now, the
distinction is discarded as soon as the def is visited.
In terms of the test suite the only difference is that we allow for non-Sendable
types to be returned from nonisolated functions. This is safe due to the rules
of rbi. We do still error when we return non-Sendable functions across isolation
boundaries though.
The reason that I am doing this now is that I am implementing a prototype that
allows for nonisolated functions to inherit isolation from their caller. This
would have required me to implement support both in Sema for results and
arguments in SIL. Rather than implement results in Sema, I just finished the
work of transitioning the result checking out of Sema and into SIL. The actual
prototype will land in a subsequent change.
rdar://127477211
