-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[region-isolation] Fix the dataflow and add support for project_block_storage #70514
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 11 commits into
swiftlang:main
from
gottesmm:pr-763529d8ee9f4164b82d03087fec25439cc1d315
Dec 19, 2023
Merged
[region-isolation] Fix the dataflow and add support for project_block_storage #70514
gottesmm
merged 11 commits into
swiftlang:main
from
gottesmm:pr-763529d8ee9f4164b82d03087fec25439cc1d315
Dec 19, 2023
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
…rnonsendable_* I also renamed sendnonsendable_basic.sil -> transfernonsendable_instruction_matching.sil since that is what the test is intended to be used for... mechanical instruction level modeling tests rather than more complex crashers.
This involved fixing a few different bugs. 1. We were just performing dataflow by setting that only the initial block needs to be updated. This means that if there isn’t anything in the initial dataflow block, we won’t visit any successor blocks. Instead the correct thing to do here is to visit all blocks in the initial round. 2. I also needed to fix a separate issue where we were updating our union-find data structure incorrectly as found by an assert on transfernonsendable.swift that was triggered once I fixed 1. Put simply, we needed to set a new max label + 1 when our new max element is less than or equal to the old max label + 1… before we just did less than so if we had a new max element that is the same as our fresh label, we wouldn’t increment the fresh label. rdar://119584497
@swift-ci smoke test |
rdar://119743743
41f051d
to
7712639
Compare
@swift-ci smoke test |
…in instructions that can only appear in non-OSSA as asserting. We already only supported Ownership SSA since we run early in the pipeline before OSSA is lowered. This just formalizes this behavior. I am marking these instructions as Asserting (even though we will never see them) so I can semantically be sure that all of the instructions are covered without using an "unsupported" like moniker that I fear will lead to new instructions being added as unsupported. Better to have a semantic thing for new instruction adders to use.
fix_lifetime just acts as a require.
…adata since they only appear in Lowered SIL. I also added a note to the SIL.rst and an assert into the SILVerifier to better document this requirement.
…t as an assign from op 0 -> result and a require of op 1. Semantically a mark_dependence returns a value that is equal to its first parameter with the extra semantics that any destroys of the 2nd operand cannot occur before any uses of the result of the instruction. From a region perspective this suggests that the instruction should be an assign from the first operand onto the result and act as a require on the result. Semantically the requirement that the 2nd operand cannot be destroyed before any uses of the result does not expose any memory or state from the first operand implying that we don't need to merge it into the result region. The restriction is purely to tell the optimizer what it can/cannot do rather.
@swift-ci smoke test |
…owed in LoweredSIL, update some tests by splitting them into their own Lowered file.
@swift-ci smoke test |
@swift-ci test |
Going to do a full test just in case another test needs updating (trying to be careful). |
Slava wants to do post-commit review. |
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.
In this PR, I fix three issues that came up when we were doing some testing:
I also renamed the tests with prefix sendnonsendable_* -> transfernonsendable_* given that I did the rename earlier. At the time I did that other rename in code I didn't do the change since I had a bunch of out of tree changes to those tests and didn't want to run into merge conflicts. That's all upstreamed now so we are good to go.
rdar://119584497
rdar://119743743