Skip to content

[libcu++] Fix peer access case in is_pointer_accessible#9478

Open
pciolkosz wants to merge 3 commits into
NVIDIA:mainfrom
pciolkosz:fix_is_pointer_for_peers
Open

[libcu++] Fix peer access case in is_pointer_accessible#9478
pciolkosz wants to merge 3 commits into
NVIDIA:mainfrom
pciolkosz:fix_is_pointer_for_peers

Conversation

@pciolkosz

Copy link
Copy Markdown
Contributor

is_pointer_accessible is currently wrong in peer-access cases. It uses cudaCeviceCanAccessPeer, which checks for possibility of peer access, not if it's enabled or not. This PR changes that to use pointer attributes to query the device pointer for a specific device and instead return false if that query fails.

This PR also fixes usage of cudaDeviceEnablePeerAccess in the tests for is_pointer_accessible to correctly juggle the explicit argument and implicit current context argument and to disable peer access after the test cases

@pciolkosz pciolkosz requested a review from a team as a code owner June 16, 2026 02:23
@pciolkosz pciolkosz requested a review from wmaxey June 16, 2026 02:23
@github-project-automation github-project-automation Bot moved this to Todo in CCCL Jun 16, 2026
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Jun 16, 2026
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 564b58fb-db42-4493-8266-4628b7284dbd

📥 Commits

Reviewing files that changed from the base of the PR and between 39b132e and 526d24c.

📒 Files selected for processing (1)
  • libcudacxx/include/cuda/__memory/is_pointer_accessible.h

Note: CodeRabbit is enabled on this repository as a convenience for maintainers
and contributors. Use your best judgment when considering its review comments and
suggestions — a suggested change may be inadequate, unnecessary, or safe to ignore.
Contributors are not expected to address every comment. Human reviews are what
ultimately matter for merging.

Summary

This PR fixes is_pointer_accessible in libcu++ so it determines actual pointer accessibility from a specific device, instead of incorrectly relying on cudaDeviceCanAccessPeer (which only indicates whether peer access could be possible, not whether it’s enabled).

Changes

Implementation (is_pointer_accessible.h)

  • Added cuda/__runtime/ensure_current_context.h and ensured the correct CUDA context is active for the target device_ref before performing the accessibility query.
  • Reworked __is_device_accessible to:
    • Wrap pointer-accessibility logic in _CCCL_TRY/_CCCL_CATCH_ALL so the nothrow path returns false on failures.
    • Query pointer attributes using CU_POINTER_ATTRIBUTE_DEVICE_POINTER rather than comparing device ordinals.
    • Determine managed-pointer eligibility based on whether the retrieved device-specific pointer is non-null after prior validation checks (e.g., unregistered vs. managed/device-memory classification and any optional mempool read access requirements).
    • Remove the prior peer-access-ordinal comparison logic.

Tests (is_pointer_accessible.pass.cpp)

  • Refactored memory-pool setup by introducing a create_memory_pool(...) helper and simplifying pool allocations via an allocate_memory_from_pool(...) helper.
  • Updated multi-device coverage to align with the corrected accessibility semantics:
    • Adjusted peer-access logic direction and the corresponding enable/disable calls.
    • Corrected assertions to match the expected accessibility results per device.
  • Fixed test handling involving cudaDeviceEnablePeerAccess so the explicit device argument is treated correctly alongside any implicit current-context behavior, and ensured peer access is properly disabled after the test completes.
  • For the multi-device “from pool” scenario, switched to validating accessibility via explicit cudaMemPoolSetAccess permissions rather than relying on peer-enabled access.

Impact

The updated implementation more reliably answers “is this pointer accessible from this device right now?”, improving correctness for cross-device pointer validation, and the tests now better exercise and verify that behavior under peer-access and memory-pool permission configurations.

Walkthrough

__is_device_accessible in is_pointer_accessible.h is extended to call __ensure_current_context and query CU_POINTER_ATTRIBUTE_DEVICE_POINTER after peer-access succeeds, returning whether the resulting device pointer is non-null. Tests split pool creation from allocation, reverse the cudaDeviceCanAccessPeer query direction, and replace peer-access enablement with cudaMemPoolSetAccess in the pool-based multi-device path.

Changes

Device pointer accessibility logic and test coverage

Layer / File(s) Summary
Expanded __is_device_accessible implementation
libcudacxx/include/cuda/__memory/is_pointer_accessible.h
Includes ensure_current_context.h; replaces the peer-access-only return with a two-stage flow: peer gate for non-managed pointers, then __ensure_current_context + CU_POINTER_ATTRIBUTE_DEVICE_POINTER query + null check; errors handled with _CCCL_TRY/_CCCL_CATCH_ALL returning false or rethrowing.
Pool helper refactor and multi-device test fixes
libcudacxx/test/libcudacxx/cuda/memory/is_pointer_accessible.pass.cpp
Introduces create_memory_pool (constructs pool, calls cudaMemPoolSetAccess, returns handle) and simplifies allocate_memory_from_pool to take only cudaMemPool_t; rewires test_memory_pool_impl; fixes cudaDeviceCanAccessPeer argument order; changes peer enablement from dev1 to dev0; replaces cudaDeviceEnablePeerAccess with cudaMemPoolSetAccess in test_multiple_devices_from_pool.

Suggested reviewers

  • Jacobfaib
  • davebayer

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

@github-actions

This comment has been minimized.


void* __device_ptr = nullptr;
const auto __ptr_status =
::cuda::__driver::__pointerGetAttributeNoThrow<::CU_POINTER_ATTRIBUTE_DEVICE_POINTER>(__device_ptr, __p);

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.

would be possible to move this query to the initial __pointerGetAttributesNoThrow call?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ahh, yes! We can remove the CU_POINTER_ATTRIBUTE_DEVICE_ORDINAL and query CU_POINTER_ATTRIBUTE_DEVICE_POINTER instead. Good idea

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 5h 06m: Pass: 100%/503 | Total: 4d 22h | Max: 1h 04m | Hits: 90%/738186

See results here.

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

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

2 participants