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

[BUG]: thrust::pair is not trivially copyable in CCCL branch/2.3.x #1246

Closed
1 task done
bdice opened this issue Jan 3, 2024 · 0 comments · Fixed by #1249
Closed
1 task done

[BUG]: thrust::pair is not trivially copyable in CCCL branch/2.3.x #1246

bdice opened this issue Jan 3, 2024 · 0 comments · Fixed by #1249
Labels
bug Something isn't working right.

Comments

@bdice
Copy link
Contributor

bdice commented Jan 3, 2024

Is this a duplicate?

Type of Bug

Compile-time Error

Component

Thrust

Describe the bug

The thrust::pair class was trivially copyable in CCCL 2.2.0 but is not trivially copyable in branch/2.3.x. I suspect #262 is the PR that introduced this change (bug?).

This leads to compilation failures in RAPIDS cuDF because RMM requires rmm::device_uvector to use trivially copyable types. For example: https://github.com/rapidsai/cudf/actions/runs/7403019153/job/20142041800?pr=14704#step:8:1714

/home/coder/.conda/envs/rapids/include/rmm/device_uvector.hpp(78): error: static assertion failed with "device_uvector only supports types that are trivially copyable."
          detected during instantiation of class "rmm::device_uvector<T> [with T=cuda::std::__4::pair<std::size_t, std::size_t>]" 
/home/coder/cudf/cpp/src/copying/contiguous_split.cu(1412): here

How to Reproduce

Switch between CCCL 2.2.0 and trunk. 2.2.0 passes, but trunk fails.
https://godbolt.org/z/oeEKa5577

Also see rapidsai/cudf#14704

Expected behavior

thrust::pair behavior should not change (it should be trivially copyable in CCCL trunk).

Reproduction link

https://godbolt.org/z/oeEKa5577

Operating System

No response

nvidia-smi output

No response

NVCC version

No response

@bdice bdice added the bug Something isn't working right. label Jan 3, 2024
miscco added a commit to miscco/cccl that referenced this issue Jan 5, 2024
trivially copyable is a requirement for memcpy. We want to ensure that our pair implementation satisfies that whenever possible.

This is especially important for thrust::pair as that is used in rmm extensively.

Fixes NVIDIA#1246
miscco added a commit to miscco/cccl that referenced this issue Jan 5, 2024
trivially copyable is a requirement for memcpy. We want to ensure that our pair implementation satisfies that whenever possible.

This is especially important for thrust::pair as that is used in rmm extensively.

Fixes NVIDIA#1246
miscco added a commit to miscco/cccl that referenced this issue Jan 5, 2024
trivially copyable is a requirement for memcpy. We want to ensure that our pair implementation satisfies that whenever possible.

This is especially important for thrust::pair as that is used in rmm extensively.

Fixes NVIDIA#1246
miscco added a commit to miscco/cccl that referenced this issue Jan 5, 2024
trivially copyable is a requirement for memcpy. We want to ensure that our pair implementation satisfies that whenever possible.

This is especially important for thrust::pair as that is used in rmm extensively.

Fixes NVIDIA#1246
miscco added a commit that referenced this issue Jan 18, 2024
trivially copyable is a requirement for memcpy. We want to ensure that our pair implementation satisfies that whenever possible.

This is especially important for thrust::pair as that is used in rmm extensively.

Fixes #1246

Co-authored-by: Georgy Evtushenko <evtushenko.georgy@gmail.com>
miscco added a commit to miscco/cccl that referenced this issue Jan 18, 2024
…IA#1249)

trivially copyable is a requirement for memcpy. We want to ensure that our pair implementation satisfies that whenever possible.

This is especially important for thrust::pair as that is used in rmm extensively.

Fixes NVIDIA#1246

Co-authored-by: Georgy Evtushenko <evtushenko.georgy@gmail.com>
miscco added a commit to miscco/cccl that referenced this issue Jan 18, 2024
…IA#1249)

trivially copyable is a requirement for memcpy. We want to ensure that our pair implementation satisfies that whenever possible.

This is especially important for thrust::pair as that is used in rmm extensively.

Fixes NVIDIA#1246

Co-authored-by: Georgy Evtushenko <evtushenko.georgy@gmail.com>
jrhemstad pushed a commit that referenced this issue Jan 18, 2024
* Ensure that `cuda::std::pair` is potentially trivially copyable (#1249)

trivially copyable is a requirement for memcpy. We want to ensure that our pair implementation satisfies that whenever possible.

This is especially important for thrust::pair as that is used in rmm extensively.

Fixes #1246

Co-authored-by: Georgy Evtushenko <evtushenko.georgy@gmail.com>

* Mark test as passing

---------

Co-authored-by: Georgy Evtushenko <evtushenko.georgy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working right.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant