[libcu++] Use static call operator when possible - #10711
Open
miscco wants to merge 1 commit into
Open
Conversation
Contributor
|
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:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds static call-operator support for tile compilation, replaces internal CPO calls with public ranges APIs, updates library components and CUDA PSTL dispatchers, and adjusts range tests and compatibility annotations. ChangesRanges CPO migration
Possibly related PRs
Suggested reviewers: Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
libcudacxx/include/cuda/std/__iterator/iter_swap.h (1)
75-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuesuggestion: Replace each annotated branch comment with
_CCCL_HAS_NOEXCEPT_MANGLING(). Lines 75 and 99 also include the negated condition. Line 109 only includes the negated condition. As per coding guidelines, an annotated#elseor#endifcomment must repeat the exact#ifcondition and must not use an inverted form.Also applies to: 99-99, 109-109
Sources: Coding guidelines, Learnings
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 39b955af-4017-4301-a9aa-ecb66fb0b129
📒 Files selected for processing (143)
libcudacxx/include/cuda/__container/buffer.hlibcudacxx/include/cuda/__functional/always_true_false.hlibcudacxx/include/cuda/__functional/call_or.hlibcudacxx/include/cuda/__functional/lazy_call_or.hlibcudacxx/include/cuda/__functional/maximum.hlibcudacxx/include/cuda/__functional/minimum.hlibcudacxx/include/cuda/__iterator/zip_common.hlibcudacxx/include/cuda/__iterator/zip_iterator.hlibcudacxx/include/cuda/__memory_resource/get_memory_resource.hlibcudacxx/include/cuda/__stream/get_stream.hlibcudacxx/include/cuda/std/__algorithm/comp.hlibcudacxx/include/cuda/std/__algorithm/iter_swap.hlibcudacxx/include/cuda/std/__algorithm/iterator_operations.hlibcudacxx/include/cuda/std/__algorithm/ranges_find_if.hlibcudacxx/include/cuda/std/__algorithm/ranges_find_if_not.hlibcudacxx/include/cuda/std/__algorithm/ranges_for_each.hlibcudacxx/include/cuda/std/__algorithm/ranges_for_each_n.hlibcudacxx/include/cuda/std/__algorithm/ranges_min.hlibcudacxx/include/cuda/std/__algorithm/ranges_min_element.hlibcudacxx/include/cuda/std/__algorithm/unwrap_range.hlibcudacxx/include/cuda/std/__cccl/dialect.hlibcudacxx/include/cuda/std/__concepts/swappable.hlibcudacxx/include/cuda/std/__expected/expected_base.hlibcudacxx/include/cuda/std/__format/format_spec_parser.hlibcudacxx/include/cuda/std/__format/output_utils.hlibcudacxx/include/cuda/std/__functional/bind_back.hlibcudacxx/include/cuda/std/__functional/bind_front.hlibcudacxx/include/cuda/std/__functional/compose.hlibcudacxx/include/cuda/std/__functional/identity.hlibcudacxx/include/cuda/std/__functional/operations.hlibcudacxx/include/cuda/std/__functional/ranges_operations.hlibcudacxx/include/cuda/std/__iterator/access.hlibcudacxx/include/cuda/std/__iterator/advance.hlibcudacxx/include/cuda/std/__iterator/common_iterator.hlibcudacxx/include/cuda/std/__iterator/concepts.hlibcudacxx/include/cuda/std/__iterator/counted_iterator.hlibcudacxx/include/cuda/std/__iterator/distance.hlibcudacxx/include/cuda/std/__iterator/iter_move.hlibcudacxx/include/cuda/std/__iterator/iter_swap.hlibcudacxx/include/cuda/std/__iterator/move_iterator.hlibcudacxx/include/cuda/std/__iterator/next.hlibcudacxx/include/cuda/std/__iterator/prev.hlibcudacxx/include/cuda/std/__iterator/reverse_access.hlibcudacxx/include/cuda/std/__iterator/reverse_iterator.hlibcudacxx/include/cuda/std/__iterator/variant_like.hlibcudacxx/include/cuda/std/__linalg/conj_if_needed.hlibcudacxx/include/cuda/std/__memory/construct_at.hlibcudacxx/include/cuda/std/__memory/temporary_buffer.hlibcudacxx/include/cuda/std/__memory/uninitialized_algorithms.hlibcudacxx/include/cuda/std/__memory/unique_ptr.hlibcudacxx/include/cuda/std/__optional/hash.hlibcudacxx/include/cuda/std/__pstl/cuda/adjacent_difference.hlibcudacxx/include/cuda/std/__pstl/cuda/copy_if.hlibcudacxx/include/cuda/std/__pstl/cuda/copy_n.hlibcudacxx/include/cuda/std/__pstl/cuda/exclusive_scan.hlibcudacxx/include/cuda/std/__pstl/cuda/find_if.hlibcudacxx/include/cuda/std/__pstl/cuda/for_each_n.hlibcudacxx/include/cuda/std/__pstl/cuda/generate_n.hlibcudacxx/include/cuda/std/__pstl/cuda/inclusive_scan.hlibcudacxx/include/cuda/std/__pstl/cuda/max_element.hlibcudacxx/include/cuda/std/__pstl/cuda/merge.hlibcudacxx/include/cuda/std/__pstl/cuda/min_element.hlibcudacxx/include/cuda/std/__pstl/cuda/partition.hlibcudacxx/include/cuda/std/__pstl/cuda/partition_copy.hlibcudacxx/include/cuda/std/__pstl/cuda/reduce.hlibcudacxx/include/cuda/std/__pstl/cuda/remove_if.hlibcudacxx/include/cuda/std/__pstl/cuda/rotate.hlibcudacxx/include/cuda/std/__pstl/cuda/rotate_copy.hlibcudacxx/include/cuda/std/__pstl/cuda/shift_left.hlibcudacxx/include/cuda/std/__pstl/cuda/shift_right.hlibcudacxx/include/cuda/std/__pstl/cuda/sort.hlibcudacxx/include/cuda/std/__pstl/cuda/stable_partition.hlibcudacxx/include/cuda/std/__pstl/cuda/transform.hlibcudacxx/include/cuda/std/__pstl/cuda/transform_reduce.hlibcudacxx/include/cuda/std/__pstl/cuda/unique.hlibcudacxx/include/cuda/std/__pstl/cuda/unique_copy.hlibcudacxx/include/cuda/std/__pstl/reverse.hlibcudacxx/include/cuda/std/__pstl/swap_ranges.hlibcudacxx/include/cuda/std/__ranges/access.hlibcudacxx/include/cuda/std/__ranges/all.hlibcudacxx/include/cuda/std/__ranges/common_view.hlibcudacxx/include/cuda/std/__ranges/concepts.hlibcudacxx/include/cuda/std/__ranges/counted.hlibcudacxx/include/cuda/std/__ranges/data.hlibcudacxx/include/cuda/std/__ranges/drop_view.hlibcudacxx/include/cuda/std/__ranges/drop_while_view.hlibcudacxx/include/cuda/std/__ranges/empty.hlibcudacxx/include/cuda/std/__ranges/filter_view.hlibcudacxx/include/cuda/std/__ranges/iota_view.hlibcudacxx/include/cuda/std/__ranges/owning_view.hlibcudacxx/include/cuda/std/__ranges/rbegin.hlibcudacxx/include/cuda/std/__ranges/ref_view.hlibcudacxx/include/cuda/std/__ranges/rend.hlibcudacxx/include/cuda/std/__ranges/repeat_view.hlibcudacxx/include/cuda/std/__ranges/reverse_view.hlibcudacxx/include/cuda/std/__ranges/single_view.hlibcudacxx/include/cuda/std/__ranges/size.hlibcudacxx/include/cuda/std/__ranges/subrange.hlibcudacxx/include/cuda/std/__ranges/take_view.hlibcudacxx/include/cuda/std/__ranges/take_while_view.hlibcudacxx/include/cuda/std/__ranges/transform_view.hlibcudacxx/include/cuda/std/__ranges/unwrap_end.hlibcudacxx/include/cuda/std/__ranges/view_interface.hlibcudacxx/include/cuda/std/__ranges/zip_view.hlibcudacxx/include/cuda/std/__simd/basic_vec.hlibcudacxx/include/cuda/std/__simd/complex_math.hlibcudacxx/include/cuda/std/__string/helper_functions.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_cat.hlibcudacxx/include/cuda/std/__type_traits/integral_constant.hlibcudacxx/include/cuda/std/__type_traits/type_list.hlibcudacxx/include/cuda/std/__utility/constant_wrapper.hlibcudacxx/include/cuda/std/__utility/monostate.hlibcudacxx/include/cuda/std/__variant/comparison.hlibcudacxx/include/cuda/std/__variant/hash.hlibcudacxx/include/cuda/std/__variant/variant_base.hlibcudacxx/include/cuda/std/__variant/variant_match.hlibcudacxx/include/cuda/std/bitsetlibcudacxx/include/cuda/std/inplace_vectorlibcudacxx/include/cuda/std/spanlibcudacxx/include/cuda/std/string_viewlibcudacxx/test/libcudacxx/cuda/iterators/zip_iterator/iter_move.pass.cpplibcudacxx/test/libcudacxx/cuda/iterators/zip_iterator/iter_swap.pass.cpplibcudacxx/test/libcudacxx/std/algorithms/alg.modifying/alg.rotate/rotate_copy.pass.cpplibcudacxx/test/libcudacxx/std/containers/sequences/array/iterators.pass.cpplibcudacxx/test/libcudacxx/std/iterators/stream.iterators/iterator.range/begin_array.pass.cpplibcudacxx/test/libcudacxx/std/iterators/stream.iterators/iterator.range/begin_const.pass.cpplibcudacxx/test/libcudacxx/std/iterators/stream.iterators/iterator.range/begin_non_const.pass.cpplibcudacxx/test/libcudacxx/std/iterators/stream.iterators/iterator.range/end_array.pass.cpplibcudacxx/test/libcudacxx/std/iterators/stream.iterators/iterator.range/end_const.pass.cpplibcudacxx/test/libcudacxx/std/iterators/stream.iterators/iterator.range/end_non_const.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.common.view/adaptor.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.drop.while/adaptor.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.drop.while/begin.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.filter/adaptor.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.filter/ctor.default.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.filter/iterator/decrement.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.filter/iterator/iter_swap.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.reverse/adaptor.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.take.while/adaptor.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.take.while/general.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.take/adaptor.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.zip/iterator/iter_move.pass.cpplibcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.zip/iterator/iter_swap.pass.cpp
💤 Files with no reviewable changes (1)
- libcudacxx/test/libcudacxx/std/ranges/range.adaptors/range.zip/iterator/iter_move.pass.cpp
miscco
force-pushed
the
static_call_operator
branch
from
August 7, 2026 10:44
839c710 to
5da0023
Compare
This comment has been minimized.
This comment has been minimized.
miscco
force-pushed
the
static_call_operator
branch
from
August 7, 2026 12:11
5da0023 to
904fe41
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libcudacxx/include/cuda/std/bitset (1)
972-975: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Add
[[nodiscard]]to the hash call operator.Line 972 returns the only value produced by this side-effect-free function. The repository C++ guideline requires
[[nodiscard]]for most non-void functions without known side effects. Add the attribute before_CCCL_API.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 334db880-0995-4298-b258-b796b6d8e3aa
📒 Files selected for processing (10)
libcudacxx/include/cuda/std/__algorithm/iterator_operations.hlibcudacxx/include/cuda/std/__iterator/concepts.hlibcudacxx/include/cuda/std/__memory/construct_at.hlibcudacxx/include/cuda/std/__memory/uninitialized_algorithms.hlibcudacxx/include/cuda/std/__ranges/drop_view.hlibcudacxx/include/cuda/std/__ranges/repeat_view.hlibcudacxx/include/cuda/std/__ranges/reverse_view.hlibcudacxx/include/cuda/std/bitsetlibcudacxx/include/cuda/std/inplace_vectorlibcudacxx/test/libcudacxx/std/containers/sequences/array/iterators.pass.cpp
🚧 Files skipped from review as they are similar to previous changes (8)
- libcudacxx/include/cuda/std/__ranges/repeat_view.h
- libcudacxx/include/cuda/std/__memory/construct_at.h
- libcudacxx/include/cuda/std/inplace_vector
- libcudacxx/include/cuda/std/__ranges/drop_view.h
- libcudacxx/include/cuda/std/__memory/uninitialized_algorithms.h
- libcudacxx/test/libcudacxx/std/containers/sequences/array/iterators.pass.cpp
- libcudacxx/include/cuda/std/__ranges/reverse_view.h
- libcudacxx/include/cuda/std/__iterator/concepts.h
miscco
force-pushed
the
static_call_operator
branch
from
August 7, 2026 12:35
904fe41 to
3d2194f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
miscco
force-pushed
the
static_call_operator
branch
from
August 7, 2026 14:26
3d2194f to
e7fef78
Compare
This comment has been minimized.
This comment has been minimized.
We want to avoid having to take the address of the global object because that gives us a lot of grief both on device and tile
miscco
force-pushed
the
static_call_operator
branch
from
August 7, 2026 16:12
e7fef78 to
4d9cf17
Compare
Contributor
⏱️ CCCL compile-time benchmark comparison: Public headers compile-time benchResult: 1 regression row(s), 7 improvement row(s) above threshold.
Artifacts: reports and traces Direct file processing
🔴 Direct file processing — Regressions
🟢 Direct file processing — Improvements
|
Contributor
😬 CI Workflow Results🟥 Finished in 3h 17m: Pass: 84%/325 | Total: 4d 10h | Max: 3h 17m | Hits: 61%/940245See results here. AI failure analysis1. Static call-operator refactor breaks NVCC registration of wrapped execution constants · 32 jobsExplanation: All 32 CUB/MSVC matrix jobs fail while compiling top-k tests because NVCC-generated registration stubs pass `__constant_wrapper` objects where the CUDA runtime expects raw variable addresses. This prevents every CUB launch-mode configuration from building. Evidence: CUB nvcc MSVC / [CTK12.0 MSVC14.29 C++17] BuildDeviceLaunch(amd64), step 4 Root cause: The PR converts `__constant_wrapper::operator()` and many callable objects to `_CCCL_STATIC_CALL_OPERATOR`; under NVCC/MSVC this changes device-code generation around wrapped global execution constants such as `output_ordering::sorted`, producing invalid `__cudaRegisterVar` calls. The logs do not contain a minimized compilation that proves which individual conversion triggers the compiler behavior, so the exact compatibility guard still needs narrow verification. Sources: libcudacxx/include/cuda/std/__utility/constant_wrapper.h:374, libcudacxx/include/cuda/std/__cccl/dialect.h:150, libcudacxx/include/cuda/__execution/output_ordering.h:70. Suggested next steps: Reproduce with `& "./ci/windows/build_cub.ps1" -lid1 -std "17"` or first build only `cub.test.device.batched_topk_api.lid_0`. Restore the explicit member/static implementation in `__constant_wrapper` or add an NVCC/MSVC compatibility guard, then verify one CUDA 12.x and one CUDA 13.x matrix entry before rerunning the full CUB matrix. Copy this prompt into a coding agentJobs:
2. Static default deleter leaves conversion-test variable unused under warnings-as-errors · 16 jobsExplanation: All 16 libcu++ jobs fail the same `default_delete` conversion test because `d1(p)` invokes a newly static call operator and therefore no longer counts as using the `d1` object. NVCC promotes the resulting unused-variable warning to an error. Evidence: libcu++ nvcc Clang / [CTK13.3 Clang17 C++20] Build(amd64), step 4 Root cause: The PR changes `default_delete::operator()` from a const member function to `_CCCL_STATIC_CALL_OPERATOR`. The test still constructs `d1` to verify the converting constructor, but calling the static operator through `d1` does not constitute an object use, triggering diagnostic 550-D under `-Werror=all-warnings`. Sources: libcudacxx/include/cuda/std/__memory/unique_ptr.h:77, libcudacxx/test/libcudacxx/std/utilities/memory/smartptr/unique.ptr/unique.ptr.dltr/unique.ptr.dltr.dflt/convert_ctor.pass.cpp:32. Suggested next steps: Explicitly consume the converted object in the test, for example by adding `(void) d1;` after its construction, while retaining `d1(p)` to test invocation. Run `./ci/build_libcudacxx.sh -std 20` or the single `convert_ctor.pass.cpp` lit test to verify the warning is gone. Copy this prompt into a coding agentJobs:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We have the issue that our CPOs are not tile accessible objects
However, we actually never need that because their call operator could be static.
So make it so that it is always static if used like this