Improves OSSALifetimeCompletion to handle trivial variables when running from
SILGenCleanup. This only affects lifetime dependence diagnostics.
For the purpose of lifetime diagnostics, trivial local variables are only valid
within their lexical scope. Sadly, SILGen only know how to insert cleanup code
on normal function exits. SILGenCleanup relies on lifetime completion to fix
lifetimes on dead end paths.
%var = move_value [var_decl]
try_apply %f() : $..., normal bb1, error error
error:
extend_lifetime %var <=== insert this
unreachable
This allows Span to depend on local unsafe pointers AND be used within
throwing closures:
_ = a.withUnsafeBufferPointer {
let buffer = $0
let view = Span(_unsafeElements: buffer)
return view.withUnsafeBufferPointer(\.count)
}
For a function to have complete lifetimes, the lifetime of every def in
the function which is backwards-reachable from a dead-end block must be
completed.
Previously, every def in every block which appears in the post-order of
the function's blocks is completed. This was not enough: defs in
"unreachable blocks" (i.e. those which aren't forwards-reachable from
the function entry) must be completed too, and such blocks do not appear
in the function's post-order.
Here, defs in unreachable blocks are be completed too. Such defs must
also be completed in relative post-order. To do this, roots for the
non-entry post-orders must be found.
rdar://141197164
Now that the two known issues which resulted in invalid SIL being
provided to lifetime completion have been addressed, tighten up the
completion done on the availability boundary not to allow leaks.
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)
Presently under -enable-ossa-complete-lifetimes.
This allows SILGen to skip OSSA cleanups, for example at dead-end
blocks.
Long term, we may remove OSSA cleanups from SILGen entirely (except
for lexical borrow scopes). This changes lets us experiment with that
option.
I recently have been running into the issue that many of these APIs perform the
deletion operation themselves and notify the caller it is going to delete
instead of allowing the caller to specify how the instruction is deleted. This
causes interesting semantic issues (see the loop in deleteInstruction I
simplified) and breaks composition since many parts of the optimizer use
InstModCallbacks for this purpose.
To fix this, I added a notify will be deleted construct to InstModCallback. In a
similar way to the rest of it, if the notify is not set, we do not call any code
implying that we should have good predictable performance in loops since we will
always skip the function call.
I also changed InstModCallback::deleteInst() to notify before deleting so we
have a default safe behavior. All previous use sites of this API do not care
about being notified and the only new use sites of this API are in
InstructionDeleter that perform special notification behavior (it notifies for
certain sets of instructions it is going to delete before it deletes any of
them). To work around this, I added a bool to deleteInst to control this
behavior and defaulted to notifying. This should ensure that all other use sites
still compose correctly.
Specifically, given code like the following:
```
(%1, %2) = multi_result_inst %0 (a)
inst_to_delete %1 (b)
inst_to_delete %2 (c)
```
We would sometimes visit (c) before (b). So if nextII was (b) at that point, we
would think that we were safe. Then we process (b), move nextII from (b) ->
(c). Then we delete both (b) and (c). =><=.
The solution to this is each iteration of the delete loop, we track /all/
instructions that eliminateDeadInstruction is going to eliminate and ensure that
after each callback is called we have moved past all visited instructions (even
ones we visited previously). This is done using a small set that I clear each
iteration.
I also re-enabled the dictionary test that exposed this issue when testing
-O/-Osize.
rdar://71933996
This is a generic API that when ownership is enabled allows one to replace all
uses of a value with a value with a differing ownership by transforming/lifetime
extending as appropriate.
This API supports all pairings of ownership /except/ replacing a value with
OwnershipKind::None with a value without OwnershipKind::None. This is a more
complex optimization that we do not support today. As a result, we include on
our state struct a helper routine that callers can use to know if the two values
that they want to process can be handled by the algorithm.
My moticiation is to use this to to update InstSimplify and SILCombiner in a
less bug prone way rather than just turn stuff off.
Noting that this transformation inserts ownership instructions, I have made sure
to test this API in two ways:
1. With Mandatory Combiner alone (to make sure it works period).
2. With Mandatory Combiner + Semantic ARC Opts to make sure that we can
eliminate the extra ownership instructions it inserts.
As one can see from the tests, the optimizer today is able to handle all of
these transforms except one conditional case where I need to eliminate a dead
phi arg. I have a separate branch that hits that today but I have exposed unsafe
behavior in ClosureLifetimeFixup that I need to fix first before I can land
that. I don't want that to stop this PR since I think the current low level ARC
optimizer may be able to help me here since this is a simple transform it does
all of the time.
and eliminate dead code. This is meant to be a replacement for the utility:
recursivelyDeleteTriviallyDeadInstructions. The new utility performs more aggresive
dead-code elimination for ownership SIL.
This patch also migrates most non-force-delete uses of
recursivelyDeleteTriviallyDeadInstructions to the new utility.
and migrates one force-delete use of recursivelyDeleteTriviallyDeadInstructions
(in IRGenPrepare) to use the new utility.
The XXOptUtils.h convention is already established and parallels
the SIL/XXUtils convention.
New:
- InstOptUtils.h
- CFGOptUtils.h
- BasicBlockOptUtils.h
- ValueLifetime.h
Removed:
- Local.h
- Two conflicting CFG.h files
This reorganization is helpful before I introduce more
utilities for block cloning similar to SinkAddressProjections.
Move the control flow utilies out of Local.h, which was an
unreadable, unprincipled mess. Rename it to InstOptUtils.h, and
confine it to small APIs for working with individual instructions.
These are the optimizer's additions to /SIL/InstUtils.h.
Rename CFG.h to CFGOptUtils.h and remove the one in /Analysis. Now
there is only SIL/CFG.h, resolving the naming conflict within the
swift project (this has always been a problem for source tools). Limit
this header to low-level APIs for working with branches and CFG edges.
Add BasicBlockOptUtils.h for block level transforms (it makes me sad
that I can't use BBOptUtils.h, but SIL already has
BasicBlockUtils.h). These are larger APIs for cloning or removing
whole blocks.
