[5.6] Copy-propagation enables lexical-lifetimes. #40517
Merged
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
|
@swift-ci please test |
|
@swift-ci please clean test windows platform |
|
We'll want to add at least one more once the PR is merged: which also contains the test cases for the last commit here 😊 |
atrick
reviewed
Dec 13, 2021
Verify that lexical-lifetimes and copy-propagation interact correctly to keep objects named by variables alive in a couple interesting cases: unsafe pointers, synchronization point calls.
Use the result returned from shrinkgBorrowScope to update the changed flag.
Promoted isRedundantLexicalBeginBorrow function to OwnershipUtils so that it can be used in more than just SemanticARCOpts.
Previously, CanonicalizeInstruction::eliminateSimpleBorrows bailed out when encountering any any [lexical] begin_borrow. While generally such borrow scopes must be preserved, they may be removed if they are redundant (when the borrowed value is guaranteed by another means: an outer lexical borrow scope or a guaranteed function argument). Here, removing such redundant lexical borrow scopes is enabled.
A guaranteed value produced by a begin_borrow can't be both used as an operand of an ownership forwarding-instruction and also reborrowed by being used as a phi argument. Avoid that by stopping rotation when encountering a header block containing an ownership-forwarding instruction whose forwarded ownership kind is guaranteed; such a rotation may result in using both the original guaranteed value and the resulting guaranteed value as phi arguments.
The effect of passing -enable-copy-propagation is both to enable the CopyPropagation pass to shorten object lifetimes and also to enable lexical lifetimes to ensure that object lifetimes aren't shortened while a variable is still in scope and used. Add a new flag, -enable-lexical-borrow-scopes=true to override -enable-copy-propagation's effect (setting it to ::ExperimentalLate) on SILOptions::LexicalLifetimes that sets it to ::Early even in the face of -enable-copy-propagation. The old flag -disable-lexical-lifetimes is renamed to -enable-lexical-borrow-scopes=false but continues to set that option to ::Off even when -enable-copy-propagation is passed.
Until swiftlang#40392 lands, run destroy hoisting with lexical lifetimes.
AllocStackHoisting creates new dealloc_stack instructions. The old
dealloc_stack instructions' locations' kinds are ::CleanupKind The new
instructions' locations need to be of that same kind too in order not to
trigger the sil verifier failure
Basic block contains a non-contiguous lexical scope at -Onone
when building at Onone.
AutoDiffAllocateSubcontext and AutoDiffProjectTopLevelSubcontext return RawPointer. They cannot be ForwardingBorrows. This was triggering an assert in ForwardingOperand's constructor.
SemanticARCOptVisitor::performGuaranteedCopyValueOptimization was
converting this SIL
%borrow = begin_borrow %copiedValue
%copy = copy_value %borrow
%borrowCopy = begin_borrow %copy
end_borrow %borrow
end_borrow %borrowCopy
destroy_value %copy
// something something
unreachable
into
%borrow = begin_borrow %copiedValue
%innerBorrow = begin_borrow %borrow
end_borrow %borrow
end_borrow %innerBorrow
// something something
unreachable
Dead-end blocks are simply irrelevant for this
optimization. Unfortunately, there were multiple layers of attempted
workarounds that were hiding the real problem, except in rare cases.
Thanks Nate Chandler for reducing the test.
The cases' new names more accurately reflect what behavior occurs when SILOptions::LexicalLifetimes is assigned that case.
If -disable-copy-propagation is passed, just emit lexical diagnostic markers but do not enable lexical lifetimes.
More bugs related to dead-end blocks and OSSA lifetimes that aren't well formed because of that. Independently, I'm working on prohibiting ill-formed OSSA even in the presence of dead-end blocks. That makes these workarounds unnecessary, but we urgently need a narrow fix here.
Previously the following pairs were prohibited incorrectly: - -enable-lexical-borrow-scopes=true, -enable-experimental-move-only - -enable-lexical-lifetimes=true, -enable-experimental-move-only and the following pairs were allowed incorrectly: - -enable-lexical-borrow-scopes=false, -enable-experimental-move-only - -enable-lexical-lifetimes=false, -enable-experimental-move-only Here, that's fixed. The first two pairs are allowed and the second two pairs prohibited.
Diagnose passing both -enable-copy-propagation and -disable-copy-propagation.
Replaced the quad-state (of state which one was illegal) of two booleans (EnableCopyPropagation and DisableCopyPropagation) with an enum.
In preparation to change the default values, specify explicitly what they are.
Previously, the default value for SILOptions::LexicalLifetimes was based on a copy propagation behavior (which can then be overridden by the flags for lexical lifetimems) only when the copy propagation was explicitly specified. Instead, set base the default value for this option (SILOptions::LexicalLifetimes) on the effective copy propagation behavior (i.e. SILOptions::CopyPropagation) regardless of whether an explicit behavior has been specified. Doing so will ensure that the desired behavior occurs as the default behavior for copy-propagation changes, but for now this change is NFC.
Replaced the -disable-copy-propagation flag with -enable-copy-propagation=false where the latter is a new multi-var -enable-copy-propagation= which can take one of three values: - true - requested-passes-only - false
Replaced findInnerTransitiveGuaranteeedUsesOfBorrowedValue with findExtendedUsesOfSimpleBorrowedValue. Starting from a borrowed value, it finds all extended (i.e., seeing through copies) uses of the borrow and its projections within the simple (i.e. without considering reborrowing) borrow scope.
So that CopyPropagation and other clients can react accordingly, pass back a list of copy_value instructions that were rewritten by ShrinkBorrowScope. In CopyPropagation, add each modified copy to the copy worklist.
The blocksWithReachedTops variable is only ever used while finding barriers and in a small helper function called at that time. Make it local to that function and make that helper a lambda in that function.
If an end_borrow cannot be hoisted, do not rewrite it.
Previously, ShrinkBorrowScope was only run alongside CanonicalizeBorrowScope under the flag -canonical-ossa-rewrite-borrows. That is no longer so, so remove that flag. Update a test case to undo the canonicalization which CanonicalizeBorrowScope was previously doing.
The new file tests the -O pipeline up to just before OSSA lowering. It will be used for verifying the effects of ShrinkBorrowScope on swift code directly, to complement the tests done on SIL by just running a single pass.
Previously, after determining that a block was dead, all its predecessors were added to the worklist. That is a problem, with the current algorithm, if any of the predecessors have a terminator over which the borrow scope cannot be shrunk. Here, before adding a dead block's predecessors to the worklist of blocks through which to shrink, ensure that all those predecessors have terminators over which the scope can be shrunk. Finally, when beginning to process a block, if it doesn't have a starting instruction, first hoist over thet terminator.
nate-chandler
force-pushed
the
cherrypick/lexical-lifetimes/enable-with-copy-propagation/release/5.6
branch
from
3bf1ddb to
284559f
Compare
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
|
Build failed |
|
Build failed |
The new Xllvm flag prints the module to stdout just before the OSSA lowering pass runs.
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
atrick
approved these changes
Dec 21, 2021
nate-chandler
deleted the
cherrypick/lexical-lifetimes/enable-with-copy-propagation/release/5.6
branch
AnthonyLatsis
added
🍒 release cherry pick
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.
Commits needed to enable lexical lifetimes when copy propagation is enabled.