[silgenpattern] Change all loadable types to always go through the ownership preserving tuple code. #19840
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
|
@swift-ci smoke test |
|
Going to consolidate these changes into 1 PR as per Slava's request. |
|
Or actually, I am going to use this 1 PR for all of them. |
…and only use BorrowAlways with objects. The reason why the asserts were bogus is that even with SILOwnership enabled, we will use the old pattern to emit address only code. In that case, the asserts will fire on good behavior. I also fixed a latent bug where when ownership was enabled we were treating address only values like objects. Found via inspection. rdar://29791263
This is really old code that I put in when I was still understanding SILGenPattern. If we are emitting code in ownership form we will never have a TakeOnSuccess consumption kind, so we should not forward in that case "automagically". rdar://29791263
…nership preserving tuple code. To do this the commit does a few things: 1. If we enter the tuple dispatch code with an address that is loadable, we always immediately perform a load_borrow and change the consumable managed value to BorrowAlways. 2. If we have a take_on_success object, we immediately borrow. We do not want to do with TakeOnSuccess since we want to define away the need to unforward since unforwarding recreates a destroy on the already invalidated parent tuple object. Notice that this code still handles TakeAlways so in simple cases where we have a +1 value, everything still works. Since emitTupleDispatchWithOwnership now handles only objects, I renamed it to emitTupleObjectDispatch. rdar://29791263
gottesmm
force-pushed
the
pr-b1a0a45c5f7ddde8879ac6fcf10003e0805603f9
branch
from
fd7571a to
359eda5
Compare
|
@slavapestov I made the changes you requested to the first commit. |
|
@swift-ci smoke test |
5 similar comments
|
@swift-ci smoke test |
|
@swift-ci smoke test |
|
@swift-ci smoke test |
|
@swift-ci smoke test |
|
@swift-ci smoke test |
slavapestov
approved these changes
Oct 12, 2018
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.
[silgenpattern] Change all loadable types to always go through the ownership preserving tuple code.
To do this the commit does a few things:
If we enter the tuple dispatch code with an address that is loadable, we
always immediately perform a load_borrow and change the consumable managed value
to BorrowAlways.
If we have a take_on_success object, we immediately borrow. We do not want to
do with TakeOnSuccess since we want to define away the need to unforward since
unforwarding recreates a destroy on the already invalidated parent tuple object.
Notice that this code still handles TakeAlways so in simple cases where we have
a +1 value, everything still works.
Since emitTupleDispatchWithOwnership now handles only objects, I renamed it to
emitTupleObjectDispatch.
rdar://29791263
Note the commit in question is just the last commit. I have a string of commits that this is based upon that are being committed in other PRs: #19837 and #19838