Skip to content

[cudax] Change the group mapping application logic#8755

Open
davebayer wants to merge 3 commits intoNVIDIA:mainfrom
davebayer:groups_change_mapping
Open

[cudax] Change the group mapping application logic#8755
davebayer wants to merge 3 commits intoNVIDIA:mainfrom
davebayer:groups_change_mapping

Conversation

@davebayer
Copy link
Copy Markdown
Contributor

This PR changes the way mappings are applied during the mapping phase of group creation. I found out it would be better if the mappings worked as transformers that take the previous mapping result and return a modified mapping result. This is a step towards allowing combining multiple mappings together to produce 1 group.

@davebayer davebayer requested a review from a team as a code owner April 30, 2026 09:30
@davebayer davebayer requested a review from andralex April 30, 2026 09:30
@github-project-automation github-project-automation Bot moved this to Todo in CCCL Apr 30, 2026
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Apr 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 33m 23s: Pass: 100%/54 | Total: 5h 59m | Max: 32m 53s | Hits: 97%/31859

See results here.

Copy link
Copy Markdown
Contributor

@pciolkosz pciolkosz left a comment

Choose a reason for hiding this comment

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

A bit confused on the testing, but the tests pass, so it probably just confusion and I will approve

@@ -33,11 +33,12 @@ __device__ void test_lane_synchronizer(const Level& level, Config config)
// Test make_instance(...).
{
const auto parent_group = cudax::make_this_group(level, config);
const ThreadsInWarpMappingResult prev_mapping_result;
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.

I don't get why is this always thread in warp mapping, while the parent varies. Applies in other tests too

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