Fix uninitialized variable errors when the appropriate sync occurs - #6
Closed
aleph-oh wants to merge 18 commits into
Closed
Fix uninitialized variable errors when the appropriate sync occurs#6aleph-oh wants to merge 18 commits into
aleph-oh wants to merge 18 commits into
Conversation
mark_cilk_tasks builds a task tree and determines all reattach points for a given task. This is useful when computing various dataflow analyses since sync changes what variables are initialized (and other state, but that's harder to integrate and not required to get code to compile). We still have to use this module to correctly handle sync terminators of basic blocks when finding what variables will be initialized, it would be nice if we had separate notions of "will-be-synced" and "may-be-synced" for each corresponding kind of dataflow, and we still need to integrate this with borrow-checking so that we don't kill loans too early.
This commit extends mark_cilk_tasks::TaskTree with useful methods for initialized variable analysis (mostly ways to observe the state of a TaskTree). It primarily extends the analyses of initialized variables to consider syncs as initializing all variables that are initialized at reattachment points. One part I'm not sure of: we want the number of initialized variables to decrease when we merge in DefinitelyInitializedVariables if the reattaches are from the same task, and increase if they're not from the same task.
This commit changes DefinitelyInitializedVariables to merge the dataflow state at the places a task exits via join (intersection), which makes sense because it reduces the number of initialized places. We then use meet (union) to merge the initialized variable state after all of the tasks are done at the sync. The bug with syncing conditional spawns is still possible.
Pulls the way we merge dataflow state within a task into a helper function. This makes the function easier-to-read. We also change the public API of mark_cilk_tasks since last_locations_by_child was hard to compose.
When we used a visitor, we saw ICEs when building rustc. I think this is because of the particular order of the visitor, but here we care about the traversal order and only need to worry about basic block terminators. A preorder traversal makes sense because we want to ensure that the only block which has a new task constructed for it is the root of the Body, and this is true in a preorder traversal as long as the Body is connected (which seems to be a reasonable assumption). If at some point the body is disconnected, we can allow reusing the default task since the task we assign doesn't actually matter: the disconnected portion of the graph (whichever one isn't reachable from the root) will never be executed and should be removed by dead code elimination.
Changes `from_body` to use helpers for handling each kind of terminator. This lets us use better names and makes from_body a little easier to read at a glance.
We now do not label basic blocks in the unwind subgraph with a task. This is because the cleanup blocks in the unwind subgraph are reachable from non-cleanup blocks when they unwind. This would lead to labeling unwind blocks with many possible tasks, which breaks our invariant that blocks have exactly one task.
Previously, the LHS of an assignment is always used by a FakeRead for better diagnostics, since it's otherwise possible to create variable that can't actually be used. The read makes those initializations an error. However, the value is not available in the case of a spawn until a sync, so we get this benefit anyways.
matching_on_spawned_expression tests that matching on an un-synced expression fails, and fib_block_recurse_type_ascription checks that type ascription works the same way as without type ascription.
aleph-oh
commented
Apr 5, 2024
OwnerAuthor
Branch mis-named, closing. |
oooacaiooo referenced
this pull request
in mcj-group/rust-cilk
Sep 28, 2025
# This is the 1st commit message: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #2: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #3: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #4: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #5: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #6: debug # This is the commit message #7: debug # This is the commit message #8: debug # This is the commit message #9: debug # This is the commit message #10: debug # This is the commit message #11: debug
oooacaiooo referenced
this pull request
in mcj-group/rust-cilk
Oct 24, 2025
# This is the 1st commit message: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #2: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #3: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #4: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #5: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #6: debug # This is the commit message #7: debug # This is the commit message #8: debug # This is the commit message #9: debug # This is the commit message #10: debug # This is the commit message #11: debug
oooacaiooo referenced
this pull request
in mcj-group/rust-cilk
Mar 25, 2026
# This is the 1st commit message: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #2: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #3: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #4: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #5: in llvm-project: Move Orphaning analysis to parent functions # This is the commit message #6: debug # This is the commit message #7: debug # This is the commit message #8: debug # This is the commit message #9: debug # This is the commit message #10: debug # This is the commit message #11: debug
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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR resolves uninitialized variable errors when using variables that have been synced. It does this through the notion of a TaskTree and a mapping from basic blocks to tasks within a body. Then, all the variables initialized from all reattached blocks at the time of a sync are marked as initialized after the sync (with modifications for whatever variable initialization pass we're referring to). One point of complexity here is that the unwind path has edges into it from more than one task, so we can't label the entire unwind path / unwind subgraph with a task.
The tests that previously expected an error because a spawned variable was used (even after a sync) have been changed to expect an error not to be raised.