Skip to content

Commit

Permalink
Require resources to always provide at least one execution space prop…
Browse files Browse the repository at this point in the history
…erty (#2489)

Currently we implicitly assumed that any resource that had no execution space property was host accessible.

However, that is not a good design, as it provides a source of surprise and numerous challenges with proper type matching down the road.

So rather than implicitly assuming that something is host accessible, we require the user to always provide at least one execution space property.
  • Loading branch information
miscco authored Oct 2, 2024
1 parent 190099c commit 59ad103
Show file tree
Hide file tree
Showing 42 changed files with 807 additions and 876 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ template <class _Tp, class... _Properties>
class uninitialized_async_buffer
{
private:
static_assert(_CUDA_VMR::__contains_execution_space_property<_Properties...>,
"The properties of cuda::experimental::mr::uninitialized_async_buffer must contain at least one "
"execution space property!");

using __async_resource = ::cuda::experimental::mr::any_async_resource<_Properties...>;
__async_resource __mr_;
::cuda::stream_ref __stream_ = {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ template <class _Tp, class... _Properties>
class uninitialized_buffer
{
private:
static_assert(_CUDA_VMR::__contains_execution_space_property<_Properties...>,
"The properties of cuda::experimental::mr::uninitialized_buffer must contain at least one execution "
"space property!");

using __resource = ::cuda::experimental::mr::any_resource<_Properties...>;
__resource __mr_;
size_t __count_ = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#endif

#include <cuda/__memory_resource/get_property.h>
#include <cuda/__memory_resource/properties.h>
#include <cuda/__memory_resource/resource.h>
#include <cuda/__memory_resource/resource_ref.h>
#include <cuda/std/__concepts/__concept_macros.h>
Expand Down Expand Up @@ -73,6 +74,10 @@ class basic_any_resource
, private _CUDA_VMR::_Filtered_vtable<_Properties...>
{
private:
static_assert(_CUDA_VMR::__contains_execution_space_property<_Properties...>,
"The properties of cuda::experimental::mr::basic_any_resource must contain at least one execution "
"space property!");

template <_CUDA_VMR::_AllocType, class...>
friend class basic_any_resource;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,40 +265,83 @@ public:
_LIBCUDACXX_REQUIRES((_CUDA_VMR::__different_resource<async_memory_resource, _Resource>) )
_CCCL_NODISCARD bool operator==(_Resource const& __rhs) const noexcept
{
return _CUDA_VMR::resource_ref<>{const_cast<async_memory_resource*>(this)}
== _CUDA_VMR::resource_ref<>{const_cast<_Resource&>(__rhs)};
if constexpr (has_property<_Resource, _CUDA_VMR::device_accessible>)
{
return _CUDA_VMR::resource_ref<_CUDA_VMR::device_accessible>{const_cast<async_memory_resource*>(this)}
== _CUDA_VMR::resource_ref<_CUDA_VMR::device_accessible>{const_cast<_Resource&>(__rhs)};
}
else
{
return false;
}
}
# else // ^^^ C++20 ^^^ / vvv C++17
template <class _Resource>
_CCCL_NODISCARD_FRIEND auto operator==(async_memory_resource const& __lhs, _Resource const& __rhs) noexcept
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>)
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>&&
has_property<_Resource, _CUDA_VMR::device_accessible>)
{
return _CUDA_VMR::resource_ref<_CUDA_VMR::device_accessible>{const_cast<async_memory_resource&>(__lhs)}
== _CUDA_VMR::resource_ref<_CUDA_VMR::device_accessible>{const_cast<_Resource&>(__rhs)};
}

template <class _Resource>
_CCCL_NODISCARD_FRIEND auto operator==(async_memory_resource const&, _Resource const&) noexcept
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>
&& !has_property<_Resource, _CUDA_VMR::device_accessible>)
{
return _CUDA_VMR::resource_ref<>{const_cast<async_memory_resource&>(__lhs)}
== _CUDA_VMR::resource_ref<>{const_cast<_Resource&>(__rhs)};
return false;
}

template <class _Resource>
_CCCL_NODISCARD_FRIEND auto operator==(_Resource const& __rhs, async_memory_resource const& __lhs) noexcept
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>)
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>&&
has_property<_Resource, _CUDA_VMR::device_accessible>)
{
return _CUDA_VMR::resource_ref<_CUDA_VMR::device_accessible>{const_cast<async_memory_resource&>(__lhs)}
== _CUDA_VMR::resource_ref<_CUDA_VMR::device_accessible>{const_cast<_Resource&>(__rhs)};
}

