Open
Conversation
This commit adds a field is_parallel_loop_header to BasicBlockData representing whether the basic block is a header to a Tapir parallel loop. Its value is computed based on whether the loop corresponds to a cilk_for, in which case the header of the loop has is_parallel_loop_header set to true. This will be used later in lowering from MIR to LLVM IR to emit the correct metadata. Whether the loop corresponds to a cilk_for is maintained through thir::ExprKind::Loop::tapir_loop_spawn, which is true when the loop corresponds to a cilk_for. ExprKind::Loop spawns the body when LoopSource is CilkFor, so lowering to MIR is also responsible for syncing the body.
This commit adds the method Builder::tapir_loop_spawn_strategy_metadata to annotate loops with a given spawning strategy. The attribute is still not added for branches at the end of loop headers. This commit also adds the MD_loop metadata kind and uses it for the annotation.
This commit annotates the branch out of the loop header with parallel loop metadata if the loop header is the header of a parallel loop.
This commit lowers parsed cilk_fors to HIR by slightly modifying
the standard for-loop lowering to loop {}. Instead of lowering to
loop { match next() { Some(..) => <body>, None => break } }, the body
is spawned (but only in the Some(..) case). This means forbidding
control flow in the body of a cilk_for should be sufficient. This
commit additionally removes the CilkSpawn introduced when lowering
HIR to THIR for parallel loops, since the body is already being
spawned.
This commit makes sure that no part of the spawned task tries to escape the loop or return from the function, which isn't allowed within a cilk_for loop, for the standard desugaring.
This commit changes CFG simplification to maintain information about parallel loop headers through the optimization. This is essential because parallel loop header metadata changes the outputted metadata of the LLVM IR to specify Tapir's loop spawning strategy. This commit also adds additional debugging information to CFG simplification since I expect that the solution used here is relatively hacky. Whenever the successor of a terminator changes (i.e. there exists a goto chain from the old successor to the new one) and the old successor is a parallel loop header, and the old successor has no more predecessors, the new successor must be a parallel loop header.
This commit changes the place we attach parallel loop metadata to the back-edge, which we compute at codegen time since we don't explicitly need to know about parallel back-edges at an earlier point.
This commit adds tests for disallowed control flow such as return and break in cilk_for and cilk_spawn. For continue, we expect cilk_for to allow continues within it, but not cilk_spawns presently.
This commit changes the structure of metadata associated with a cilk_for
loop (or the loop that eventually results from one in the emitted IR, to
be more precise) so that each item is a metadata node:
!14 = distinct !{ !14, !15 }
!15 = {!"tapir.loop.spawn.strategy", i32 1 }
compared to the existing
!14 = {!"tapir.loop.spawn.strategy", i32 1 }
without the self-referential metadata.
aleph-oh
commented
Jul 18, 2024
Owner
Author
aleph-oh
left a comment
aleph-oh
left a comment
There was a problem hiding this comment.
A few nits, fix please :)
aleph-oh
commented
Jul 18, 2024
aleph-oh
commented
Jul 18, 2024
aleph-oh
commented
Jul 18, 2024
Owner
Author
|
I'd also like it if there was some function whose purpose it was to insert self-references into metadata. It's okay if it's unsafe, but it's also less badly unsafe than the unrestricted FFI version. Maybe |
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 free
to 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 adds support for the cilk_for keyword. I only expect it to work when iterating over a range where the operation per-element is thread-safe and does not contain a
continue,break, orreturn. It additionally lowers parallel loops to use divide-and-conquer parallelism rather than serially spawning each loop iteration through Tapir hints.