Skip to content

[OSSACanonicalizeOwned] Dead-end extend base lifetime. #79521

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 4 commits into from
Feb 27, 2025

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Feb 20, 2025

Regardless of consumes of copies, if the original lexical value is not consumed on a dead-end path, it must remain live to that dead-end.

rdar://145226757

@meg-gupta
Copy link
Contributor

@swift-ci build toolchain

@nate-chandler nate-chandler changed the title [OSSACanOwned] Dead-end extend base lifetime. [OSSACanonicalizeOwned] Dead-end extend base lifetime. Feb 20, 2025
@nate-chandler nate-chandler force-pushed the rdar145154549 branch 4 times, most recently from c3743bb to 8ff107f Compare February 22, 2025 00:05
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.

Here are my review notes. Maybe they are useful for comments...

  1. lexical-scope-extended liveness (lexical vars only):

1a. Separately compute the lexical lifetime of the original def.

This cannot include any consumes of copies. Otherwise, we would fail to extend the original lexical def:

 %copy = copy_value %def
 consume %copy
 apply foo()
 // need to extend the liveness of %def up to this point.
 unreachable

Note that none of the copied values are lexical, so they don't need to be extended to dead-end blocks.

1b. visit the lexical lifetime's availability boundary

Add the lexical boundary as uses to the current copy-extended liveness

Now we have an adjusted liveness that considers both the lexical lifetime and the copy-extended lifetime

===
2. complete liveness (always needed)

2a. create a temporary copy of copy-extended liveness

2b. demote and consumes within temporary liveness

If a lifetime-ending use is not on the liveness boundary, consider it a non-lifetime-ending use

Required by visitAvailabilityBoundary--the interior consumes will always be copied so they never kill availability.

2c. call visitAvailabilityBoundary on temporary liveness

Add instructions on the availability boundary to the original liveness, thereby extending it

Required to avoid inserting destroys within a borrow scope

 %c = copy_value %def
 %sb = store_borrow %c to ...
 // destroy_value %def - invalid
 unreachable

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please apple silicon benchmark

@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

@nate-chandler
Copy link
Contributor Author

@swift-ci please build toolchain

@nate-chandler
Copy link
Contributor Author

@swift-ci please test macos platform

1 similar comment
@nate-chandler
Copy link
Contributor Author

@swift-ci please test macos platform

Uses of borrows may occur between the borrow and a dead-end regardless
of lexicality, so always extend lifetimes to dead-ends.
Regardless of consumes of copies, if the original lexical value is not
consumed on a dead-end path, it must remain live to that dead-end.

rdar://145226757
If there is a pointer escape, we cannot safely rewrite its lifetime.
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler nate-chandler marked this pull request as ready for review February 27, 2025 22:36
@nate-chandler nate-chandler merged commit 39acb9a into swiftlang:main Feb 27, 2025
5 checks passed
@nate-chandler nate-chandler deleted the rdar145154549 branch February 27, 2025 22:36
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.

3 participants