[cudax] Implement cudax::interwarp_synchronizer - #11211
Conversation
c540f53 to
9aa72e4
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 7 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughChangesThe PR adds Interwarp synchronizer
Merge Risk: 🟡 Moderate · up to The new interwarp synchronizer may fail for invalid group mappings or certain otherwise accepted barrier-ID ranges. These API correctness issues should be resolved before merge. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cde92d75-e1e5-47c5-89c2-c2f4b7539157
📒 Files selected for processing (5)
cudax/include/cuda/experimental/__group/fwd.cuhcudax/include/cuda/experimental/__group/synchronizer/interwarp_synchronizer.cuhcudax/include/cuda/experimental/group.cuhcudax/test/CMakeLists.txtcudax/test/group/synchronizer/interwarp_synchronizer.cu
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
cudax/include/cuda/experimental/__group/synchronizer/interwarp_synchronizer.cuh (2)
117-118: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winimportant: Handle invalid mappings before advancing the range.
For
group_by<2, false>over three warps, the invalid warp can haveis_valid() == falseandgroup_rank() == 0xffff'ffff. Line 117 advances from the range start by that sentinel, and Line 118 dereferences the resulting iterator. Return an inert__synchronizer_instancebefore advancing. The supplied test atcudax/test/group/synchronizer/interwarp_synchronizer.cu:77-130does not cover this path.
118-118: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winimportant: Enforce unique barrier IDs for active groups.
make_instanceselects one ID per group rank, but no check rejects duplicate IDs. Two groups can then target the same CTA barrier resource, allowing arrivals from one group to satisfy the other group's barrier count. This can release a group before all of its members arrive. Reject duplicates or enforce uniqueness as an API precondition. The supplied test uses distinct IDs and does not cover this case. (docs.nvidia.com)Source: MCP tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8b0b23dc-77ca-451d-977b-072d8f2a9a37
📒 Files selected for processing (1)
cudax/include/cuda/experimental/__group/synchronizer/interwarp_synchronizer.cuh
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.
9aa72e4 to
e7b7a2c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 11e8252a-8f29-431f-a907-7fe2c25c8016
📒 Files selected for processing (2)
cudax/include/cuda/experimental/__group/synchronizer/interwarp_synchronizer.cuhcudax/include/cuda/experimental/__group/synchronizer/level_synchronizer.cuh
Included review availability: Your plan provides up to 12 included reviews per hour; 8 remain after this review.
1d628e8 to
7028b82
Compare
1be7d2d to
f4a9311
Compare
🥳 CI Workflow Results🟩 Finished in 1h 36m: Pass: 100%/63 | Total: 14h 17m | Max: 54m 27s | Hits: 60%/59452See results here. |
1 participant
This PR implements
interwarp_synchronizersynchronizer that allows you to use the native warp barriers as for synchronizing warps within a single block.The idea is that the user passes a range of barrier ids that must be convertible to
uint32_t, which are split among the newly created groups. The barrier id is then used in__barrier_sync_count(...)call.Using this synchronizer requires some deeper knowledge of the device architecture and it is expected to be used by advanced users.