[6.0][region-isolation] Cherry-pick recent work #73150
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
… Windows Error Codes Specifically, I put information about the system error code lookup tool and links to Microsoft's documentation. (cherry picked from commit 0922665)
…p to the checker. This means that I added an IsolationHistory field to Partition. Just upstreaming the beginning part of this work. I added some unittests to exercise the code as well. NOTE: This means that I did need to begin tracking an IsolationHistoryFactory and propagating IsolationHistory in the pass itself... but we do not use it for anything. A quick overview of the design. IsolationHistory is the head of an immutable directed acyclic graph. It is actually represented as an immutable linked list with a special node that ties in extra children nodes. The users of the information are expected to get a SmallVectorImpl and process those sibling nodes afterwards. The reason why we use an immutable approach is that it fits well with the problem and saves space since different partitions could be pointing at the same linked list node. Operations occur on an isolation history by pushing/popping nodes. It is assumed that the user will push nodes in batches with a sequence boundary at the bottom of the addition which signals to stop processing nodes. Tieing this together, each Partition within it contains an IsolationHistory. As the PartitionOpEvaluator applies PartitionOps to Partition in PartitionOpEvaluator::apply, the evaluator also updates the isolation history in the partition by first pushing a SequenceBoundary node and then pushing nodes that will undo the operation that it is performing. This information is used by the method Partition::popHistory. This pops linked list nodes from its history, performing the operation in reverse until it hits a SequenceBoundary node. This allows for one to rewind Partition history. And if one stashes an isolation history as a target, one can even unwind a partition to what its state was at a specific transfer point or earlier. Once we are at that point, we can begin going one node back at a time and see when two values that we are searching for no longer are apart of the same region. That is a place where we want to emit a diagnostic. We then process until we find for both of our values history points where they were the immediate reason why the two regions merge. rdar://123479934 (cherry picked from commit aee5a37)
…History. We package all isolation history nodes from a single instruction by placing a sequence boundary at the bottom. When ever we pop, we actually pop a PartitionOp at a time meaning that we pop until we see a SequenceBoundary. Thus the sequence boundary will always be the last element visited when popping meaning that it is a convenient place to stick the SILLocation associated with the entire PartitionOp. As a benefit, there was some unused space in IsolationHistory::Node for that case since we were not using the std::variant field at all. (cherry picked from commit d6b4f16)
…ut do not use it to emit diagnostics yet. Specifically: 1. I copy the history that we have been tracking from the transferring operand value at the transfer point. This is then available for use to emit diagnostics. 2. I added the ability for SILIsolationInfo to not only track the ActorIsolation of an actor isolated value, but also if we have a value, we can track that as well. Since we now track a value for task isolated and actor isolated SILIsolationInfo, I just renamed the field to isolatedValue and moved it out of the enum. In a subsequent commit, I am going to wire it up to a few diagnostics. rdar://123479934 (cherry picked from commit 6844b99)
(cherry picked from commit cc1a873)
Specifically, I am transforming it from "may cause a race" -> "may cause a data race". Adding data is a small thing, but it adds a bunch of nice clarity. (cherry picked from commit a56d0f5)
rdar://126060540 (cherry picked from commit 518f8f7)
…se a MapVector instead of a DenseMap. This ensures that we can efficiently iterate over the map which we will need to do for equality queries. I am going to add the equality queries in a subsequent commit. Just chopping off a larger commit. (cherry picked from commit 13d0139)
…nline in a TransferringOperand data structure. This is backing out an approach that I thought would be superior, but ended up causing problems. Originally, we mapped a region number to an immutable pointer set containing Operand * where the region was tranferred. This worked great for a time... until I began to need to propagate other information from the transferring code in the analysis to the actual diagnostic emitter. To be able to do that, my thought was to make a wrapper type around Operand called TransferringOperand that contained the operand and the other information I needed. This seemed to provide me what I wanted but I later found that since the immutable pointer set was tracking TransferringOperands which were always newly wrapped with an Operand *, we actually always created new pointer sets. This is of course wasteful from a memory perspective, but also prevents me from tracking transferring operand sets during the dataflow since we would never converge. In this commit, I fix that issue by again tracking just an Operand * in the TransferringOperandSet and instead map each operand to a state structure which we merge dataflow state into whenever we visit it. This provides us with everything we need to in the next commit to including a region -> transferring operand set equality check in our dataflow equations and always converge. (cherry picked from commit ca8179a)
…the dataflow convergence. The reason why I am doing this is that really we are running two different dataflow equations at the same time... one for propagating tracking transferring sets and the other for propagating regions. Since at the source level the two dataflow problems are very interrelated, I was unable to come up with an example where we fail to iterate because of this, but I would like to be sure that we do not hit one, so I am fixing this here. rdar://126170014 (cherry picked from commit d2bdec2)
… isolation info. I am finding that I am calling that a bunch so this just makes it a little more convenient. (cherry picked from commit beaab91)
…ment. Having two artificial typedefs for the same wrapped value is just confusing. Better to just have one and make the code simpler to understand. (cherry picked from commit c9fe8ff)
(cherry picked from commit b80faee)
…they are marked as a var_decl.
The reason why we do this is that we want to treat this as a separate value from
their operand since they are the result of defining a new value.
This has a few nice side-effects, one of which is that if a let results in just
a begin_borrow [var_decl], we emit names for it.
I also did a little work around helping variable name utils to lookup names from
applies that are fed into a {move_value,begin_borrow} [var_decl] which then has
the debug_value we are searching for.
(cherry picked from commit 20c2429)
…ment_addr and global_addr onto SILIsolationInfo and call that instead. (cherry picked from commit 513ab78)
…efInst/ClassMethodInst into SILIsolationInfo::get instead of handrolling in RegionAnalysis. (cherry picked from commit 2a7714a)
…ationInfo::get instead of computing actor isolation by hand. Just more recoring on top of SILIsolationInfo::get. (cherry picked from commit baca235)
…ionInfo::get. (cherry picked from commit b407b21)
This ensures that when we process, we consider load/load_borrow's result to be equivalent to its operand. This ensures that a load/load_borrow cannot act as a use of its operand preventing spurious diagnostics. (cherry picked from commit eed51e7)
…ce that a value is isolated to if we are dealing with an actor instance. This will let us distinguish in between values derived from two actor instances of the same type and to emit better errors. (cherry picked from commit d8f39f7)
…o VariableNameInferrer. I have been using these in TransferNonSendable and they are useful in terms of reducing the amount of code that one has to type to use this API. I am going to need to use it in SILIsolationInfo, so it makes sense to move it into SILOptimizer/Utils. NFCI. (cherry picked from commit 51ef67d)
…iagnostics, if we have a SIL actor instance, print -isolated instead of actor-isolated. rdar://122501400 (cherry picked from commit 62a4820)
…ctor self. rdar://122501400 (cherry picked from commit a9c163f)
…ated to a #isolated as being globally isolated to that. Instead, we need to consider the isolation at the apply site. rdar://126285681 rdar://125078448 (cherry picked from commit 2e5b3bc)
The thinko was that rather than use the actual isolated parameter of a partial apply as the isolated parameter, I instead just used the last operand... :doh:. This fixes: rdar://126297097 (cherry picked from commit 969dd69)
(cherry picked from commit 9d965f8)
… actors. (cherry picked from commit fa20e52)
…isAnyActor(). (cherry picked from commit 3c29997)
|
@swift-ci test |
hborla
approved these changes
Apr 19, 2024
|
@swift-ci test |
…tion are not accessible again after call to nonisolated async function I also fixed the nonisolated(unsafe) issue. rdar://126667934 rdar://126174959 (cherry picked from commit 0e21c58)
…s succeed. Specifically, the partition unit tests pass in bogus instructions/operands so we cannot call /any/ methods on them. So I created stubed out helpers on the evaluator that in the case of mocking just return a default initialized SILIsolationInfo(). (cherry picked from commit 1113a61)
|
@swift-ci test |
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 cherry-picking recent work.
Explanation: A bunch of recent work in region isolation.
Original PRs:
Risk: Low. Only affects region isolation.
Testing: Added many functional and unit tests.
Reviewer: N/A