[libcu++] Make constant_wrapper work with nvrtc and without warnings suppressions - #10709
[libcu++] Make constant_wrapper work with nvrtc and without warnings suppressions#10709davebayer wants to merge 1 commit into
constant_wrapper work with nvrtc and without warnings suppressions#10709Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe constant-wrapper implementation adds Constant wrapper implementation
Constant wrapper tests
Possibly related PRs
Suggested reviewers: Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
libcudacxx/include/cuda/std/__utility/constant_wrapper.h (2)
368-371: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Add
[[nodiscard]]to__get(). It returns a reference and has no side effect. As per coding guidelines, “Most functions with a non-void return type should use[[nodiscard]], except for functions with known side effects.”Sources: Coding guidelines, Path instructions
323-345: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Remove negated conditions from annotated
#elseand#endifcomments. Repeat the original#ifcondition exactly.
libcudacxx/include/cuda/std/__utility/constant_wrapper.h#L323-L345: Update the comments on Lines 329, 334, 342, and 345 to use only_CCCL_HAS_MULTIARG_OPERATOR_BRACKETS().libcudacxx/include/cuda/std/__utility/constant_wrapper.h#L373-L457: Update the comments on Lines 378, 380, 391, 393, 407, 421, 428, 436, 450, and 457 to use only their corresponding original condition.Based on learnings, annotated
#elseand#endifcomments must repeat the exact condition text and must not use an inverted condition.Source: Learnings
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5ba5c1c1-020d-43cd-b7cc-112693135d75
📒 Files selected for processing (17)
libcudacxx/include/cuda/std/__utility/constant_wrapper.hlibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/adl.compile.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/assign.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/binary_ops.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/call.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/comma.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/comp.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/convert.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/cw.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/general.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/kernel_parameter.array.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/kernel_parameter.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/mem_ptr.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/pseudo_mutators.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/subscript.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/types.compile.pass.cpplibcudacxx/test/libcudacxx/std/utilities/const.wrap.class/unary_ops.pass.cpp
💤 Files with no reviewable changes (1)
- libcudacxx/test/libcudacxx/std/utilities/const.wrap.class/general.pass.cpp
⏱️ CCCL compile-time benchmark comparison: Public headers compile-time benchResult: 2 regression row(s), 4 improvement row(s) above threshold.
Artifacts: reports and traces TU total compilation
🔴 TU total compilation — Regressions
🟢 TU total compilation — Improvements
Direct file processing
🔴 Direct file processing — Regressions
🟢 Direct file processing — Improvements
|
😬 CI Workflow Results🟥 Finished in 3h 21m: Pass: 91%/115 | Total: 4d 23h | Max: 3h 20m | Hits: 51%/1107006See results here. AI failure analysis1. `constant_wrapper` conversion no longer constant-evaluates under MSVC · 5 jobsExplanation: Five nvcc/MSVC configurations reject `const.wrap.class` static assertions after the wrapper conversion and accessor return types changed from `decltype(value)` to `decltype((_Xp))`. This breaks compile-time use of otherwise correctly typed arithmetic results and conversions. Evidence: libcu++ nvcc MSVC / [CTK12.0 MSVC14.39 C++20] Build(amd64), step 4 libcu++ nvcc MSVC / [CTK13.3 MSVC14.44 C++20] Build(amd64), step 4 Root cause: The PR changed both the conversion operator and new `__get()` accessor to return `decltype((_Xp))` directly. nvcc-generated MSVC host code does not preserve the prior constant-evaluation behavior for these expressions, while the original `decltype(value)` return type encoded the wrapper's intended value-versus-reference semantics. Sources: libcudacxx/include/cuda/std/__utility/constant_wrapper.h:363, libcudacxx/include/cuda/std/__utility/constant_wrapper.h:368, libcudacxx/test/libcudacxx/std/utilities/const.wrap.class/binary_ops.pass.cpp:444, libcudacxx/test/libcudacxx/std/utilities/const.wrap.class/convert.pass.cpp:99. Suggested next steps: Restore `decltype(value)` as the return type of the conversion operator and `__get()`, while continuing to return `(_Xp)` so device code does not read the static member. Run the focused `const.wrap.class` tests with one nvcc/MSVC configuration before rechecking the remaining Windows matrix entries. Copy this prompt into a coding agentJobs: 2. Address-of test takes the wrapper temporary instead of its value · 4 jobsExplanation: Four nvcc/GCC configurations fail the same newly modified identity test because `&cuda::std::__constant_wrapper<V>{}` applies built-in address-of to a temporary wrapper rather than obtaining the address of the wrapped template-parameter object. Compilation stops before the static assertion can be evaluated. Evidence: libcu++ nvcc GCC / [CTK13.0 GCC11 C++20] Build(amd64), step 4 libcu++ nvcc GCC / [CTK13.3 GCC11 C++20] Build(amd64), step 4 Root cause: The PR replaced `&cuda::std::__constant_wrapper<V>::value` with `&cuda::std::__constant_wrapper<V>{}`. GCC resolves this as the address of the temporary wrapper, producing both an rvalue-address error and a pointer-type mismatch instead of testing the wrapped object's identity. Sources: libcudacxx/test/libcudacxx/std/utilities/const.wrap.class/types.compile.pass.cpp:61, libcudacxx/include/cuda/std/__utility/constant_wrapper.h:368. Suggested next steps: Change the comparison to take the address of the accessor result, for example `&V == &cuda::std::__constant_wrapper<V>::__get()`, or restore the original `.value` expression if static-member access is acceptable. Compile only `types.compile.pass.cpp` with nvcc/GCC first, then run the focused `const.wrap.class` suite. Copy this prompt into a coding agentJobs: 3. Windows CUB job could not acquire AWS cache credentials · 1 jobExplanation: The CUB job failed before fetching its container or starting the build because the pinned AWS credentials action ended unsuccessfully. The saved log contains no AWS diagnostic, so the exact OIDC, IAM, or action failure cannot be determined from this run. Evidence: CUB nvcc MSVC / [CTK13.3 MSVC14.50 C++20] BuildNoLaunch(amd64): sm{86}, step 4 Root cause: Credential setup failed inside `aws-actions/configure-aws-credentials`, but the action emitted no saved error message. Because other jobs using the same role proceeded, this is most consistent with a transient runner/OIDC action failure, though missing debug output prevents confirmation. Sources: .github/actions/workflow-run-job-windows/action.yml:89. Suggested next steps: Rerun only this CUB matrix job. If it fails again, enable action and runner debug logging and verify the workflow's `id-token: write` permission, OIDC token availability, and IAM trust conditions before changing build code. Copy this prompt into a coding agentJobs: |
2 participants
This PR enables
constant_wrapperfor nvrtc and removes warnings suppression from the implementation. There are some limitations though. For some reason nvrtc doesn't like thestatic constexprvaluemember being instantiated by any non-trivial types without-default-deviceflag. So basically it has the same limitations as C++17 mode.Removing warnings suppressions regarding accessing
__host__variable from__device__/__global__functions have very similar effect. I found a workaround with a__get()staticmember function that returns the reference to the template parameter instead.If we merge this PR, we can make
constant_wrapperpublic. There will be just some compiler limitations, which are not fixable from our side.