template <class _Resource>
_CCCL_NODISCARD_FRIEND auto operator==(_Resource const&, async_memory_resource const&) noexcept
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>
&& !has_property<_Resource, _CUDA_VMR::device_accessible>)
{
return _CUDA_VMR::resource_ref<>{const_cast<async_memory_resource&>(__lhs)}
== _CUDA_VMR::resource_ref<>{const_cast<_Resource&>(__rhs)};
return false;
}

template <class _Resource>
_CCCL_NODISCARD_FRIEND auto operator!=(async_memory_resource const& __lhs, _Resource const& __rhs) noexcept
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>)
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>&&
has_property<_Resource, _CUDA_VMR::device_accessible>)
{
return _CUDA_VMR::resource_ref<_CUDA_VMR::device_accessible>{const_cast<async_memory_resource&>(__lhs)}
!= _CUDA_VMR::resource_ref<_CUDA_VMR::device_accessible>{const_cast<_Resource&>(__rhs)};
}

template <class _Resource>
_CCCL_NODISCARD_FRIEND auto operator!=(async_memory_resource const&, _Resource const&) noexcept
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>
&& !has_property<_Resource, _CUDA_VMR::device_accessible>)
{
return _CUDA_VMR::resource_ref<>{const_cast<async_memory_resource&>(__lhs)}
!= _CUDA_VMR::resource_ref<>{const_cast<_Resource&>(__rhs)};
return true;
}

template <class _Resource>
_CCCL_NODISCARD_FRIEND auto operator!=(_Resource const& __rhs, async_memory_resource const& __lhs) noexcept
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>)
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>&&
has_property<_Resource, _CUDA_VMR::device_accessible>)
{
return _CUDA_VMR::resource_ref<_CUDA_VMR::device_accessible>{const_cast<async_memory_resource&>(__lhs)}
!= _CUDA_VMR::resource_ref<_CUDA_VMR::device_accessible>{const_cast<_Resource&>(__rhs)};
}

template <class _Resource>
_CCCL_NODISCARD_FRIEND auto operator!=(_Resource const&, async_memory_resource const&) noexcept
_LIBCUDACXX_TRAILING_REQUIRES(bool)(_CUDA_VMR::__different_resource<async_memory_resource, _Resource>
&& !has_property<_Resource, _CUDA_VMR::device_accessible>)
{
return _CUDA_VMR::resource_ref<>{const_cast<async_memory_resource&>(__lhs)}
!= _CUDA_VMR::resource_ref<>{const_cast<_Resource&>(__rhs)};
return true;
}
# endif // _CCCL_STD_VER <= 2017

Expand Down
12 changes: 8 additions & 4 deletions cudax/test/containers/uninitialized_async_buffer.cu
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,17 @@ struct my_property
{
using value_type = int;
};
constexpr int get_property(const cuda::experimental::uninitialized_async_buffer<int, my_property>&, my_property)
constexpr int get_property(
const cuda::experimental::uninitialized_async_buffer<int, cuda::mr::device_accessible, my_property>&, my_property)
{
return 42;
}

TEMPLATE_TEST_CASE(
"uninitialized_async_buffer", "[container]", char, short, int, long, long long, float, double, do_not_construct)
{
using uninitialized_async_buffer = cuda::experimental::uninitialized_async_buffer<TestType>;
using uninitialized_async_buffer =
cuda::experimental::uninitialized_async_buffer<TestType, cuda::mr::device_accessible>;
static_assert(!cuda::std::is_default_constructible<uninitialized_async_buffer>::value, "");
static_assert(!cuda::std::is_copy_constructible<uninitialized_async_buffer>::value, "");
static_assert(!cuda::std::is_copy_assignable<uninitialized_async_buffer>::value, "");
Expand Down Expand Up @@ -132,8 +134,10 @@ TEMPLATE_TEST_CASE(
static_assert(cuda::has_property<cuda::experimental::uninitialized_async_buffer<int, cuda::mr::device_accessible>,
cuda::mr::device_accessible>,
"");
static_assert(cuda::has_property<cuda::experimental::uninitialized_async_buffer<int, my_property>, my_property>,
"");
static_assert(
cuda::has_property<cuda::experimental::uninitialized_async_buffer<int, cuda::mr::device_accessible, my_property>,
my_property>,
"");
}

SECTION("conversion to span")
Expand Down
8 changes: 6 additions & 2 deletions cudax/test/containers/uninitialized_buffer.cu
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ struct my_property
{
using value_type = int;
};
constexpr int get_property(const cuda::experimental::uninitialized_buffer<int, my_property>&, my_property)
constexpr int get_property(
const cuda::experimental::uninitialized_buffer<int, cuda::mr::device_accessible, my_property>&, my_property)
{
return 42;
}
Expand Down Expand Up @@ -139,7 +140,10 @@ TEMPLATE_TEST_CASE(
static_assert(cuda::has_property<cuda::experimental::uninitialized_buffer<int, cuda::mr::device_accessible>,
cuda::mr::device_accessible>,
"");
static_assert(cuda::has_property<cuda::experimental::uninitialized_buffer<int, my_property>, my_property>, "");
static_assert(
cuda::has_property<cuda::experimental::uninitialized_buffer<int, cuda::mr::device_accessible, my_property>,
my_property>,
"");
}

SECTION("convertion to span")
Expand Down
18 changes: 10 additions & 8 deletions cudax/test/memory_resource/any_async_resource.cu
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@

TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_async_resource", "[container][resource]", big_resource, small_resource)
{
using TestResource = TestType;
using TestResource = TestType;
static_assert(cuda::mr::resource_with<TestResource, cuda::mr::host_accessible>);
constexpr bool is_big = sizeof(TestResource) > sizeof(cuda::mr::_AnyResourceStorage);

SECTION("construct and destruct")
{
Counts expected{};
CHECK(this->counts == expected);
{
cudax::mr::any_async_resource<> mr{TestResource{42, this}};
cudax::mr::any_async_resource<cuda::mr::host_accessible> mr{TestResource{42, this}};
expected.new_count += is_big;
++expected.object_count;
++expected.move_count;
Expand All @@ -43,7 +44,7 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_async_resource", "[container][resou
Counts expected{};
CHECK(this->counts == expected);
{
cudax::mr::any_async_resource<> mr{TestResource{42, this}};
cudax::mr::any_async_resource<cuda::mr::host_accessible> mr{TestResource{42, this}};
expected.new_count += is_big;
++expected.object_count;
++expected.move_count;
Expand Down Expand Up @@ -78,7 +79,7 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_async_resource", "[container][resou
Counts expected{};
CHECK(this->counts == expected);
{
cudax::mr::any_async_resource<> mr{TestResource{42, this}};
cudax::mr::any_async_resource<cuda::mr::host_accessible> mr{TestResource{42, this}};
expected.new_count += is_big;
++expected.object_count;
++expected.move_count;
Expand Down Expand Up @@ -107,7 +108,7 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_async_resource", "[container][resou
CHECK(this->counts == expected);
{
cudax::stream stream{};
cudax::mr::any_async_resource<> mr{TestResource{42, this}};
cudax::mr::any_async_resource<cuda::mr::host_accessible> mr{TestResource{42, this}};
expected.new_count += is_big;
++expected.object_count;
++expected.move_count;
Expand All @@ -134,13 +135,13 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_async_resource", "[container][resou
{
Counts expected{};
{
cudax::mr::any_async_resource<> mr{TestResource{42, this}};
cudax::mr::any_async_resource<cuda::mr::host_accessible> mr{TestResource{42, this}};
expected.new_count += is_big;
++expected.object_count;
++expected.move_count;
CHECK(this->counts == expected);

cuda::mr::resource_ref<> ref = mr;
cuda::mr::resource_ref<cuda::mr::host_accessible> ref = mr;

CHECK(this->counts == expected);
auto* ptr = ref.allocate(bytes(100), align(8));
Expand All @@ -164,7 +165,8 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_async_resource", "[container][resou
Counts expected{};
CHECK(this->counts == expected);
{
cudax::mr::any_async_resource<> mr = cudax::mr::make_any_async_resource<TestResource>(42, this);
cudax::mr::any_async_resource<cuda::mr::host_accessible> mr =
cudax::mr::make_any_async_resource<TestResource, cuda::mr::host_accessible>(42, this);
expected.new_count += is_big;
++expected.object_count;
CHECK(this->counts == expected);
Expand Down
13 changes: 7 additions & 6 deletions cudax/test/memory_resource/any_resource.cu
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_resource", "[container][resource]",
Counts expected{};
CHECK(this->counts == expected);
{
cudax::mr::any_resource<> mr{TestResource{42, this}};
cudax::mr::any_resource<cuda::mr::host_accessible> mr{TestResource{42, this}};
expected.new_count += is_big;
++expected.object_count;
++expected.move_count;
Expand All @@ -43,7 +43,7 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_resource", "[container][resource]",
Counts expected{};
CHECK(this->counts == expected);
{
cudax::mr::any_resource<> mr{TestResource{42, this}};
cudax::mr::any_resource<cuda::mr::host_accessible> mr{TestResource{42, this}};
expected.new_count += is_big;
++expected.object_count;
++expected.move_count;
Expand Down Expand Up @@ -78,7 +78,7 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_resource", "[container][resource]",
Counts expected{};
CHECK(this->counts == expected);
{
cudax::mr::any_resource<> mr{TestResource{42, this}};
cudax::mr::any_resource<cuda::mr::host_accessible> mr{TestResource{42, this}};
expected.new_count += is_big;
++expected.object_count;
++expected.move_count;
Expand All @@ -105,13 +105,13 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_resource", "[container][resource]",
{
Counts expected{};
{
cudax::mr::any_resource<> mr{TestResource{42, this}};
cudax::mr::any_resource<cuda::mr::host_accessible> mr{TestResource{42, this}};
expected.new_count += is_big;
++expected.object_count;
++expected.move_count;
CHECK(this->counts == expected);

cuda::mr::resource_ref<> ref = mr;
cuda::mr::resource_ref<cuda::mr::host_accessible> ref = mr;

CHECK(this->counts == expected);
auto* ptr = ref.allocate(bytes(100), align(8));
Expand All @@ -135,7 +135,8 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "any_resource", "[container][resource]",
Counts expected{};
CHECK(this->counts == expected);
{
cudax::mr::any_resource<> mr = cudax::mr::make_any_resource<TestResource>(42, this);
cudax::mr::any_resource<cuda::mr::host_accessible> mr =
cudax::mr::make_any_resource<TestResource, cuda::mr::host_accessible>(42, this);
expected.new_count += is_big;
++expected.object_count;
CHECK(this->counts == expected);
Expand Down
16 changes: 0 additions & 16 deletions cudax/test/memory_resource/async_memory_resource.cu
Original file line number Diff line number Diff line change
Expand Up @@ -434,14 +434,6 @@ TEST_CASE("async_memory_resource comparison", "[memory_resource]")
CHECK(!(second_ref != first));
}

{ // comparison against a async_memory_resource wrapped inside a resource_ref<>
cuda::experimental::mr::async_memory_resource second{};
CHECK(first == cuda::mr::resource_ref<>{second});
CHECK(!(first != cuda::mr::resource_ref<>{second}));
CHECK(cuda::mr::resource_ref<>{second} == first);
CHECK(!(cuda::mr::resource_ref<>{second} != first));
}

{ // comparison against a async_memory_resource wrapped inside a async_resource_ref
cuda::experimental::mr::async_memory_resource second{};
cuda::mr::async_resource_ref<cuda::mr::device_accessible> second_ref{second};
Expand All @@ -452,14 +444,6 @@ TEST_CASE("async_memory_resource comparison", "[memory_resource]")
CHECK(!(second_ref != first));
}

{ // comparison against a async_memory_resource wrapped inside a async_resource_ref<>
cuda::experimental::mr::async_memory_resource second{};
CHECK(first == cuda::mr::async_resource_ref<>{second});
CHECK(!(first != cuda::mr::async_resource_ref<>{second}));
CHECK(cuda::mr::async_resource_ref<>{second} == first);
CHECK(!(cuda::mr::async_resource_ref<>{second} != first));
}

{ // comparison against a different resource through resource_ref
resource<AccessibilityType::Host> host_resource{};
resource<AccessibilityType::Device> device_resource{};
Expand Down
9 changes: 5 additions & 4 deletions cudax/test/memory_resource/shared_resource.cu
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "shared_resource", "[container][resource
++expected.object_count;
CHECK(this->counts == expected);

cuda::mr::resource_ref<> ref = mr;
cuda::mr::resource_ref<cuda::mr::host_accessible> ref = mr;

CHECK(this->counts == expected);
auto* ptr = ref.allocate(bytes(100), align(8));
Expand All @@ -128,7 +128,8 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "shared_resource", "[container][resource
align(alignof(int) * 4);
{
bytes(42 * sizeof(int));
cudax::uninitialized_buffer<int> buffer{cudax::mr::shared_resource<TestResource>(42, this), 42};
cudax::uninitialized_buffer<int, cuda::mr::host_accessible> buffer{
cudax::mr::shared_resource<TestResource>(42, this), 42};
++expected.object_count;
++expected.allocate_count;
CHECK(this->counts == expected);
Expand All @@ -137,7 +138,7 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "shared_resource", "[container][resource
{
// accounting for new storage
bytes(1337 * sizeof(int));
cudax::uninitialized_buffer<int> other_buffer{buffer.get_resource(), 1337};
cudax::uninitialized_buffer<int, cuda::mr::host_accessible> other_buffer{buffer.get_resource(), 1337};
++expected.allocate_count;
CHECK(this->counts == expected);
}
Expand All @@ -149,7 +150,7 @@ TEMPLATE_TEST_CASE_METHOD(test_fixture, "shared_resource", "[container][resource

{
// Moving the resource should not do anything
cudax::uninitialized_buffer<int> third_buffer = ::cuda::std::move(buffer);
cudax::uninitialized_buffer<int, cuda::mr::host_accessible> third_buffer = ::cuda::std::move(buffer);
CHECK(this->counts == expected);
}

Expand Down
2 changes: 2 additions & 0 deletions cudax/test/memory_resource/test_resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ struct test_resource
++test_fixture_::counts_->delete_count;
return ::operator delete(pv);
}

friend constexpr void get_property(const test_resource&, cuda::mr::host_accessible) noexcept {}
};

using big_resource = test_resource<uintptr_t>;
Expand Down
Loading

0 comments on commit 59ad103

Please sign in to comment.