Skip to content

[CanonicalizeOSSALifetime] Extend lexical lifetimes to unreachables. #68608

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
merged 6 commits into from
Sep 22, 2023

Conversation

nate-chandler
Copy link
Contributor

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_applys 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_borrows.

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

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

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.
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

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
Copy link
Contributor Author

@swift-ci please benchmark

@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

@nate-chandler
Copy link
Contributor Author

@swift-ci please apple silicon benchmark

@nate-chandler
Copy link
Contributor Author

@swift-ci please test windows platform

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