Copy propagation redesign and the CanonicalizeBorrowScopes utility #38220
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's fine to allow some optimization of access markers without invalidating this simple analysis.
for cross-file use.
so they don't need to be passed around on the side everywhere.
Explaining a footgun that I've forgotten to check for more than once.
before accessing the instruction's operands, which will crash.
We're getting lucky that this utility isn't breaking people's code.
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)
And fix the way it handles of borrow scopes so we can enable borrow scope rewiting. Make sure SILCombine only does canonicalization that operates on a self-contained single-value-lifetime. It's important to limit SILCombine to transformations where each individual step converges quickly to a more canonical form. Rewriting borrow scopes requires the copy propagation pass to coordinate all the individual transformations. Make canonicalizeLifetimes a SILCombine utility. This moves complexity out of the main loop. SILCombine knows which values it wants to canonicalize and can directly call either canonicalizeValueLifetime or canonicalizeFunctionArgument for each one. Respect the -enable/disable-copy-propagation options.
Currently disabled, but now ready to be enabled.
More dead code happens to be eliminated now because of consistent use of the InstructionDeleter.
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test windows |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Think of this as a complete rewrite of functionality that was mostly disabled anyway but is now ready to be enabled.
One incidental functionality change is that more dead code will now be eliminated during the copy propagation pass.
The one big commit is "Copy propagation redesign".
One medium size commit enabled by the big commit is "Simplify lifetime canonicalization in SILCombine".
Both those commits fix the -canonical-ossa-rewrite-borrows mode, which is now ready to be enabled by default.
The big commit is too hard to split into a separate PR, and the small commits are too trivial to be worth a separate PR.
In the big commit, the large amount of code that was deleted from CanonicalOSSALifetime.cpp can just be considered the old implemenation so can be ignored. The implementation in CanonicalizeBorrowScopes.cpp is new. Although it looks like a big diff, it is now much easier to review the source itself. The design changes are described in the commit message.