Skip to content

Conversation

eeckstein
Copy link
Contributor

@eeckstein eeckstein commented Aug 9, 2024

It was disabled so far to not hide bugs in the deinit code generation. Now hopefully deinit code generation is stable enough to enable the pass.

Also fix a bug some bugs which showed up after enabling the pass.
E.g. don't convert non-copyable owned -> guaranteed arguments in FunctionSignatureOpts: It's not safe to insert a compensating release_value at the call site. This release_value calls the deinit which might have been already de-virtualized in the callee.

@eeckstein eeckstein requested a review from rjmccall as a code owner August 9, 2024 15:33
@eeckstein eeckstein requested review from atrick and removed request for rjmccall August 9, 2024 15:33
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein eeckstein requested a review from kavon August 9, 2024 15:37
@eeckstein
Copy link
Contributor Author

@swift-ci benchmark

… arguments

It's not safe to insert a compensating release_value at the call site.
This release_value calls the deinit which might have been already de-virtualized in the callee.
…estroyed value can't be de-virtualized

This can happen if the destroyed type is resilient.
Fixes a verifier crash.
It was disabled so far to not hide bugs in the deinit code generation. Now hopefully deinit code generation is stable enough to enable the pass.
@eeckstein eeckstein force-pushed the enable-deinit-devirtualization branch from 5c0689c to 1dab294 Compare August 12, 2024 11:35
@eeckstein eeckstein requested a review from jckarter as a code owner August 12, 2024 11:35
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci benchmark

@eeckstein eeckstein merged commit 06604a0 into swiftlang:main Aug 12, 2024
6 checks passed
@eeckstein eeckstein deleted the enable-deinit-devirtualization branch August 12, 2024 17:20
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