Skip to content

Conversation

@atrick
Copy link
Contributor

@atrick atrick commented Apr 1, 2023

Fix a miscompile in Debug builds at -Onone.

This optimization ignores uses of owned values that aren't enclosed in borrow scopes. This is fairly eggregious since project_box instructions are never borrowed, which means that all local variables have this problem.

This is a well-known issue that occurs throughout OSSA optimizations. The reason that we don't see the problem often is that the optimizations are hidden behind a pile of ad-hoc pattern matching, so they only kick in for simple cases. This approach to optimization is great at hiding problems for a long time.

We're attempting to design away this class of problems in the next release. Until then, it's one miscompile at a time.

Fixes rdar://107420448 (Variable mutation in block isn't reflected in outer scope: new behavior in swift 5.9)

Fix a miscompile in Debug builds at -Onone.

This optimization ignores uses of owned values that aren't enclosed in
borrow scopes. This is fairly eggregious since project_box
instructions are never borrowed, which means that all local variables
have this problem.

This is a well-known issue that occurs throughout OSSA
optimizations. The reason that we don't see the problem often is that
the optimizations are hidden behind a pile of ad-hoc pattern matching,
so they only kick in for simple cases. This approach to optimization
is great at hiding problems for a long time.

We're attempting to design away this class of problems in the next
release. Until then, it's one miscompile at a time.

Fixes rdar://107420448 (Variable mutation in block isn't reflected
in outer scope: new behavior in swift 5.9)
@atrick atrick requested review from eeckstein and meg-gupta April 1, 2023 03:34
@atrick
Copy link
Contributor Author

atrick commented Apr 1, 2023

@swift-ci test

@atrick
Copy link
Contributor Author

atrick commented Apr 1, 2023

@swift-ci benchmark

@atrick atrick requested a review from nate-chandler April 2, 2023 01:18
Copy link
Contributor

@meg-gupta meg-gupta left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

lgtm!

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