Ownership rauw uses a shared ownership fixup context to maintain state.
When ownership rauw fails, due to some invalid condition, we leave
behind stale data in this shared ownership fixup context.
This stale context can indvertantly affect the next rauw on addresses.
In addition to setting the ownership fixup context to nullptr, we
should also clear it so that it's internal data structures are
cleared.
OSSA rauw cleans up end of scope markers before rauw'ing.
This can lead to inadvertant deleting of end_lifetime, later
resulting in an ownership verifier error indicating a leak.
This PR stops treating end_lifetime scope ending like end_borrow/end_access.
SROA and Mem2Reg now can leverage DIExpression -- op_fragment, more
specifically -- to generate correct debug info for optimized SIL. Some
important highlights:
- The new swift::salvageDebugInfo, similar to llvm::salvageDebugInfo,
tries to restore / transfer debug info from a deleted instruction.
Currently I only implemented this for store instruction whose
destination is an alloc_stack value.
- Since we now have source-variable-specific SIL location inside a
`debug_value` instruction (and its friends), this patch teaches
SILCloner and SILInliner to remap the debug scope there in addition
to debug scope of the instruction.
- DCE now does not remove `debug_value` instruction whose associating
with a function argument SSA value that is not used elsewhere. Since
that SSA value will not disappear so we should keep the debug info.
This rewrites functionality that was mostly disabled but is now ready
to be enabled.
Allow lifetime canonicalization of owned values and function arguments
as a simple stand-alone utility. This is now being called from within
SILCombine, so we should only do the kind of canonicalization that
makes sense in that context.
Canonicalizing other borrow scopes should *not* be invoked as a
single-value cleanup because it affects other lifetimes outside the
borrow scope boundary. It is a somewhat complicated process that
hoists and sinks forwarding instructions and can generate surrounding
compensation code. The copy propagation pass knows how to post-process
the related lifetimes in just the right order. So borrow scope
rewriting should only be done in the copy propagation pass.
Similarly, only do simple canonicalization of owned values and
function arguments at -Onone.
The feature to canoncalize borrow scopes is now ready to be
enabled (-canonical-ossa-rewrite-borrows), but flipping the switch
should be a separate commit. So most of the functionality that was
affected is not exposed by this PR.
Changes:
Split canonicalization of owned lifetimes vs. borrowed lifetimes into
separate utilities. The owned lifetime utility is now back to being
the simple utility that I originally envisioned. So not much happened
to it other than removing complexity.
We now have a separate entry point for finding the starting point for
rewriting borrow scopes:
CanonicalizeBorrowScope::getCanonicalBorrowedDef.
We now have a utility that defines forwarding instructions that we can
treat consistently as part of a guaranteed lifetime,
CanonicalizeBorrowScope::isRewritableOSSAForward.
We now have a utility that defines the uses of a borrowed value that
are considered part of its lifetime,
CanonicalizeBorrowScope::visitBorrowScopeUses. This single utility is
used to implement three different parts of the alogrithm:
1. Find any uses of the borrowed value that need to be propagated
outside the borrow scope
2. RewriteInnerBorrowUses for SILFunction arguments and borrow scopes
with no outer uses.
3. RewriteOuterBorrowUses for borrow scopes with outer uses. Handling
these involves creating new copies outside the borrow scope and
hoisting forwarding instructions.
The end result is that a lot of borrow scopes can be eliminated and
owned values can be forwarded to destructures, reducing copies and
destroys.
If we stop generating borrow scopes for all interior pointers, then
we'll need to design a comparable optimization that works on
"implicit" borrow scopes:
%ownedDef = ...
%element struct_extract %ownedDef
%copy = copy_value %element
apply(@guaranteed %element)
apply(@owned %copy)
destroy %ownedDef
Should be:
%ownedDef = ...
%borrowedElement = destructure_struct @guaranteed %ownedDef
apply(@guaranteed %borrowedElement)
%ownedElement = destructure_struct %ownedDef
apply(@owned %copy)
Track in-use iterators and update them both when instructions are
deleted and when they are added.
Safe iteration in the presence of arbitrary changes now looks like
this:
for (SILInstruction *inst : deleter.updatingRange(&bb)) {
modify(inst);
}
Required to break circular dependence when introducing
UpdatingInstructionIterator.
Also, this is a lot of detail that doesn't belong in the general
InstOptUtils APIs. It's not something people should ever reach for.
Clarify the API. Make it suitable for use everywhere in the
compiler. We should try to standardize on it and allow it to do the
OSSA fixup more often.
Add InstructionDeleter::updatingIterator() factory so we never
normally need to use InstModCallbacks.
Fix bugs in which notifyWillBeDeleted() was being called on invalid
SIL. The bugs are easily exposed just by removing copy_value side
effects, but that will be in the follow-up commit.
Call notifyWillBeDeleted() only when identifying new dead instructions
that the client may not know about. Give the client control over
force-deleting instructions. When doing its own lifetime fixups, the
client may force-delete a set of related instructions. Invoking
callbacks for these force-deleted instructions is wrong.
TODO: partial_apply support is only partial. I disabled the buggy
cases. This should be easy to fix but requires designing some
InstructionDeleter test cases.
Based on code in CopyPropagation. It is assumed that the passed in set of defs
is unique and that all such defs were found by using
CanonicalizeOSSALifetime::getCanonicalCopiedDef(copy).
InstModCallback is a value type and as such the original callback struct is not
being modified. Instead, a new InstModCallback struct is returned that is the
old callback + assignment of the passed in callback to the appropriate
field. Thus it makes sense to put this attribute on these methods so that we get
a warning if one does not use the new returned callback (otherwise, why would
one call this method?!). More likely a bug.
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.
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.
This enables passes to use this as a utility that properly composes with how the
pass maintains its state. If an InstModCallback isn't passed in, we use the
default InstModCallback which should be cheap (always succeeding check for
nullptr + call inline default callback).
It now handles looking through forwarding consumes such as
destructures.
Expected to be NFC since borrow consolidation is still disabled by
default.
TODO: Write unit tests for various forwarding consumes in addition to
destructure.
struct_extract and tuple_extract do not belong in OSSA (except to
workaround certain extreme cases). They completely defeat
simplification that OSSA provides for optimizing owned lifetimes.
Copy propagation uses this utility to canonicalize owned values before
canonicalizing their lifetime.
This API is useful when writing compiler code that needs to handle ossa/non-ossa
as well as load_borrow/load while in OSSA. I am going to use this in
SILMem2Reg.
We have for a long time talked about creating a scope like data structure for
use in the SILOptimizer. The discussion was whether or not to reuse the
infrastructure in SILGen that does this already. There were concerns about doing
so since the code in the SILOptimizer and SILGen can work differently.
With that in mind, I added a small AssertingScope class and built on top of that
a composition SIL level class called SILOptScope that one can use to add various
cleanups. One is able to both destructively pop at end of scope and pop along
early exits.
At an implementation level, I kept it simple and:
1. Represented a scope as a stack of Optional<Cleanup> which are just a wrapper
around a std::function. The Optional is so that we can invalidate a cleanup.
2. Based all of these scopes around the idea that the user of the scope must
invalidate the scope by hand. If not, the scope object will assert at the end
of its RAII scope.
3. Rather than creating a whole class hierarchy, I just used std::function
closures to keep things simple.
Instead, put the archetype->instrution map into SIlModule.
SILOpenedArchetypesTracker tried to maintain and reconstruct the mapping locally, e.g. during a use of SILBuilder.
Having a "global" map in SILModule makes the whole logic _much_ simpler.
I'm wondering why we didn't do this in the first place.
This requires that opened archetypes must be unique in a module - which makes sense. This was the case anyway, except for keypath accessors (which I fixed in the previous commit) and in some sil test files.
This directly adds support to BasicBlockCloner for updating OSSA.
It also adds a much more general-purpose GuaranteedPhiBorrowFixup
utility which can be used for more complicated SSA updates, in which
multiple phis need to be created. More generally, it handles adding
nested borrow scopes for guaranteed phis even when that phi is used by
other guaranteed phis.
This is a basic SSA-based liveness algorithm. It should just do what
it says. This change rescues the core logic from the nonsense that's
been hacked in. There are now two APIs:
(1) computeLifetimeBoundary (new) only does lifetime analysis. It
provides a new API that always succeeds and provides the last use
points so clients can do the right thing based on the user. I need
this for OSSA utilities.
(2) computeFrontier (old) emulates the original API with a simplified
version of the original logic.
Next steps:
- replace the old API with a separate utility that computes destroy
insertion points. Completely remove DeadEndBlocks from lifetime
analysis, because it simply has nothing to do with liveness.
- Gradually migrate clients to either the new lifetime API provided
here or the new destroy insertion API to be provided later.
OwnershipEliminator lowers destroy_value [poison] to debug_value
[poison].
IRGen overwrites all references in shadow copies with a sentinel value
in place of debug_value [poison].
Part 2/2: rdar://75012368 (-Onone compiler support for early object
deinitialization with sentinel dead references)
Ensure that any object lifetime that will be shortened ends in
destroy_value [poison], indicating that the reference should not be
dereferenced (e.g. by the debugger) beyond this point.
This has no way of knowing whether the object will actually be
deallocated at this point. It conservatively avoids showing garbage to
debuggers.
Part 1/2: rdar://75012368 (-Onone compiler support for early object
deinitialization with sentinel dead references)
It is currently disabled so this commit is NFC.
MandatoryCopyPropagation canonicalizes all all OSSA lifetimes with
either CopyValue or DestroyValue operations. While regular
CopyPropagation only canonicalizes lifetimes with copies. This ensures
that more lifetime program bugs are found in debug builds. Eventually,
regular CopyPropagation will also canonicalize all lifetimes, but for
now, we don't want to expose optimized code to more behavior change
than necessary.
Add frontend flags for developers to easily control copy propagation:
-enable-copy-propagation: enables whatever form of copy propagation
the current pipeline runs (mandatory-copy-propagation at -Onone,
regular copy-propation at -O).
-disable-copy-propagation: similarly disables any form of copy
propagation in the current pipelien.
To control a specific variant of the passes, use
-Xllvm -disable-pass=mandatory-copy-propagation
or -Xllvm -disable-pass=copy-propagation instead.
The meaning of these flags will stay the same as we adjust the
defaults. Soon mandatory-copy-propagation will be enabled by
default. There are two reasons to do this, both related to predictable
behavior across Debug and Release builds.
1. Shortening object lifetimes can cause observable changes in program
behavior in the presense of weak/unowned reference and
deinitializer side effects.
2. Programmers need to know reliably whether a given code pattern will
copy the storage for copy-on-write types (Array, Set). Eliminating
the "unexpected" copies the same way at -Onone and -O both makes
debugging tractable and provides assurance that the code isn't
relying on the luck of the optimizer in a particular compiler
release.
When the underlying utility was changed for OSSA, it changed the
semantics of the callback, which breaks the way I've always used a
deletion callback to update iterators.
/// \p callback is called on each deleted instruction before deleting any
/// instructions. This way, the SIL is valid in the callback. However, the
/// callback cannot be used to update instruction iterators since other
/// instructions to be deleted remain in the instruction list.
Enable most simplify-cfg optimizations as long as the block arguments
have trivial types. Enable most simplify CFG unit tests cases.
This massively reduces the size of the CFG during OSSA passes.
Test cases that aren't supported in OSSA yet have been moved to a
separate test file for disabled OSSA tests,
Full simplify-cfg support is currently blocked on OSSA utilities which
I haven't checked in yet.
The DiagnoseLifetimeIssuesPass pass prints a warning if an object is stored to a weak property (or is weakly captured) and destroyed before the property (or captured reference) is ever used again.
This can happen if the programmer relies on the lexical scope to keep an object alive, but copy-propagation can shrink the object's lifetime to its last use.
For example:
func test() {
let k = Klass()
// k is deallocated immediately after the closure capture (a store_weak).
functionWithClosure({ [weak k] in
// crash!
k!.foo()
})
}
Unfortunately this pass can only catch simple cases, but it's better than nothing.
rdar://73910632
