[OSSACanonicalizeOwned] Fix liveness passed to completion. #78624
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
The pass is structured to drain an instruction worklist and perform a sequence of operations on each popped instruction. Extract that sequence of operations into a new processInstruction function. Enables testing the sequence on a single instruction.
According to the documentation, ``` It's ok if postDomBlocks has duplicates or extraneous blocks, as long as they jointly post-dominate all live blocks that aren't on dead-end paths. ``` Previously, though, duplicates resulted in trapping: each element of `postDomBlocks` is pushed into a `BasicBlockWorklist`; when the second occurrence of an element in `postDomBlocks` was encountered, `BasicBlockWorklist::push` would trigger an assertion failure. Here, `pushIfNotVisited` is called instead.
nate-chandler
force-pushed
the
rdar142520491_2
branch
2 times, most recently
from
30024b3 to
4f8cf61
Compare
|
Source compat failures match failures in baseline. |
nate-chandler
force-pushed
the
rdar142520491_2
branch
from
4f8cf61 to
6a2fadc
Compare
To determine where a lifetime ends within dead-end blocks, OSSACanonicalizeOwned uses OSSACompleteLifetime's visitAvailabilityBoundary. This API takes a liveness which it uses to walk forward to the availability boundary. Specifically, the liveness passed from OSSACanonicalizeOwned to OSSACompleteLifetime is a variation of OSSACanonicalizeOwned's own liveness (it has destroys added). There is a mismatch in the characteristics of livenesses created by OSSACanonicalizeOwned and OSSACompleteLifetime: The former deals with not only direct uses of a value but also uses of its copies; that introduces the possibility for consuming uses in the middle of liveness. The latter on the other hand deals only with uses of a single value (nestedly, but at each level of nesting only a single value). Passing a liveness from the former to the latter without handling this mismatch is incorrect: OSSACompleteLifetime understands consuming uses to always end a lifetime, even when they are in the middle of a copy-extended liveness. The result is that OSSACompleteLifetime produces non-sensical results when provided with such a liveness. To address this, fixup the liveness passed from OSSACanonicalizeOwned to OSSACompleteLifetime by demoting consuming uses that appear within (that's to say _not_ on the boundary) of liveness to non-consuming uses. rdar://142846936
nate-chandler
force-pushed
the
rdar142520491_2
branch
from
6a2fadc to
ebf6028
Compare
|
@swift-ci please test |
|
@swift-ci please apple silicon benchmark |
|
@swift-ci please test source compatibility |
|
@swift-ci please test macos platform |
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.
To determine where a lifetime ends within dead-end blocks,
OSSACanonicalizeOwnedusesOSSACompleteLifetime's visitAvailabilityBoundary. This API takes a liveness which it uses to walk forward to the availability boundary. Specifically, the liveness passed fromOSSACanonicalizeOwnedtoOSSACompleteLifetimeis a variation ofOSSACanonicalizeOwned's own liveness (it has destroys added).There is a mismatch in the characteristics of livenesses created by
OSSACanonicalizeOwnedandOSSACompleteLifetime: The former deals with not only direct uses of a value but also uses of its copies; that introduces the possibility for consuming uses in the middle of liveness. The latter on the other hand deals only with uses of a single value (nestedly, but at each level of nesting only a single value). Passing a liveness from the former to the latter without handling this mismatch is incorrect:OSSACompleteLifetimeunderstands consuming uses to always end a lifetime, even when they are in the middle of a copy-extended liveness. The result is thatOSSACompleteLifetimeproduces non-sensical results when provided with such a liveness.To address this, fixup the liveness passed from
OSSACanonicalizeOwnedtoOSSACompleteLifetimeby demoting consuming uses that appear within (that's to say not on the boundary) of liveness to non-consuming uses.Also, check in a few other changes that facilitated tracking this down:
-sil-print-module-on-errorSILCombineto extract the processing it does on a single instruction into a separate functionFunctionTestto call that newSILCombinefunctionrdar://142846936