Skip to content

[cudax] Delete group's copy/move cons + assign op - #11216

Open
davebayer wants to merge 1 commit into
NVIDIA:mainfrom
davebayer:groups_non_copyable
Open

[cudax] Delete group's copy/move cons + assign op#11216
davebayer wants to merge 1 commit into
NVIDIA:mainfrom
davebayer:groups_non_copyable

Conversation

@davebayer

Copy link
Copy Markdown
Contributor

It doesn't make sense to have a copy constructor for a group, because you wouldn't be able to supply a new synchronizer for the group.

Moving a group could work, but it brings problems regarding conditional synchronizer destruction and synchronization, because a synchronizer can be in a moved-from state, which would cost some performance. And I don't have any use case for it yet.

Assigning a group is problematic, because basically every group has it's own signature, so it would work only under some very specific conditions.

So my idea is that if you need to pass a group, pass it by reference. If you need to pass it by copy, just pass a group_view.

@davebayer
davebayer requested a review from a team as a code owner September 4, 2026 15:03
@davebayer
davebayer requested a review from andralex September 4, 2026 15:03
@github-project-automation github-project-automation Bot moved this to Todo in CCCL Sep 4, 2026
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Sep 4, 2026
@davebayer
davebayer enabled auto-merge (squash) September 4, 2026 15:07
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4bd9f37c-20eb-4eaa-8dd8-defa77928dd4

📥 Commits

Reviewing files that changed from the base of the PR and between 0941c14 and bd5a525.

📒 Files selected for processing (1)
  • cudax/include/cuda/experimental/__group/synchronizer/barrier_synchronizer.cuh

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Summary

Summary by CodeRabbit

  • API Changes
    • Group-based reduction and summation operations now accept group objects by reference.
    • Group objects and barrier synchronizers can no longer be copied, moved, or assigned.
    • Barrier synchronizers now provide a dedicated non-owning view.
  • Behavior
    • Reduction supports groups without statically known sizes.
    • Reduction results and synchronization behavior remain unchanged.
  • Tests
    • Updated coverage verifies group references and barrier synchronizer view behavior.

Walkthrough

Changes

Cooperative group types are now non-copyable. Reduction and sum overloads accept groups by const reference. Warp reduction uses group_view, and barrier synchronizers provide non-owning views with updated deinitialization behavior.

Cooperative group API

Layer / File(s) Summary
Group object lifetime and barrier views
cudax/include/cuda/experimental/__group/group.cuh, cudax/include/cuda/experimental/__group/this_group.cuh, cudax/include/cuda/experimental/__group/virtual_group.cuh, cudax/include/cuda/experimental/__group/synchronizer/barrier_synchronizer.cuh, cudax/test/group/synchronizer/barrier_synchronizer.cu
Group types and owning barrier synchronizers delete copy and move operations. Barrier synchronizers return a dedicated non-owning view. Deinitialization validates the mapping and destroys the barrier for the designated thread. Tests expect the new view type.
Reduction reference API
cudax/include/cuda/experimental/__coop/reduce.cuh
Internal and public reduction overloads accept groups by const reference. Warp reduction uses group_view for static warp-count lookup. Public static-size assertions are removed.
Sum overload alignment
cudax/test/group/cooperative_algorithm.cu
Thread, warp, block, cluster, and generic sum overloads use const-reference group parameters.

Merge Risk: ⚪ Minimal · up to bd5a5

Groups and owning synchronizers are now non-copyable, with reductions borrowing groups by reference and views providing non-owning synchronization access. The remaining deinitialization concern is addressed by requiring a valid mapping, so no merge-blocking current-head risk remains.


Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8ab2b6dc-2327-435f-8422-c67305be524f

📥 Commits

Reviewing files that changed from the base of the PR and between c6cdc15 and 3067bb6.

📒 Files selected for processing (6)
  • cudax/include/cuda/experimental/__coop/reduce.cuh
  • cudax/include/cuda/experimental/__group/group.cuh
  • cudax/include/cuda/experimental/__group/synchronizer/barrier_synchronizer.cuh
  • cudax/include/cuda/experimental/__group/this_group.cuh
  • cudax/include/cuda/experimental/__group/virtual_group.cuh
  • cudax/test/group/cooperative_algorithm.cu

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Comment thread cudax/include/cuda/experimental/__group/synchronizer/barrier_synchronizer.cuh Outdated
Comment thread cudax/include/cuda/experimental/__group/synchronizer/barrier_synchronizer.cuh Outdated
@davebayer
davebayer disabled auto-merge September 4, 2026 15:42
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4bc342df-1e5b-4ec7-a718-e794702e004b

📥 Commits

Reviewing files that changed from the base of the PR and between 3067bb6 and 0941c14.

📒 Files selected for processing (3)
  • cudax/include/cuda/experimental/__coop/reduce.cuh
  • cudax/include/cuda/experimental/__group/synchronizer/barrier_synchronizer.cuh
  • cudax/test/group/synchronizer/barrier_synchronizer.cu

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Comment thread cudax/include/cuda/experimental/__group/synchronizer/barrier_synchronizer.cuh Outdated
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 1h 08m: Pass: 100%/63 | Total: 12h 46m | Max: 48m 58s | Hits: 63%/56982

See results here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

2 participants