An instruction can consume multiple (discontiguous) fields. Use a
SmallBitVector to track the fields consumed by an instruction rather
than a TypeTreeLeafRange.
rdar://125103951
Previously, whenever an instruction was recorded as a final consume, a
new entry was added to finalBlockConsumes. Here, this is changed to add
the new bits being consumed by the instruction to the preexisting
SmallBitVector, if there is one.
A `try_apply` with indirect out arguments is only a def for those arguments on
the success path. Model this by sinking the def-ness of the instruction into the
success branch of the try_apply, and introducing a new `DeadToLiveEdge` mode for
block liveness which stops propagation of use-before-def conditions into the
block that introduced the def. Fixes rdar://118567869.
The comment stated that this was used to turn switches into consumes (which
is no longer relevant), but it looks like it is also necessary for the
move-only address checker to be able to properly understand partial consumes
of fields in some code patterns. Now that we surround borrowing switches
in plenty of opaque accesses to protect them from being pried apart by the
move checker, it should be safe to reenable this.
For years, optimizer engineers have been hitting a common bug caused by passes
assuming all SILValues have a parent function only to be surprised by SILUndef.
Generally we see SILUndef not that often so we see this come up later in
testing. This patch eliminates that problem by making SILUndef uniqued at the
function level instead of the module level. This ensures that it makes sense for
SILUndef to have a parent function, eliminating this possibility since we can
define an API to get its parent function.
rdar://123484595
This prevents the move-only checker from trying to analyze the bindings
as partial consumptions, which ought to be unnecessary since SILGen
will always fully consume the subject as part of forming the bindings.
Fill in a missing path for destructuring loadable elements from
address-only tuples in a borrowing context. Enclose projections in their
own separate accesses so that they are analyzed independently by the
move checker.
`unchecked_take_enum_data_addr` should not be considered a write when it's non-
destructive; this should eventually be an inherent property of the instruction,
but there are other passes which miscompile currently if we change that now.
Meanwhile, wrapping a copyable value should always be considered an initialization
too.
To avoid dialecticization based on compilation mode, ban for
non-resilient modules partial consumption of aggregates which would be
illegal were those modules instead resilient.
Relax some existing pattern matches and add some unhandled instructions to the
walkers so that borrowing switches over address-only enums are properly analyzed
for incorrect consumption. Add a `[strict]` flag to `mark_unresolved_move_only_value`
to indicate a borrow access that should remain a borrow access even if the subject
is later stack-promoted from a box.
When extending liveness, the instructions prior to a preexisting destroy
are added to liveness. If that prior instruction is the def, adding it
to liveness results in multi-def liveness understanding that there is a
prior def.
Fixes a bug exposed by adding coroutine ends to liveness rather than
function ends.
Use the same code path to add users at which an address must have been
reinitialized for both kinds of mark_unresolved_non_copyable_value which
permit assignment.
There are several kinds of scopes at which it is required that an
address be initialized:
(1) the whole function -- for inout argument to the function
(2) the region a coroutine is active -- for an inout yielded by a
coroutine into the function
(3) the region of a memory access -- for a `begin_access [modify]`.
The move checker enforces that they are initialized at that point by
adding instructions at which the field must be live to liveness.
Previously, all such scopes used the end of the function as the point at
which the memory had to have been reinitialized. Here, the relevant end
of scope markers are used instead.
More importantly, here the diagnostic is made to vary--the diagnostic,
that is, that is issued in the face an address not being initialized at
the end of these different kind of scopes.
Mark the result of a move-only addressor as unresolved. The pointed-at value
cannot be consumed so ensure that only [read] or [modify] accesses are
performed. Update the move-only checker to recognize code patterns
from addressors.
Following https://github.com/apple/swift/pull/70333, do the same thing for
modify coroutines, marking the result so that we check uses of the result to
ensure it isn't consumed (without being reinitialized).
Mark the result of starting a read coroutine to be checked by the move-only checker, and then
update the pattern matching in the move checker itself so that it recognizes code patterns
involving yielding from and receiving yields from read coroutines. Teach move only diagnostics
to get the property name for an access through a read coroutine from the referenced declaration.
When a address-only noncopyable value is dead-def'ed by an indirect return from a `try_apply`,
the cleanup should be inserted on the normal return successor block. Fixes rdar://118255228.
And use it in lifetime maximization.
The preexisting member function updateForUse has been updated to match
PrunedLiveness and gravitate towards lifetimeEnding=false.
For a fixed instruction and bit, if called with lifetimeEnding=true and
then lifetimeEnding=false, the lifetime-ending-ness of the instruction
at the bit will be false; and if it is again called with
lifetimeEnding=true, the lifetime-ending-ness of the instruction at the
bit will remain false.
In contrast the new member function extendToUse does not alter the
lifetime-ending-ness if it is already set. If it is unset, the function
sets the bit to lifetimeEnding=false.
I was originally hoping to reuse mark_must_check for multiple types of checkers.
In practice, this is not what happened... so giving it a name specifically to do
with non copyable types makes more sense and makes the code clearer.
Just a pure rename.
Also, the store_borrow work in the previous patch caused some additional issues
to crop up. I fixed them in this PR and added some tests in the process.
