Skip to content

[libcu++] Fix __detectably_invalid value returned during constant evaluation#8987

Open
davebayer wants to merge 1 commit into
NVIDIA:mainfrom
davebayer:fix_mdspan_validity_checks
Open

[libcu++] Fix __detectably_invalid value returned during constant evaluation#8987
davebayer wants to merge 1 commit into
NVIDIA:mainfrom
davebayer:fix_mdspan_validity_checks

Conversation

@davebayer
Copy link
Copy Markdown
Contributor

@davebayer davebayer commented May 14, 2026

In host/device mdspans, we check whether the pointer address corresponds with the type in __detectably_invalid accessor method. However, we have no way to find out during constant evaluation. Unfortunately we returned true (always invalid) instead of false (always valid) in this case. This PR fixes that.

@davebayer davebayer requested a review from a team as a code owner May 14, 2026 17:48
@davebayer davebayer requested a review from ericniebler May 14, 2026 17:48
@github-project-automation github-project-automation Bot moved this to Todo in CCCL May 14, 2026
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Review in CCCL May 14, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Corrected accessor validation logic so validity checks yield accurate boolean results across compilation contexts, avoiding spurious device/host usage errors.
    • Improves consistency of host/device memory operation checks without changing public interfaces.

suggestion:

Walkthrough

Three accessor implementations adjust consteval handling: __host_accessor::access wraps a device-target verification inside _CCCL_IF_NOT_CONSTEVAL_DEFAULT, and the __detectably_invalid fallback return value changes from true to false in __host_accessor, __device_accessor, and __managed_accessor.

Changes

Accessor consteval and fallback changes

Layer / File(s) Summary
Host access: device-target check guarded by not-consteval
libcudacxx/include/cuda/__mdspan/host_device_accessor.h
__host_accessor::access (lines ~199-202) now emits the NV_IF_TARGET(NV_IS_DEVICE, ...) verification only inside an _CCCL_IF_NOT_CONSTEVAL_DEFAULT block.
Consteval fallback correction in __detectably_invalid
libcudacxx/include/cuda/__mdspan/host_device_accessor.h
__host_accessor::__detectably_invalid (line ~223), __device_accessor::__detectably_invalid (line ~358), and __managed_accessor::__detectably_invalid (line ~478) change their _CCCL_IF_NOT_CONSTEVAL_DEFAULT fallback return value from true to false.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
libcudacxx/include/cuda/__mdspan/host_device_accessor.h (2)

199-202: ⚡ Quick win

suggestion: add a regression test that exercises __host_accessor::access in constant evaluation and non-constant-evaluation device compilation paths, so the new _CCCL_IF_NOT_CONSTEVAL_DEFAULT guard behavior is pinned.


223-223: ⚡ Quick win

suggestion: add constexpr-focused regression tests for __detectably_invalid in host/device/managed accessors to lock the new return false fallback and prevent behavior drift.

As per coding guidelines: "libcudacxx/**/*: Focus on correctness, ABI/API stability, standard-library conformance, host/device availability, NVRTC compatibility, supported C++ standards, tests, and portability across supported CUDA toolchains."

Also applies to: 358-358, 478-478


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 18e3baf1-78d9-4ab6-b47f-346ae08b05ce

📥 Commits

Reviewing files that changed from the base of the PR and between b832c9f and f8168ba.

📒 Files selected for processing (1)
  • libcudacxx/include/cuda/__mdspan/host_device_accessor.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

2 participants