-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[OSSACanonicalizeOwned] Record traversed defs and don't traverse copies of guaranteed values. #77968
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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
15be37b
to
b9c5c41
Compare
Add a type which distinguishes among the types of defs that are pushed onto the "def-use worklist". Note that it's not possible to rely on the kind of value because the root may itself be a copy_value. For now, the distinction is discarded as soon as the def is visited.
When rewriting uses, determine how that rewriting should proceed based on the kind of def whose use is being visited. Direct uses of reborrows and borrowed-froms never need to be rewritten because every such use is a use of a guaranteed value and can never require a copy.
Add a local variable that shadows the `defUseWorklist` instance member, enabling further constraints (e.g. `const`ness) to eventually be imposed.
In preparation for only recording the defs once, replace the GraphNodeWorklist of defs with a SetVector. Preserve the current visitation order by creating a worklist of indices to be visited.
The field is no longer a worklist, just a list of discovered defs.
b9c5c41
to
bedb690
Compare
bedb690
to
4566f57
Compare
The utility performs two def-use traversals. The first determines liveness from uses. The second rewrites copies. Previously, the defs whose uses were analyzed were discovered twice, once during each traversal. The non-triviality of the discovery logic (i.e. the logic determining when to walk into the values produced by the instructions which were the users of visited uses) opened the possibility for a divergence between the two discoveries. This possibility had indeed been realized--the two traversals didn't visit exactly the same uses, and issues ensue. Here, the defs whose uses are analyzed are discovered only once (and not discarded as their uses are analyzed) during the first traversal. The second traversal reuses the defs discovered in the first traversal, eliminating the possibility of a def discovery difference. The second traversal is now done in a different order. This results in perturbing the SIL in certain cases.
Because discovery of defs walks into reborrows and borrowed-from instructions, copies may be seen whose underlying value is a guaranteed value (namely, a reborrow or a borrowed-from instruction). Such copies may be used beyond the lifetime end of such guaranteed values, so it's not allowed to sink copies to their consuming uses. Such canonicalization is the responsibility of the OSSACanonicalizeGuaranteed utility. rdar://139842132
4566f57
to
33135a1
Compare
@swift-ci please test |
@swift-ci please test source compatibility |
@swift-ci please apple silicon benchmark |
@swift-ci please clean test linux platform |
atrick
approved these changes
Dec 5, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@swift-ci please test linux platform |
@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.
The utility performs two def-use traversals. The first determines liveness from uses. The second rewrites copies.
Previously, the defs whose uses are analyzed were discovered twice, once during each traversal. The non-triviality of the discovery logic (i.e. the logic determining when to walk into the values produced by the instructions which were the users of visited uses) opened the possibility for a divergence between the two discoveries. This possibility had indeed been realized--the two discoveries didn't find exactly the same defs, the two traversals didn't visit exactly the same uses, and issues ensued.
Here, the defs whose uses are analyzed are discovered only once during the first traversal. The second "traversal" now reuses the defs discovered in the first traversal, eliminating the possibility of a def discovery difference.
Extra care is taken to preserve the traversal order of the first traversal (as compared to the order before this change). Evidently diagnostics depend on it. The second traversal's order is changed, which results in slight SIL perturbations in certain cases (e.g. an instruction which consumes a value and its copy may see the order of those operands reversed).
Because discovery of defs walks into reborrows and borrowed-from instructions, copies are processed whose underlying value is a reborrow or borrowed-from. Such copies cannot generally be sunk to consuming uses because the lifetime of their underlying value (the reborrow or borrowed-from) may not extend to the consuming use. Eliminating such copies is outside the scope of OSSACanonicalizeOwned--it's the responsibility of a separate OSSACanonicalizeGuaranteed utility. For this reason, when visiting copies during the first traversal, the copies are only walked into (and added to the list of defs) if their operand is not a reborrow or borrowed-from.
rdar://139842132