Skip to content

Commit ad99c11

Browse files
authored
Use forward_property for property forwarding in resource_ref wrappers (#2328)
Replace the hand-rolled friend `get_property` templates in `cccl_resource_ref` and `cccl_async_resource_ref` with inheritance from `cuda::forward_property`. This delegates property forwarding to CCCL's own machinery, which correctly handles `dynamic_accessibility_property` ([NVIDIA/cccl#7727](NVIDIA/cccl#7727)) and any future properties without ambiguity. Each wrapper now exposes `upstream_resource()` returning the inner `ResourceType`, as required by `forward_property` for stateful properties. Tests add minimal `forward_property` adaptors using RMM resource refs as upstream, exercising the exact scenario that causes the ambiguity. Note: this is a temporary solution for the `main` branch -- resolving #2323 / #2325 will remove this code on the `staging` branch while I continue working on CCCL MR migrations. Closes #2322. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Rong Ou (https://github.com/rongou) URL: #2328
1 parent 1bef459 commit ad99c11

File tree

2 files changed

+145
-16
lines changed

2 files changed

+145
-16
lines changed

cpp/include/rmm/detail/cccl_adaptors.hpp

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,15 @@ class cccl_async_resource_ref;
4747
* interface of the underlying type. It enables constructing resource refs from
4848
* device_memory_resource pointers by wrapping them in a device_memory_resource_view.
4949
*
50+
* Inherits from cuda::forward_property to delegate property queries to the wrapped
51+
* resource_ref, avoiding ambiguity with CCCL's default get_property overloads
52+
* (e.g. for dynamic_accessibility_property, NVIDIA/cccl#7727).
53+
*
5054
* @tparam ResourceType The underlying CCCL synchronous_resource_ref type
5155
*/
5256
template <typename ResourceType>
53-
class cccl_resource_ref {
57+
class cccl_resource_ref
58+
: public cuda::forward_property<cccl_resource_ref<ResourceType>, ResourceType> {
5459
public:
5560
using wrapped_type = ResourceType;
5661

@@ -244,14 +249,11 @@ class cccl_resource_ref {
244249
}
245250

246251
/**
247-
* @brief Forwards a property query to the wrapped resource_ref.
252+
* @brief Returns a const reference to the wrapped resource_ref.
253+
*
254+
* Required by cuda::forward_property to forward stateful property queries.
248255
*/
249-
template <typename Property>
250-
friend auto constexpr get_property(cccl_resource_ref const& ref, Property prop) noexcept
251-
-> decltype(get_property(std::declval<ResourceType const&>(), prop))
252-
{
253-
return get_property(ref.ref_, prop);
254-
}
256+
[[nodiscard]] ResourceType const& upstream_resource() const noexcept { return ref_; }
255257

256258
/**
257259
* @brief Attempts to get a property from the wrapped resource_ref.
@@ -276,6 +278,10 @@ class cccl_resource_ref {
276278
* to avoid recursive constraint satisfaction issues with CCCL 3.2's basic_any-based
277279
* resource_ref types. It provides both synchronous and asynchronous allocation methods.
278280
*
281+
* Inherits from cuda::forward_property to delegate property queries to the wrapped
282+
* resource_ref, avoiding ambiguity with CCCL's default get_property overloads
283+
* (e.g. for dynamic_accessibility_property, NVIDIA/cccl#7727).
284+
*
279285
* @tparam ResourceType The underlying CCCL resource_ref type (async)
280286
*/
281287
// Suppress spurious warning about calling a __host__ function from __host__ __device__ context
@@ -286,7 +292,8 @@ class cccl_resource_ref {
286292
#pragma nv_diag_suppress 20011
287293
#endif
288294
template <typename ResourceType>
289-
class cccl_async_resource_ref {
295+
class cccl_async_resource_ref
296+
: public cuda::forward_property<cccl_async_resource_ref<ResourceType>, ResourceType> {
290297
public:
291298
using wrapped_type = ResourceType;
292299

@@ -528,14 +535,11 @@ class cccl_async_resource_ref {
528535
}
529536

530537
/**
531-
* @brief Forwards a property query to the wrapped resource_ref.
538+
* @brief Returns a const reference to the wrapped resource_ref.
539+
*
540+
* Required by cuda::forward_property to forward stateful property queries.
532541
*/
533-
template <typename Property>
534-
friend auto constexpr get_property(cccl_async_resource_ref const& ref, Property prop) noexcept
535-
-> decltype(get_property(std::declval<ResourceType const&>(), prop))
536-
{
537-
return get_property(ref.ref_, prop);
538-
}
542+
[[nodiscard]] ResourceType const& upstream_resource() const noexcept { return ref_; }
539543

540544
/**
541545
* @brief Attempts to get a property from the wrapped resource_ref.

cpp/tests/mr/resource_ref_conversion_tests.cpp

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77

88
#include <rmm/cuda_stream.hpp>
99
#include <rmm/detail/error.hpp>
10+
#include <rmm/mr/cuda_memory_resource.hpp>
1011
#include <rmm/mr/pinned_host_memory_resource.hpp>
1112
#include <rmm/resource_ref.hpp>
1213

14+
#include <cuda/memory_resource>
1315
#include <thrust/host_vector.h>
1416

1517
#include <gtest/gtest.h>
@@ -218,3 +220,126 @@ TEST(ResourceRefConversionAllocator, VectorMove)
218220
auto vec2 = std::move(vec1);
219221
ASSERT_EQ(vec2[0], 42);
220222
}
223+
224+
// --------------------------------------------------------------------------
225+
// Tests for forward_property adaptor with RMM resource_ref as upstream.
226+
//
227+
// This exercises the fix for https://github.com/rapidsai/rmm/issues/2322:
228+
// an unconstrained friend get_property in cccl_async_resource_ref caused
229+
// ambiguity with CCCL's default get_property for dynamic_accessibility_property.
230+
// --------------------------------------------------------------------------
231+
232+
// A minimal adaptor that uses cuda::forward_property with an RMM async resource ref as upstream.
233+
struct forwarding_adaptor
234+
: cuda::forward_property<forwarding_adaptor, rmm::device_async_resource_ref> {
235+
explicit forwarding_adaptor(rmm::device_async_resource_ref upstream) : upstream_{upstream} {}
236+
237+
rmm::device_async_resource_ref upstream_resource() const { return upstream_; }
238+
239+
void* allocate(cuda::stream_ref stream, std::size_t bytes, std::size_t alignment)
240+
{
241+
return upstream_.allocate(stream, bytes, alignment);
242+
}
243+
void deallocate(cuda::stream_ref stream,
244+
void* ptr,
245+
std::size_t bytes,
246+
std::size_t alignment) noexcept
247+
{
248+
upstream_.deallocate(stream, ptr, bytes, alignment);
249+
}
250+
void* allocate_sync(std::size_t bytes, std::size_t alignment)
251+
{
252+
return upstream_.allocate_sync(bytes, alignment);
253+
}
254+
void deallocate_sync(void* ptr, std::size_t bytes, std::size_t alignment) noexcept
255+
{
256+
upstream_.deallocate_sync(ptr, bytes, alignment);
257+
}
258+
259+
friend bool operator==(forwarding_adaptor const& lhs, forwarding_adaptor const& rhs)
260+
{
261+
return lhs.upstream_ == rhs.upstream_;
262+
}
263+
friend bool operator!=(forwarding_adaptor const& lhs, forwarding_adaptor const& rhs)
264+
{
265+
return !(lhs == rhs);
266+
}
267+
268+
private:
269+
rmm::device_async_resource_ref upstream_;
270+
};
271+
272+
// A minimal adaptor using forward_property with an RMM sync resource ref as upstream.
273+
struct forwarding_sync_adaptor
274+
: cuda::forward_property<forwarding_sync_adaptor, rmm::device_resource_ref> {
275+
explicit forwarding_sync_adaptor(rmm::device_resource_ref upstream) : upstream_{upstream} {}
276+
277+
rmm::device_resource_ref upstream_resource() const { return upstream_; }
278+
279+
void* allocate_sync(std::size_t bytes, std::size_t alignment)
280+
{
281+
return upstream_.allocate_sync(bytes, alignment);
282+
}
283+
void deallocate_sync(void* ptr, std::size_t bytes, std::size_t alignment) noexcept
284+
{
285+
upstream_.deallocate_sync(ptr, bytes, alignment);
286+
}
287+
288+
friend bool operator==(forwarding_sync_adaptor const& lhs, forwarding_sync_adaptor const& rhs)
289+
{
290+
return lhs.upstream_ == rhs.upstream_;
291+
}
292+
friend bool operator!=(forwarding_sync_adaptor const& lhs, forwarding_sync_adaptor const& rhs)
293+
{
294+
return !(lhs == rhs);
295+
}
296+
297+
private:
298+
rmm::device_resource_ref upstream_;
299+
};
300+
301+
// Compile-time checks: verify that the forwarding adaptor satisfies resource concepts.
302+
static_assert(cuda::has_property<forwarding_adaptor, cuda::mr::device_accessible>,
303+
"forwarding_adaptor must have device_accessible property via forward_property");
304+
305+
static_assert(cuda::has_property<forwarding_sync_adaptor, cuda::mr::device_accessible>,
306+
"forwarding_sync_adaptor must have device_accessible property via forward_property");
307+
308+
// Type-erasing a forward_property adaptor into resource_ref triggers the ambiguity
309+
// from issue #2322. If the constraint on cccl_async_resource_ref::get_property is missing,
310+
// this will fail to compile when CCCL has dynamic_accessibility_property.
311+
TEST(ForwardPropertyAdaptor, TypeEraseAsyncAdaptor)
312+
{
313+
rmm::mr::cuda_memory_resource mr{};
314+
rmm::device_async_resource_ref upstream{mr};
315+
forwarding_adaptor adaptor{upstream};
316+
cuda::mr::resource_ref<cuda::mr::device_accessible> erased{adaptor};
317+
318+
rmm::cuda_stream stream{};
319+
void* ptr = erased.allocate(stream, 1024, 256);
320+
ASSERT_NE(ptr, nullptr);
321+
erased.deallocate(stream, ptr, 1024, 256);
322+
}
323+
324+
TEST(ForwardPropertyAdaptor, TypeEraseSyncAdaptor)
325+
{
326+
rmm::mr::cuda_memory_resource mr{};
327+
rmm::device_resource_ref upstream{mr};
328+
forwarding_sync_adaptor adaptor{upstream};
329+
cuda::mr::synchronous_resource_ref<cuda::mr::device_accessible> erased{adaptor};
330+
331+
void* ptr = erased.allocate_sync(1024);
332+
ASSERT_NE(ptr, nullptr);
333+
erased.deallocate_sync(ptr, 1024);
334+
}
335+
336+
// Verify that get_property still works correctly through the forwarding adaptor.
337+
TEST(ForwardPropertyAdaptor, GetPropertyDeviceAccessible)
338+
{
339+
rmm::mr::cuda_memory_resource mr{};
340+
rmm::device_async_resource_ref upstream{mr};
341+
forwarding_adaptor adaptor{upstream};
342+
343+
// Should compile and not throw - device_accessible is a stateless property
344+
get_property(adaptor, cuda::mr::device_accessible{});
345+
}

0 commit comments

Comments
 (0)