Skip to content

[libcu++] Fix default make_shared_resource construction #9044

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.

Sign up for GitHub

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

Merged
bdice merged 4 commits into from
May 17, 2026
Merged

[libcu++] Fix default make_shared_resource construction #9044

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fc1cb12
[libcu++] Test default make_shared_resource construction
bdice May 16, 2026
2981aac
[libcu++] Fix default make_shared_resource construction
bdice May 16, 2026
3b1aa56
[libcu++] Fix shared_block_ptr in_place construction ambiguity
bdice May 16, 2026
4949d8e
Fix clang-format
bdice May 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion libcudacxx/include/cuda/__memory_resource/shared_block_ptr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <cuda/std/__type_traits/remove_cvref.h>
#include <cuda/std/__utility/exchange.h>
#include <cuda/std/__utility/forward.h>
#include <cuda/std/__utility/in_place.h>
#include <cuda/std/__utility/move.h>
#include <cuda/std/__utility/swap.h>
#include <cuda/std/atomic>
Expand Down Expand Up @@ -67,9 +68,17 @@ class __shared_block_ptr
//! @brief Constructs a null ``__shared_block_ptr`` with no control block.
_CCCL_HIDE_FROM_ABI __shared_block_ptr() = default;

//! @brief Constructs a new control block, forwarding arguments to the payload.
template <class... _Args>
_CCCL_HOST_API explicit __shared_block_ptr(::cuda::std::in_place_type_t<_Payload>, _Args&&... __args)
: __block_(new __block_t(::cuda::std::forward<_Args>(__args)...))
{}

//! @brief Constructs a new control block, forwarding arguments to the payload.
_CCCL_TEMPLATE(class _Arg, class... _Rest)
_CCCL_REQUIRES((!::cuda::std::is_same_v<::cuda::std::remove_cvref_t<_Arg>, __shared_block_ptr>) )
_CCCL_REQUIRES(
(!::cuda::std::is_same_v<::cuda::std::remove_cvref_t<_Arg>, __shared_block_ptr>)
&& (!::cuda::std::is_same_v<::cuda::std::remove_cvref_t<_Arg>, ::cuda::std::in_place_type_t<_Payload>>) )
_CCCL_HOST_API explicit __shared_block_ptr(_Arg&& __arg, _Rest&&... __rest)
: __block_(new __block_t(::cuda::std::forward<_Arg>(__arg), ::cuda::std::forward<_Rest>(__rest)...))
{}
Comment on lines +71 to 84
Copy link
Copy Markdown
Contributor

@miscco miscco May 18, 2026

Choose a reason for hiding this comment

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

I believe the additional constraint should not be necessary. We should probably just require everybody to use in_place

Copy link
Copy Markdown
Contributor Author

@bdice bdice May 18, 2026

Choose a reason for hiding this comment

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

Happy to do this in a follow-up but we need to land the backport first to get CI unblocked for RAPIDS.

GCC 7 jobs failed without the additional constraint (ambiguous with two forwarding constructors).

Expand Down
2 changes: 1 addition & 1 deletion libcudacxx/include/cuda/__memory_resource/shared_resource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ struct shared_resource
//! @param __args The arguments to be passed to the \c _Resource constructor.
template <class... _Args>
explicit shared_resource(::cuda::std::in_place_type_t<_Resource>, _Args&&... __args)
: __block_(::cuda::std::forward<_Args>(__args)...)
: __block_(::cuda::std::in_place_type<_Resource>, ::cuda::std::forward<_Args>(__args)...)
{}

//! @brief Copy-constructs a \c shared_resource object resulting in an copy that shares
Expand Down
35 changes: 35 additions & 0 deletions libcudacxx/test/libcudacxx/cuda/memory_resource/shared_resource.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,41 @@

#include "test_resource.cuh"

struct default_constructible_resource
{
default_constructible_resource()
{
++constructed;
}

void* allocate_sync(size_t, size_t)
{
return this;
}

void deallocate_sync(void*, size_t, size_t) noexcept {}

friend bool operator==(default_constructible_resource const&, default_constructible_resource const&) noexcept
{
return true;
}

friend bool operator!=(default_constructible_resource const&, default_constructible_resource const&) noexcept
{
return false;
}

static inline int constructed = 0;
};

C2H_CCCLRT_TEST("make_shared_resource default constructs resource", "[container][resource]")
{
default_constructible_resource::constructed = 0;
auto resource = cuda::mr::make_shared_resource<default_constructible_resource>();
CHECK(default_constructible_resource::constructed == 1);
CHECK(resource.allocate_sync(1, 1) != nullptr);
}

TEMPLATE_TEST_CASE_METHOD(test_fixture, "shared_resource", "[container][resource]", big_resource, small_resource)
{
using TestResource = TestType;
Expand Down
Loading