Skip to content

Replace __popc with cude::std::popcounter - #6213

Merged
miscco merged 9 commits into
NVIDIA:mainfrom
viralbhadeshiya:FEA/6107
Oct 22, 2025
Merged

miscco merged 9 commits into
NVIDIA:mainfrom
viralbhadeshiya:FEA/6107

Conversation

@viralbhadeshiya

@viralbhadeshiya viralbhadeshiya commented Oct 14, 2025 •

Copy link
Copy Markdown
Contributor

Description

closes #6107

  • Replace all __popc occurrence with cuda::std::pop_count

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@copy-pr-bot

copy-pr-bot Bot commented Oct 14, 2025

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Oct 14, 2025
@davebayer

Copy link
Copy Markdown
Contributor

/ok to test afd9ae6

@davebayer

Copy link
Copy Markdown
Contributor

Thanks for working on this :)

Comment thread libcudacxx/examples/nv_target.cu Resolved
@github-project-automation github-project-automation Bot moved this from In Review to In Progress in CCCL Oct 14, 2025
@github-actions

This comment has been minimized.

Comment thread libcudacxx/examples/nv_target.cu Resolved
Comment thread libcudacxx/examples/nv_target.cu Resolved

@davebayer davebayer 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.

There is one more __popc(...) left in thrust/thrust/system/cuda/detail/set_operations.h

@viralbhadeshiya
viralbhadeshiya requested a review from a team as a code owner October 14, 2025 15:42
@viralbhadeshiya

Copy link
Copy Markdown
Contributor Author

There is one more __popc(...) left in thrust/thrust/system/cuda/detail/set_operations.h

This is done.

@davebayer

Copy link
Copy Markdown
Contributor

/ok to test 00154d6

@github-actions

This comment has been minimized.

@davebayer
davebayer requested a review from miscco October 14, 2025 18:07
@miscco

miscco commented Oct 15, 2025

Copy link
Copy Markdown
Contributor

@elstehle can I bully you into checking SASS?

@elstehle elstehle 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.

Confirmed that there are no sass changes.

Fyi, these are the steps I usually follow to check whether there are any sass changes:

  1. Identify the Device* algorithm(s) that may be affected by a change. This isn't always straight-forward. Sometimes, the change may only be affected on certain GPU architectures (this would require looking into the tuning policy of affected algorithms)
  2. Compile the benchmarks for the specific Device* algorithm(s) identified in (1) and dump the sass code. E.g., ninja cub.bench.radix_sort.keys.base && cuobjdump -sass ./bin/cub.bench.radix_sort.keys.base |c++filt > ./radix_sort.keys_after.sass.
  3. Checkout the main branch to compare against baseline sass code being generated: git checkout $(git merge-base HEAD upstream/main)
  4. Dump the sass code emitted on main branch. E.g., ninja cub.bench.radix_sort.keys.base && cuobjdump -sass ./bin/cub.bench.radix_sort.keys.base |c++filt > ./radix_sort.keys_before.sass.
  5. Check whether there are sass changes: git diff --text --no-index --word-diff radix_sort.keys_before.sass radix_sort.keys_after.sass

@viralbhadeshiya

Copy link
Copy Markdown
Contributor Author

can we run test on this PR? other then that I think everything is done.

@davebayer

Copy link
Copy Markdown
Contributor

/ok to test ff78f01

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 18h 40m: Pass: 100%/176 | Total: 9d 04h | Max: 4h 48m | Hits: 62%/358506

See results here.

@github-project-automation github-project-automation Bot moved this from In Progress to In Review in CCCL Oct 22, 2025
@miscco
miscco merged commit 638c03d into NVIDIA:main Oct 22, 2025
557 of 564 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in CCCL Oct 22, 2025
@miscco

miscco commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

Thanks a lot @viralbhadeshiya for working on this 🎉

pciolkosz pushed a commit to pciolkosz/cccl that referenced this pull request Oct 23, 2025
---------

Co-authored-by: David Bayer <48736217+davebayer@users.noreply.github.com>
pciolkosz pushed a commit to pciolkosz/cccl that referenced this pull request Oct 24, 2025
---------

Co-authored-by: David Bayer <48736217+davebayer@users.noreply.github.com>
Naraenda pushed a commit to ROCm/rocm-libraries that referenced this pull request Oct 26, 2026
…d::popcounter (NVIDIA/cccl#6213)

Upstream: NVIDIA/cccl@638c03d

HIP: no change; hip/detail/set_operations.h keeps the HIP __popc intrinsic (valid on all HIP targets; switching to _THRUST_STD::popcount would require libhipcxx and gives no correctness benefit). system/cuda: __popc -> ::cuda::std::popcount. test/: no counterpart. testing/: no counterpart. Not compiled, tests not run (per human)

Co-Authored-By: Claude <noreply@anthropic.com>
amd-jmahovsky added a commit to ROCm/rocm-libraries that referenced this pull request Oct 28, 2026
…unt in hip set_operations

Related upstream commit: 638c03d54f7 Replace __popc with cude::std::popcounter (NVIDIA/cccl#6213)
Related local port commit: baac406

The port changed only system/cuda. libhipcxx is now mandatory, so system/hip/detail/set_operations.h
follows upstream: thread_output_count uses
_THRUST_STD::popcount(static_cast<unsigned>(active_mask)) instead of the __popc intrinsic,
includes _THRUST_STD_INCLUDE(__bit/popcount.h), and the max/min includes are no longer guarded
(the !_THRUST_HAS_DEVICE_SYSTEM_STD algorithm_wrapper.h fallback is removed).

Not compiled, tests not run (per human)

Co-Authored-By: Claude <noreply@anthropic.com>
Naraenda pushed a commit to ROCm/rocm-libraries that referenced this pull request Oct 29, 2026
…d::popcounter (NVIDIA/cccl#6213)

Upstream: NVIDIA/cccl@638c03d

HIP: no change; hip/detail/set_operations.h keeps the HIP __popc intrinsic (valid on all HIP targets; switching to _THRUST_STD::popcount would require libhipcxx and gives no correctness benefit). system/cuda: __popc -> ::cuda::std::popcount. test/: no counterpart. testing/: no counterpart. Not compiled, tests not run (per human)

Co-Authored-By: Claude <noreply@anthropic.com>
Naraenda pushed a commit to ROCm/rocm-libraries that referenced this pull request Oct 29, 2026
…d::popcounter (NVIDIA/cccl#6213)

Upstream: NVIDIA/cccl@638c03d

HIP: no change; hip/detail/set_operations.h keeps the HIP __popc intrinsic (valid on all HIP targets; switching to _THRUST_STD::popcount would require libhipcxx and gives no correctness benefit). system/cuda: __popc -> ::cuda::std::popcount. test/: no counterpart. testing/: no counterpart. Not compiled, tests not run (per human)

Co-Authored-By: Claude <noreply@anthropic.com>
Naraenda pushed a commit to ROCm/rocm-libraries that referenced this pull request Oct 29, 2026
…unt in hip set_operations

Related upstream commit: 638c03d54f7 Replace __popc with cude::std::popcounter (NVIDIA/cccl#6213)
Related local port commit: baac406

The port changed only system/cuda. libhipcxx is now mandatory, so system/hip/detail/set_operations.h
follows upstream: thread_output_count uses
_THRUST_STD::popcount(static_cast<unsigned>(active_mask)) instead of the __popc intrinsic,
includes _THRUST_STD_INCLUDE(__bit/popcount.h), and the max/min includes are no longer guarded
(the !_THRUST_HAS_DEVICE_SYSTEM_STD algorithm_wrapper.h fallback is removed).

Not compiled, tests not run (per human)

Co-Authored-By: Claude <noreply@anthropic.com>
Naraenda pushed a commit to ROCm/rocm-libraries that referenced this pull request Oct 30, 2026
…d::popcounter (NVIDIA/cccl#6213)

Upstream: NVIDIA/cccl@638c03d

HIP: no change; hip/detail/set_operations.h keeps the HIP __popc intrinsic (valid on all HIP targets; switching to _THRUST_STD::popcount would require libhipcxx and gives no correctness benefit). system/cuda: __popc -> ::cuda::std::popcount. test/: no counterpart. testing/: no counterpart. Not compiled, tests not run (per human)

Co-Authored-By: Claude <noreply@anthropic.com>
Beanavil pushed a commit to ROCm/rocm-libraries that referenced this pull request Oct 30, 2026
…d::popcounter (NVIDIA/cccl#6213)

Upstream: NVIDIA/cccl@638c03d

HIP: no change; hip/detail/set_operations.h keeps the HIP __popc intrinsic (valid on all HIP targets; switching to _THRUST_STD::popcount would require libhipcxx and gives no correctness benefit). system/cuda: __popc -> ::cuda::std::popcount. test/: no counterpart. testing/: no counterpart. Not compiled, tests not run (per human)

Co-Authored-By: Claude <noreply@anthropic.com>
amd-jmahovsky added a commit to ROCm/rocm-libraries that referenced this pull request Oct 30, 2026
…unt in hip set_operations

Related upstream commit: 638c03d54f7 Replace __popc with cude::std::popcounter (NVIDIA/cccl#6213)
Related local port commit: baac406

The port changed only system/cuda. libhipcxx is now mandatory, so system/hip/detail/set_operations.h
follows upstream: thread_output_count uses
_THRUST_STD::popcount(static_cast<unsigned>(active_mask)) instead of the __popc intrinsic,
includes _THRUST_STD_INCLUDE(__bit/popcount.h), and the max/min includes are no longer guarded
(the !_THRUST_HAS_DEVICE_SYSTEM_STD algorithm_wrapper.h fallback is removed).

Not compiled, tests not run (per human)

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[FEA]: Replaces uses of __popc with cuda::std::popcount

6 participants