Skip to content

TempLValue opt: don't insert destroy_addr too early due to ignoring deinit barriers #68903

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 1 commit into from
Oct 4, 2023

Conversation

eeckstein
Copy link
Contributor

This can happen for inout arguments. It fixes a potential miscompile, e.g. observable by a weak reference being nil where it shouldn't.

rdar://116335089

@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci benchmark

…einit barriers

This can happen for inout arguments. It fixes a potential miscompile, e.g. observable by a weak reference being nil where it shouldn't.

rdar://116335089
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test

@atrick
Copy link
Contributor

atrick commented Oct 2, 2023

We haven't promised to respect deinit barriers for inout variables yet. I think we need to overcome the major performance issues first. Seems like all optimizations should be consistent until then.

Although if this can happen for non-inout variables, then it's definitely a bug.

@eeckstein
Copy link
Contributor Author

In TempLValueOpt it's not specific to inout addresses. But I don't know if such a pattern can be a result of anything else than an inout. Maybe as a result of optimizations.
Anyway, benchmarks don't show any regressions. So I think we are on the safe side with this fix.

@eeckstein eeckstein merged commit 8e9ff9b into swiftlang:main Oct 4, 2023
@eeckstein eeckstein deleted the fix-templvalue-opt branch October 4, 2023 05:46
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