Skip to content

[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
merged 9 commits into from
Jun 2, 2022

Conversation

gottesmm
Copy link
Contributor

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.

gottesmm added 9 commits May 31, 2022 13:09
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.
@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 1, 2022

@gottesmm gottesmm merged commit 12c0198 into swiftlang:release/5.7 Jun 2, 2022
@gottesmm gottesmm deleted the release/5.7-movedbginfo branch June 2, 2022 06:11
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