Fix LoadableByAddress to use @in_guaranteed for unowned values and enable SIL verification for on-stack partial_apply ownership #67907
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
atrick
force-pushed
the
verify-onstack-capture
branch
from
3e391a3 to
9300c95
Compare
|
@swift-ci test |
|
@swift-ci benchmark |
|
@swift-ci test source compatibility |
|
I'm waiting for the source compat test to finish before committing the fix for |
|
This PR has no significant effect on existing benchmarks. I did not attempt to create a benchmark for the improvement that eliminates copies from the closure context. |
LoadableByAddress was accidentally changing ownership of direct_unowned values to @in (owned). This generates unsupported SIL for on-stack partial applies, which now breaks SIL verification. This also resulted in extra copies of values inside of closure contexts. Before calling the original function, the value would need to be copied onto the stack and the context would be destroyed. Now, we simply pass a pointer directly from the closure context. See IRGen/indirect_argument.sil+huge_partial_application. I'm pretty sure fixing this has no effect on the mangling of public symbols.
atrick
force-pushed
the
verify-onstack-capture
branch
from
9300c95 to
f2287b9
Compare
|
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LoadableByAddress was accidentally changing ownership of
direct_unowned values to @in (owned). This generates unsupported SIL
for on-stack partial applies, which now breaks SIL verification.
This also resulted in extra copies of values inside of closure
contexts. Before calling the original function, the value would need to
be copied onto the stack and the context would be destroyed. Now, we
simply pass a pointer directly from the closure context. See
IRGen/indirect_argument.sil+huge_partial_application.
I'm pretty sure fixing this has no effect on the mangling of public symbols.