I've also fixed this so that it should work on instructions that
define multiple values. Someday we'll change all the open_existential
instructions to produce different values for the type dependency and
the value result; today is not that day, though.
Add TermInst::forwardedOperand.
Add SILArgument::forwardedTerminatorResultOperand. This API will be
moved into a proper TerminatorResult abstraction.
Remove getSingleTerminatorOperand, which could be misused because it's
not necessarilly forwarding ownership.
Remove the isTransformationTerminator API, which is not useful or well
defined.
Rewrite several instances of complex logic to handle block arguments
with the simple terminator result API. This defines away potential
bugs where we don't detect casts that perform implicit conversion.
Replace uses of the SILPhiArgument type and code that explicitly
handle block arguments. Control flow is irrelevant in these
situations. SILPhiArgument needs to be deleted ASAP. Instead, use
simple APIs like SILArgument::isTerminatorResult(). Eventually this
will be replaced by a TerminatorResult type.
Add `deletableInstructions()` and `reverseDeletableInstructions()` in SILBasicBlock.
It allows deleting instructions while iterating over all instructions of the block.
This is a replacement for `InstructionDeleter::updatingRange()`.
It's a simpler implementation than the existing `UpdatingListIterator` and `UpdatingInstructionIteratorRegistry`, because it just needs to keep the prev/next pointers for "deleted" instructions instead of the iterator-registration machinery.
It's also safer, because it doesn't require to delete instructions via a specific instance of an InstructionDeleter (which can be missed easily).
Thanks to the invariants of store_borrow, rewriting a store_borrow is a
simple matter of replacing its (non end_borrow) uses with uses of the
underlying address-only value whose use was stored.
When finding the PrunedLiveness on whose boundary end_borrows are to be
inserted, allow lifetime ending uses if they are end_borrows. Such
end_borrows appear validly when an inner use is a nested begin_borrow.
The begin_apply instruction introduces storage which is only valid until
the coroutine is ended via end_apply/abort_apply. Previously,
AddressLowering assumed the memory was valid "as long as it needed to
be": either as an argument to the function being lowered (so valid for
the whole lifetime) or as a new alloc_stack (whose lifetime is
determined by AddressLowering itself). The result was that the storage
made available by the begin_apply was used after the
end_apply/abort_apply when there were uses of [a copy of] [a projection
of] the yield after the end_apply/abort_apply.
Here, the behavior is fixed. Now, the storage is used "as soon as
possible". There are two cases:
(1) If an indirect value's ownership is transferred into the
caller (i.e. its convention is `@in` or `@in_constant`), the value is
`copy_addr [take]`n into caller-local storage when lowering the
begin_apply.
(2) If an indirect value's ownership is only lent to the caller (i.e.
its convention is `@in_guaranteed`), no equivalent operation could be
done: there's no `copy_addr [borrow]`; on the other hand, there's no
need to do it--uses of the value must have occurred within the
coroutine's range. Instead, it's necessary to disable the store-copy
optimization in this case: storage must be allocated for a copy of [a
projection of] the yielded value.
It is valid for owned values not to be destroyed on paths terminating in
unreachable. Similarly, it is valid for storage not to be deallocated
on paths terminating in unreachable. However, it is _not_ valid for
storage to be deallocated before being deinitialized, even in paths
terminating in unreachable. Consequently, when AddressLowering,
dealloc_stacks must not be created in dead-end blocks: because the input
OSSA may not destroy an owned value, the output may not deinitialize
owned storage; so a dealloc_stack in an unreachable block could dealloc
storage which was not deinitialized.
This just allows me to sink busy code into helpers on the state making the
implementation easier to understand. It also gave me a good place to add
comments that will make it clearer what we are actually doing.
NFCI.
During storage allocation, create all alloc_stacks at the front of the
function entry block, except those for opened existential types. Do not
create any dealloc_stacks, even for opened existential types.
Then, after function rewriting, position the alloc_stacks according to
uses, except those for opened existential types. This means allocating
in the block that's the least common ancestor of all uses. At this
point, create dealloc_stacks on the dominance frontier of the
alloc_stacks, even for allocations for opened existential types.
The behavior for opened existential types diverges from all others in
order to maintain SIL validity (as much as possible) while the pass
runs. It would be invalid to create the alloc_stacks for opened
existential types at the front of the entry block because the type which
is opened is not yet available, but that type's opening must dominate
the corresponding alloc_stack.
On Windows CI builds, we have observed a failure to build the Swift
Standard Library after 10/27 (first noticed on 11/11 snapshot due to
other failures). The failure is an invalid `isa` cast where the
instance is a `nullptr`. The SILPipeline suggests that the
SILOptimizer might be the one triggering the incorrect use of `isa`.
The only instance of `isa` introduced in that range in the
SILOptimizer/Mandatory region for that duration is this particular
instance. Tighten the assertion to ensure that `oper->getUser()`
returns a non-`nullptr` value.
Thanks for @gwynne for helping narrow down the search area.
DefiniteInitialization used a running SILBuilder to insert all instructions and
didn't consistently set the scope to a meaningful value. This patch replaces all
uses of the running Builder with ad-hoc instances of SILBuilderWithScope which
should capture the intention of the code better.
rdar://102296138
