-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[move-only] Add support for switch_enum in borrow2destructure #63530
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
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
…n of PrunedLiveBlocks called FieldSensitivePrunedLiveBlocks. This will let the non-field sensitive version use a more performant implementation internally. This is important since PrunedLiveBlocks is used in the hot path when working with Ownership SSA, while the field sensitive version is only used for certain diagnostics. NOTE: I did not refactor PrunedLiveness to use the faster implementation... this is just a quick pass over the code to prepare for that change.
… different sized root values.
… invoked by the tester/main checker pass rather than a utility. This is in preparation for sinking the main computation of the pass into an implementation local helper struct that can process multiple values. I am doing this so we can use the same code to process switch_enum arguments and am using the current implementation to make sure I don't break anything.
…ansform into a private namespace in preparation for sinking into the implementation file.
… its own header and out of MoveOnlyObjectChecker.h
… its interface by moving gatherUses onto the impl.
… an implementation local static function.
…DestructureTransform driver to the local Implementation.
…ransform in the implementation file.
… and allow for it to be initialized separately from construction.
…ecific cleanup and inserting compensating destroys.
…terestingOperandIdnexMap, and blocksToUses into the implementation. For now, I am creating new implementations every time... but once I get the tests passing, I am going to improve the performance by reusing the same Implementation for all computations so we can reuse memory.
As mentioned previously, I am right now creating new Implementations for each switch_enum argument. Once I get tests working I am going to refactor and reuse the same implementation so I can reuse memory. NOTE: This currently does not impact SILGen since I did not change SILGenPattern to emit noncopyable switched upon values as guaranteed. Keep in mind that I am not going to do this for no implicit copy though since no implicit copy values cannot contain move only values so we do not need to work with destructures.
…ecker so we handle load [copy] switch_enum.
…r in the pass pipeline and move post checker verification into the MoveOnlyAddressChecker. The reason why I am doing this is that: 1. There are sometimes copy_value on move only values loaded from memory that the MoveOnlyAddressChecker needs to eliminate. 2. Previously, the move only address checker did not rewrite copy_value -> explicit_copy_value if it failed in diagnostics (it did handle load [copy] and copy_addr though). This could then cause the copy_value elimination verification in the MoveOnlyObjectChecker to then fail. So this suggested that I needed to move the verification from the object checkt to the address checker. 3. If we run the verification in the address checker, then the object checker (which previously ran before the address checker) naturally needed to run /before/ the address checker.
…dle switch_enum, emit move only enum switches at +0.
This test was taken from some generated SIL when the address checker ran /before/ the object checker and before the address checker validated that the object checker also didn't leave any copy_value. This is just cleaning up the source as if the object checker ran on this specific function so the address checker check doesn't trip.
@swift-ci smoke test |
@swift-ci smoke test macOS platform |
This was rdar://104874356 |
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.
Previously for move only types, SILGen would emit an owned switch_enum. This became an issue since for objects, SILGenPattern always attempts to push it towards borrowing, so eventually we would borrow which could have introduced an inadvertent copy along the decision tree before bindings are assigned.
This patch fixes this issue by doing the following: