[libcu++] Implement `cuda::ffs` by Aminsed · Pull Request #6192 · NVIDIA/cccl

Aminsed · 2025-10-13T03:21:33Z

Implement type-safe cuda::std::ffs function as replacement for __ffs intrinsic.

Returns 1-based index of first set bit (0 for no bits set)
Works on all platforms and integer types
Handles x == 0 correctly (returns 0)

Fixes #6108

copy-pr-bot · 2025-10-13T03:21:38Z

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.

miscco

Thanks a lot, this is already looking really great 🎉

I have two minor comments, but this is missing tests. Please have a look what we do for e.g countr_zero in libcudacxx/test/libcudacxx/std/numerics/bit/bitops.count/countr_zero.pass.cpp

If you have any questions, feel free to ask

Aminsed · 2025-10-13T14:00:13Z

Addressed all feedback:

Moved to cuda/__bit/ffs.h
Changed to cuda:: namespace
Updated header guards
Using _CCCL_DEVICE_API
Fixed signed cast with make_signed_t
Simplified implementation
Builtin definitions moved to ffs.h
Added comprehensive tests

Aminsed · 2025-10-13T15:53:39Z

Builtins use _CCCL_HAS_BUILTIN inside prologue
Simplified to direct int/long long casts
Flattened ffs() implementation
Added 128-bit integer support with hi/lo split
Tests use assert in constexpr context
Added large value tests with _u128 literals

fbusato

thanks a lot for your contribution. When you get a chance, please also add the documentation

Aminsed · 2025-10-13T19:12:27Z

@fbusato

Removed unnecessary casts
Fixed unsigned shift (avoid UB)
Added static_assert and _CCCL_ASSUME
Simplified MSVC ternary

Skipped:

Using declarations (bit_reverse.h doesn't use them)
Helper extraction (conflicts with @davebayer's flatten request)

Aminsed · 2025-10-19T22:25:30Z

@fbusato @miscco @davebayer Is there any changes I could make to make this PR merge ready?

miscco · 2025-10-20T08:37:25Z

@Aminsed sorry about the delay, this is just a matter of prioritization

fbusato · 2025-10-20T16:08:05Z

/ok to test 73a7a6a

fbusato · 2025-10-20T16:10:02Z

@Aminsed please ping us when you make progress. It is very easy to miss updates on the PR from our side.
Please also mark the comments as resolved after you address them. Thanks again for the contribution.

Aminsed · 2025-11-08T01:53:50Z

@fbusato

For verification, I pulled nvidia/cuda:13.0.2-devel-ubuntu22.04 (NVCC 13.0.88) and built/ran libcudacxx/test/libcudacxx/cuda/bit/ffs.pass.cpp; it passes, so the new cuda::ffs path behaves as expected with the latest toolchain.

What I've learned from the CI run: If we re-run the full libcudacxx presets, CMake still aborts with “cuda_std_20 is not known to CUDA compiler” (NVCC 12.5 and even 13.0 don’t expose that compile feature yet).

jrhemstad · 2025-11-08T12:50:31Z

pre-commit.ci autofix

jrhemstad · 2025-11-08T12:52:03Z

/ok to test fd5edab

Aminsed · 2025-11-16T02:20:41Z

Quick update: I re-ran the full libcudacxx, Thrust, CUB, and cudax presets in the rapidsai/devcontainers:25.12-cpp-llvm20-cuda13.0 image and they’re all green now. I also kicked off the c-parallel preset, but it still fails on existing issues (Thrust warnings treated as errors for unused parameters, plus lambda/index_sequence bugs in c/parallel/src/jit_templates). Those failures happen on current main as well, so I didn’t try to patch them here. Could you please rerun CI when you get a chance?
@fbusato @jrhemstad Thanks

fbusato · 2025-11-17T21:24:13Z

/ok to test 9b5ddd2

miscco · 2025-11-30T10:24:55Z

/ok to test a03dfe1

Aminsed · 2025-11-30T18:05:10Z

The latest CI failures were NVRTC-related: the __ffs_host* functions were missing _CCCL_DEVICE annotations for JIT mode, and I had accidentally used instead of <cuda/std/cstddef> which NVRTC can't find.
@fbusato @miscco Would you mind triggering another CI run when you get a chance?

fbusato · 2025-12-01T19:34:42Z

/ok to test 554a348

fbusato

I see there are multiple changes not related to ffs. Let me update the branch to align the code with the latest version

fbusato · 2025-12-01T19:38:56Z

+}
+
+#if _CCCL_COMPILER(NVRTC)
+[[nodiscard]] _CCCL_DEVICE inline int __ffs_host32(::cuda::std::uint32_t __v) noexcept


I think that merging all __ffs_host* functions with if constexpr (sizeof(T) == X) would result in a shorted/cleaner code, or at least the constant evaluation part

fbusato · 2025-12-01T19:40:30Z

+#else
+[[nodiscard]] _CCCL_HOST_API inline int __ffs_host32(::cuda::std::uint32_t __v) noexcept
+{
+  if (::cuda::std::__cccl_default_is_constant_evaluated())


please use the new macro based on consteval

fbusato · 2025-12-01T19:41:27Z

+#endif // _CCCL_HAS_INT128()
+
+#if _CCCL_CUDA_COMPILATION()
+[[nodiscard]] _CCCL_DEVICE_API inline int __ffs_device32(::cuda::std::uint32_t __v) noexcept


same here. We could merge the function bodies

fbusato · 2025-12-01T19:43:05Z

+{
+  using _Unsigned = ::cuda::std::remove_cv_t<_Tp>;
+
+#if !defined(_CCCL_BUILTIN_IS_CONSTANT_EVALUATED)


we should not check the built-in here. We can use consteval @davebayer

fbusato · 2025-12-01T19:44:04Z

+#  if _CCCL_HAS_INT128()
+  if constexpr (sizeof(_Unsigned) == sizeof(__uint128_t))
+  {
+    NV_IF_ELSE_TARGET(NV_IS_HOST,
+                      (return __detail::__ffs_host128(static_cast<__uint128_t>(__v));),
+                      (return __detail::__ffs_device128(static_cast<__uint128_t>(__v));))
+  }
+#  endif // _CCCL_HAS_INT128()
+
+  if constexpr (sizeof(_Unsigned) <= sizeof(::cuda::std::uint32_t))
+  {
+    NV_IF_ELSE_TARGET(NV_IS_HOST,
+                      (return __detail::__ffs_host32(static_cast<::cuda::std::uint32_t>(__v));),
+                      (return __detail::__ffs_device32(static_cast<::cuda::std::uint32_t>(__v));))
+  }
+  else
+  {
+    NV_IF_ELSE_TARGET(NV_IS_HOST,
+                      (return __detail::__ffs_host64(static_cast<::cuda::std::uint64_t>(__v));),
+                      (return __detail::__ffs_device64(static_cast<::cuda::std::uint64_t>(__v));))


all these functions could be merged after the previous suggestions

fbusato · 2025-12-01T19:50:07Z

/ok to test f896eac

miscco

I am deeply concerned that this PR touches files that are not even remotely related to the topic at hand.

I would strongly encourage you to only touch files that are related to the topic at hand

Aminsed · 2025-12-14T21:11:12Z

@fbusato Addressed your feedback:

Reverted force_include.h changes (unrelated test infrastructure)
Reverted base.pass.cpp changes (unrelated test workaround)

The PR now only contains changes directly related to cuda::ffs:

docs/libcudacxx/extended_api/bit.rst - doc index
docs/libcudacxx/extended_api/bit/ffs.rst - new docs
libcudacxx/include/cuda/__bit/ffs.h - implementation
libcudacxx/include/cuda/bit - include
libcudacxx/include/cuda/pipeline - usage of cuda::ffs
libcudacxx/test/libcudacxx/cuda/bit/ffs.pass.cpp - tests

All tests verified locally:

bitfield
bitmask
bit_reverse
ffs

Regarding the function merging suggestion on line 132 - the current pattern follows the existing structure in the codebase. Happy to refactor if you prefer the if constexpr template pattern used in bit_reverse.h.

Ready for CI when you have a chance!

fbusato · 2025-12-15T23:36:39Z

/ok to test 816159c

fbusato

looks good! I think the refactoring is not mandatory. The code in ffs.h is well-organized at the end.

github-actions · 2025-12-16T05:27:11Z

😬 CI Workflow Results

🟥 Finished in 5h 48m: Pass: 91%/91 | Total: 1d 18h | Max: 3h 09m | Hits: 98%/197722

See results here.

Implements cuda::ffs for unsigned integer types with optimized device implementations using CUDA intrinsics and host implementations using compiler builtins where available. - Uses __ffs intrinsic on device for 32-bit, __ffsll for 64-bit - Uses _CCCL_BUILTIN_FFS/FFSLL on host when available - Falls back to constexpr implementation for compile-time evaluation - Inlines host logic in NV_IF_ELSE_TARGET to avoid NVRTC issues - Includes comprehensive tests and documentation

fbusato · 2026-09-30T23:31:30Z

closing as it is a duplicate of #9609

Aminsed requested a review from a team as a code owner October 13, 2025 03:21

github-project-automation Bot added this to CCCL Oct 13, 2025

Aminsed requested a review from griwes October 13, 2025 03:21

github-project-automation Bot moved this to Todo in CCCL Oct 13, 2025

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

miscco requested changes Oct 13, 2025

View reviewed changes

Comment thread libcudacxx/include/cuda/std/__bit/ffs.h Outdated

Comment thread libcudacxx/include/cuda/std/__bit/ffs.h Outdated

github-project-automation Bot moved this from In Review to In Progress in CCCL Oct 13, 2025

davebayer requested changes Oct 13, 2025

View reviewed changes

Comment thread libcudacxx/include/cuda/__barrier/barrier_block_scope.h Outdated

Comment thread libcudacxx/include/cuda/std/__bit/ffs.h Outdated

Comment thread libcudacxx/include/cuda/std/__bit/ffs.h Outdated

Comment thread libcudacxx/include/cuda/std/__cccl/builtin.h Outdated

Aminsed force-pushed the feature/implement_bit_ffs branch from 02c7d56 to 731da14 Compare October 13, 2025 13:57

Aminsed requested review from davebayer and miscco October 13, 2025 14:35

davebayer requested changes Oct 13, 2025

View reviewed changes

miscco reviewed Oct 13, 2025

View reviewed changes

Comment thread libcudacxx/test/libcudacxx/cuda/bit/ffs.pass.cpp Outdated

Comment thread libcudacxx/test/libcudacxx/cuda/bit/ffs.pass.cpp Outdated

Comment thread libcudacxx/test/libcudacxx/cuda/bit/ffs.pass.cpp Outdated

Aminsed force-pushed the feature/implement_bit_ffs branch from 79ef976 to 69d084e Compare October 13, 2025 15:49

Aminsed requested review from davebayer and miscco October 13, 2025 15:59

fbusato requested changes Oct 13, 2025

View reviewed changes

fbusato changed the title ~~[libcu++] Implement cuda::std::ffs~~ [libcu++] Implement cuda::ffs Oct 13, 2025

Aminsed force-pushed the feature/implement_bit_ffs branch from 69d084e to 73a7a6a Compare October 13, 2025 17:06

Aminsed requested a review from fbusato October 13, 2025 19:13

davebayer requested changes Oct 20, 2025

View reviewed changes

Comment thread libcudacxx/include/cuda/__bit/ffs.h Outdated

Comment thread libcudacxx/include/cuda/__bit/ffs.h Outdated

This comment has been minimized.

Sign in to view

Aminsed requested a review from fbusato November 8, 2025 01:54

This comment has been minimized.

Sign in to view

Aminsed force-pushed the feature/implement_bit_ffs branch from 4c609e6 to 28703eb Compare November 30, 2025 05:04

This comment has been minimized.

Sign in to view

fbusato requested changes Dec 1, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

miscco requested changes Dec 2, 2025

View reviewed changes

Aminsed requested review from fbusato and miscco December 6, 2025 22:53

fbusato requested changes Dec 8, 2025

View reviewed changes

Comment thread libcudacxx/test/libcudacxx/force_include.h Outdated

Comment thread ...xx/test/libcudacxx/std/iterators/predef.iterators/move.iterators/move.sentinel/base.pass.cpp Outdated

Comment thread libcudacxx/include/cuda/__bit/ffs.h

fbusato approved these changes Dec 15, 2025

View reviewed changes

Aminsed force-pushed the feature/implement_bit_ffs branch from 816159c to 8e26465 Compare December 22, 2025 05:57

Merge branch 'main' into feature/implement_bit_ffs

eac0bde

fbusato closed this Sep 30, 2026

Aminsed deleted the feature/implement_bit_ffs branch October 2, 2026 14:40

Conversation

Aminsed commented Oct 13, 2025

Uh oh!

copy-pr-bot Bot commented Oct 13, 2025

Uh oh!

miscco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Aminsed commented Oct 13, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Aminsed commented Oct 13, 2025

Uh oh!

fbusato left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Aminsed commented Oct 13, 2025

Uh oh!

Aminsed commented Oct 19, 2025

Uh oh!

miscco commented Oct 20, 2025

Uh oh!

fbusato commented Oct 20, 2025

Uh oh!

fbusato commented Oct 20, 2025

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Aminsed commented Nov 8, 2025

Uh oh!

jrhemstad commented Nov 8, 2025

Uh oh!

jrhemstad commented Nov 8, 2025

Uh oh!

This comment has been minimized.

Aminsed commented Nov 16, 2025

Uh oh!

fbusato commented Nov 17, 2025

Uh oh!

This comment has been minimized.

miscco commented Nov 30, 2025

Uh oh!

This comment has been minimized.

Aminsed commented Nov 30, 2025

Uh oh!

fbusato commented Dec 1, 2025

Uh oh!

fbusato left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fbusato Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Aminsed commented Dec 14, 2025 •

edited

Loading