-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[release/5.7][move-function] Remaining _move changes for 5.7 #59184
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
gottesmm
merged 9 commits into
swiftlang:release/5.7
from
gottesmm:release/5.7-movedbginfo
Jun 2, 2022
Merged
[release/5.7][move-function] Remaining _move changes for 5.7 #59184
gottesmm
merged 9 commits into
swiftlang:release/5.7
from
gottesmm:release/5.7-movedbginfo
Jun 2, 2022
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
Previously, "m" was below "k". Now they flipped order.
…and re-enable that test. The pattern started to fail here since the variable "m" is not a variable that we moved so as a result we use heuristics to determine if it should become an entry value. Noting that we only pattern match "m" to ensure that we can easily check the variable "k" afterwards (the thing that was actually moved), I just loosened the pattern so that we validate that we found "m" but do not check "m"'s location. This guarantees that on all platforms this will pattern match appropriately. rdar://91467528
The problem is that this pass is setup such that when asserts are enabled we are able to turn off/on internal parts of the transformation. When asserts are disabled, we don’t support this and just run with everything. This causes the output to differ. rdar://72627921
…g a moved value to a different value. Most of the tests that I wrote used full on non-consuming/consuming uses rather than initialization of a variable. This commit fixes that hole in the model and adds some tests that exercise this behavior. Address moves did not have this problem due to different codegen.
… debug_value undef before the destroy_addr. Otherwise in async contexts we may propagate forward an original non-invalidated debug_value created a use of the invalidated memory. This then causes the MemoryLifetimeVerifier to trigger. I also discovered via this test case that we were not ignoring end_access as a liveness use when visiting non-closure uses. With this change we ignore it now. That being said, the reason why the end_access has not been an issue previously is that due to the hacky way that the move builtin lowers, we actually create the move outside of the relevant access scope (which even worse is a read!). I am going to fix that in a different PR though.
…ard coded registers to use a regex. Otherwise there is a potential that various versions of the compiler may use a slightly different register causing the test to fail. We are not attempting to test this so it just makes sense to make this a more general pattern. rdar://91467528
…or async vars and clones the dbg info after funclet points. The overall flow of the pass is: 1. We walk over the blocks summarizing the debug info instruction the blocks gen as well as whether or not the block had an async funclet edge with in it. 2. We then perform a simple forward iterative optimistic dataflow using intersection at merge points. At points where we find after merging that we have a conflict and thus need to stop propagation, we insert a debug_value undef. 3. We then walk the CFG again visiting only blocks that we know had async funclet edges. We then walk each said block from top to bottom starting with the propagating gen information and updating as we go, dumping the current set of debug_info we are tracking after each coroutine funclet boundary. rdar://85020571
…t we don't need to pattern match and varies with asserts.
…ontrol flow tests.
tbkka
approved these changes
Jun 1, 2022
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.
Just adding the remaining changes for supporting _move to 5.7. Will not affect any code that doesn't use _move directly so is very safe. There is a companion set of LLVM level changes.