Skip to content

[semantic-arc] In SILGen always assign a copy_value's argument to its result. #5661

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 2 commits into from
Nov 7, 2016

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Nov 7, 2016

This ensures that ownership is properly propagated forward through the use-def
graph.

This was the work that was stymied by issues relating to SILBuilder performing local ARC dataflow. I ripped out that local dataflow in 6f4e2ab and added a cheap ARC guaranteed dataflow pass that performs the same optimization.

Also in the process of doing this work, I found that there were many SILGen tests that were either pattern matching in the wrong functions or had wrong CHECK lines (for instance CHECK_NEXT). I fixed all of these issues and also expanded many of the tests so that they verify ownership. The only work I left for a future PR is that there are certain places in tests where we are using the projection from an original value, instead of a copy. I marked those with a message SEMANTIC ARC TODO so that they are easy to find.

rdar://28685236

@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 7, 2016

@swift-ci Please smoke test

@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 7, 2016

I am verifying that OS X passes and that Linux passes. I have a feeling that I will have simulator failures that I am fixing tonight.

@gottesmm gottesmm force-pushed the forward-copy-value branch 2 times, most recently from 7593a92 to 3b2f913 Compare November 7, 2016 07:16
…TypeLowering.

I am removing these usages of this API since it conflicts with SILGen's want to
hold onto copy_value return values for ownership propagation purposes. If any of
the copy_value are folded, the reference that SILGen holds onto will be
invalid.

rdar://28685236
… result.

This ensures that ownership is properly propagated forward through the use-def
graph.

This was the work that was stymied by issues relating to SILBuilder performing
local ARC dataflow. I ripped out that local dataflow in 6f4e2ab and added a
cheap ARC guaranteed dataflow pass that performs the same optimization.

Also in the process of doing this work, I found that there were many SILGen
tests that were either pattern matching in the wrong functions or had wrong
CHECK lines (for instance CHECK_NEXT). I fixed all of these issues and also
expanded many of the tests so that they verify ownership. The only work I left
for a future PR is that there are certain places in tests where we are using the
projection from an original value, instead of a copy. I marked those with a
message SEMANTIC ARC TODO so that they are easy to find.

rdar://28685236
@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 7, 2016

@swift-ci Please smoke test OS X platform

@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 7, 2016

@swift-ci Please test

@swift-ci
Copy link
Contributor

swift-ci commented Nov 7, 2016

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - ca5bb764755af79ef0c367c1c34b7fb2b350adf9
Test requested by - @gottesmm

@swift-ci
Copy link
Contributor

swift-ci commented Nov 7, 2016

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - ca5bb764755af79ef0c367c1c34b7fb2b350adf9
Test requested by - @gottesmm

@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 7, 2016

@swift-ci Please clean test linux platform

@gottesmm gottesmm merged commit 5a772d7 into swiftlang:master Nov 7, 2016
@gottesmm gottesmm deleted the forward-copy-value branch November 7, 2016 08:48
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