Skip to content

[clf] Simplify the double diamond lifetime extension for escaping swi… #25320

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

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Jun 9, 2019

…ft funcs -> noescape blocks.

Specifically, when we optimize conversions such as:

  Optional<@escaping () -> ()>

  ->

  Optional<@noescape () -> ()>

  ->

  Optional<@noescape @convention(block) () -> ()>

previously we were lifetime extending over the @NoEscape lifetime barrier by
making a copy and then putting a mark_dependence from the copy onto the original
value. This was just a quick way to tell the ownership verifier that the copy
was tied to the other value and thus should not be eliminated. The correctness
of the actual lifetime extension comes from the optimizer being conservative
around rr insts.

This commit instead changes our optimization to borrow the copied optional
value, extract the payload, and use that instead.

@gottesmm gottesmm requested a review from aschwaighofer June 9, 2019 23:09
@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 9, 2019

@swift-ci smoke test

…ft funcs -> noescape blocks.

Specifically, when we optimize conversions such as:

  Optional<@escaping () -> ()>

  ->

  Optional<@NoEscape () -> ()>

  ->

  Optional<@NoEscape @convention(block) () -> ()>

previously we were lifetime extending over the @NoEscape lifetime barrier by
making a copy and then putting a mark_dependence from the copy onto the original
value. This was just a quick way to tell the ownership verifier that the copy
was tied to the other value and thus should not be eliminated. The correctness
of the actual lifetime extension comes from the optimizer being conservative
around rr insts.

This commit instead changes our optimization to borrow the copied optional
value, extract the payload, and use that instead.
@gottesmm gottesmm force-pushed the pr-393856a2bc74013494a4598e46d156795de076f2 branch from d150ffb to cf0405c Compare June 10, 2019 05:05
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

2 similar comments
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

Copy link
Contributor

@aschwaighofer aschwaighofer left a comment

Choose a reason for hiding this comment

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

lgtm

@gottesmm gottesmm merged commit b90e413 into swiftlang:master Jun 10, 2019
@gottesmm gottesmm deleted the pr-393856a2bc74013494a4598e46d156795de076f2 branch June 10, 2019 17:31
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