Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't presume pointers are mutually exclusive for device/host. #6128

Open
wants to merge 2 commits into
base: branch-24.12
Choose a base branch
from

Conversation

robertmaynard
Copy link
Contributor

A pointer can be usable on the device and host at the same time. We can't invert is_dev_ptr() to check that something is a host pointer.

Here is the results of looking at the cudaPointerGetAttributes of different allocation types. As we can see things like cudaMallocManaged and cudaMallocHost allow the same pointer to be both host and device.

cudaPointerGetAttributes attributes malloc ptr
  is_dev_ptr  -> 0
  is_host_ptr -> 1
  memory loc  -> unregistered

cudaPointerGetAttributes attributes cudaMalloc ptr
  is_dev_ptr  -> 1
  is_host_ptr -> 0
  memory loc  -> device

cudaPointerGetAttributes attributes cudaMallocManaged cudaMemAttachGlobal ptr
  is_dev_ptr  -> 1
  is_host_ptr -> 1
  memory loc  -> managed

cudaPointerGetAttributes attributes cudaMallocManaged cudaMemAttachHost ptr
  is_dev_ptr  -> 1
  is_host_ptr -> 1
  memory loc  -> managed

cudaPointerGetAttributes attributes cudaMallocHost ptr
  is_dev_ptr  -> 1
  is_host_ptr -> 1
  memory loc  -> host

@robertmaynard robertmaynard added bug Something isn't working 3 - Ready for Review Ready for review by team non-breaking Non-breaking change labels Oct 30, 2024
@robertmaynard robertmaynard requested a review from a team as a code owner October 30, 2024 15:35
Here is the results of looking at the cudaPointerGetAttributes
of different allocation types. As we can see things like
cudaMallocManaged allow the same pointer to be both host and device.
```
cudaPointerGetAttributes attributes integer
  is_dev_ptr -> 0
  is_host_ptr -> 0

cudaPointerGetAttributes attributes std::vector<int> data
  is_dev_ptr -> 0
  is_host_ptr -> 0

cudaPointerGetAttributes attributes malloc ptr
  is_dev_ptr -> 0
  is_host_ptr -> 0

cudaPointerGetAttributes attributes cudaMalloc ptr
  is_dev_ptr -> 1
  is_host_ptr -> 0

cudaPointerGetAttributes attributes cudaMallocManaged cudaMemAttachGlobal ptr
  is_dev_ptr -> 1
  is_host_ptr -> 1

cudaPointerGetAttributes attributes cudaMallocManaged cudaMemAttachHost ptr
  is_dev_ptr -> 1
  is_host_ptr -> 1

cudaPointerGetAttributes attributes cudaMallocHost ptr
  is_dev_ptr -> 1
  is_host_ptr -> 1
```
@robertmaynard robertmaynard force-pushed the bug/device_ptrs_can_be_host_ptrs branch from 9d432af to 5bf3358 Compare October 31, 2024 18:30
@robertmaynard robertmaynard added the 5 - DO NOT MERGE Hold off on merging; see PR for details label Oct 31, 2024
@robertmaynard
Copy link
Contributor Author

Currently running some more manual validation checks

@robertmaynard robertmaynard removed the 5 - DO NOT MERGE Hold off on merging; see PR for details label Nov 1, 2024
Copy link
Member

@dantegd dantegd left a comment

Choose a reason for hiding this comment

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

lgtm

@robertmaynard
Copy link
Contributor Author

/merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team bug Something isn't working CUDA/C++ non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants