Skip to content

5.10: [CanonicalizeOSSALifetime] Extend lexical lifetimes to unreachables. #68691

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

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Sep 22, 2023

Description: Work around incomplete lifetimes to extend lifetimes to unreachables.

When a non-lexical value's lifetime is canonicalized, copies are propagated to consuming uses and destroys to last non-consuming uses. Shortening a lexical value's lifetime in that way isn't allowed: it doesn't respect deinit barriers.

The original lifetime of a value ends at its original lifetime-ending uses: destroys, last consumes, and--without complete OSSA lifetimes enabled--unreachableinstructions. For a lexical value, the lifetime ends after canonicalization can be hoisted but not over any instructions that are deinit barriers. For destroys, this is handled via a backwards walk.

Here, because complete OSSA lifetimes aren't enabled, handling is added for unreachables. The approach is to add the instructions before the unreachable instructions to liveness and then skip creating a destroy_value if the next instruction is an unreachable.

To find the relevant unreachable instructions, a copy of PrunedLiveness that canonicalization uses is created and the destroys are added to it. (Destroys are not in the original instance of liveness and should not be added to it because having them there would result in extra copies.) That copy is then passed to some utility code shared with OSSALifetimeCompletion to find the unreachable instructions that terminate the dead-end blocks found by walking forward from the boundary of liveness. Once those unreachables are found, the instructions prior to them are added to the original copy of liveness.
Risk: Low. Uses two preexisting utilities (PrunedLiveness, OSSALifetimeCompletion)

Scope: Narrow. Only affects lifetimes in dead end blocks.

Original PR: #68608

Reviewed By: Andrew Trick ( @atrick )

Testing: Added tests.

Resolves: rdar://115468707

It's useful to be able to form a second copy of liveness which is
differently pruned by having more instructions added to it, e.g..
Don't respect deinit barriers when canonicalizing if lexical lifetimes
are disabled.
Extracted the new visitUnreachableLifetimeEnds static member of
OSSALifetimeCompletion from the preexisting
endLifetimeAtUnreachableBlocks which now calls through the former.
When canonicalizing the lifetime of a lexical value, deinit barriers are
respected. This is done by walking backwards from lifetime ends and
adding encountered deinit barriers to liveness.

Only destroy lifetime ends were walked back from under the assumption
that lifetimes would be complete. Without complete OSSA lifetimes,
however, it's necessary to also necessary to consider lifetimes that end
with unreachables. Unfortunately, we can't simply walk back from those
unreachables because there may be instructions which are secretly users
of the value being canonicalized (e.g. destroys of `partial_apply`s to
which a `begin_borrow` of the value was passed). Such uses don't appear
in the use list because lifetime canonicalization expects complete
lifetimes and only visits lifetime ends of `begin_borrow`s.

Here, instead, the instructions before the relevant unreachables are
added to liveness. In order to determine which unreachables are
relevant, it's necessary to have a liveness that includes the original
destroys. So a copy of liveness is created and those destroys are added
to it.

rdar://115468707
Such destroys are part of the lifetime of lexical values inserted to
prevent shortening lifetimes over deinit baarriers by
CanonicalizeOSSALifetime.  After OSSALifetimeCompletion is enabled, more
instructions will need to be preserved.
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler nate-chandler marked this pull request as ready for review September 22, 2023 14:25
@nate-chandler nate-chandler requested a review from a team as a code owner September 22, 2023 14:25
@nate-chandler nate-chandler requested a review from tbkka September 22, 2023 14:25
@nate-chandler nate-chandler merged commit 8745302 into swiftlang:release/5.10 Sep 22, 2023
@nate-chandler nate-chandler deleted the cherrypick/release/5.10/rdar115468707 branch September 22, 2023 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants