Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
miscco committed Nov 2, 2023
1 parent 2cf03a7 commit 76d55fa
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 30 deletions.
18 changes: 0 additions & 18 deletions include/rmm/mr/device/device_memory_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,6 @@ class device_memory_resource {
*
* The returned pointer will have at minimum 256 byte alignment.
*
* If supported, this operation may optionally be executed on a stream.
* Otherwise, the stream is ignored and the null stream is used.
*
* @throws `rmm::bad_alloc` When the requested `bytes` cannot be allocated on
* the specified `stream`.
*
Expand All @@ -185,9 +182,6 @@ class device_memory_resource {
* it points to must not yet have been deallocated, otherwise behavior is
* undefined.
*
* If supported, this operation may optionally be executed on a stream.
* Otherwise, the stream is ignored and the null stream is used.
*
* @throws Nothing.
*
* @param ptr Pointer to be deallocated
Expand All @@ -205,9 +199,6 @@ class device_memory_resource {
*
* The returned pointer will have at minimum 256 byte alignment.
*
* If supported, this operation may optionally be executed on a stream.
* Otherwise, the stream is ignored and the null stream is used.
*
* @throws `rmm::bad_alloc` When the requested `bytes` cannot be allocated on
* the specified `stream`.
*
Expand All @@ -226,9 +217,6 @@ class device_memory_resource {
*
* The returned pointer will have at minimum 256 byte alignment.
*
* If supported, this operation may optionally be executed on a stream.
* Otherwise, the stream is ignored and the null stream is used.
*
* @throws `rmm::bad_alloc` When the requested `bytes` cannot be allocated on
* the specified `stream`.
*
Expand All @@ -249,9 +237,6 @@ class device_memory_resource {
* it points to must not yet have been deallocated, otherwise behavior is
* undefined.
*
* If supported, this operation may optionally be executed on a stream.
* Otherwise, the stream is ignored and the null stream is used.
*
* @throws Nothing.
*
* @param ptr Pointer to be deallocated
Expand All @@ -276,9 +261,6 @@ class device_memory_resource {
* it points to must not yet have been deallocated, otherwise behavior is
* undefined.
*
* If supported, this operation may optionally be executed on a stream.
* Otherwise, the stream is ignored and the null stream is used.
*
* @throws Nothing.
*
* @param ptr Pointer to be deallocated
Expand Down
2 changes: 0 additions & 2 deletions include/rmm/mr/device/per_device_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
#include <map>
#include <mutex>

#include <cuda/memory_resource>

/**
* @file per_device_resource.hpp
* @brief Management of per-device `device_memory_resource`s
Expand Down
20 changes: 11 additions & 9 deletions include/rmm/mr/device/pool_memory_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,24 @@ namespace detail {
*
* @tparam PoolResource the pool_memory_resource class
* @tparam Upstream memory_resource to use for allocating the pool.
* @tparam Property The property we want to potentially remove.
*/
template <class PoolResource, class Upstream, class = void>
template <class PoolResource, class Upstream, class Property, class = void>
struct maybe_remove_property {};

/**
* @brief Specialization of maybe_remove_property to not propagate non existing properties
*/
template <class PoolResource, class Upstream>
struct maybe_remove_property<
PoolResource,
Upstream,
cuda::std::enable_if_t<!cuda::has_property<Upstream, cuda::mr::device_accessible>>> {
template <class PoolResource, class Upstream, class Property>
struct maybe_remove_property<PoolResource,
Upstream,
Property,
cuda::std::enable_if_t<!cuda::has_property<Upstream, Property>>> {
/**
* @brief Explicit removal of the friend function so we do not pretent to provide device
* @brief Explicit removal of the friend function so we do not pretend to provide device
* accessible memory
*/
friend void get_property(const PoolResource&, cuda::mr::device_accessible) = delete;
friend void get_property(const PoolResource&, Property) = delete;
};
} // namespace detail

Expand All @@ -93,7 +94,8 @@ struct maybe_remove_property<
*/
template <typename Upstream>
class pool_memory_resource final
: public detail::maybe_remove_property<pool_memory_resource<Upstream>, Upstream>,
: public detail::
maybe_remove_property<pool_memory_resource<Upstream>, Upstream, cuda::mr::device_accessible>,
public detail::stream_ordered_memory_resource<pool_memory_resource<Upstream>,
detail::coalescing_free_list>,
public cuda::forward_property<pool_memory_resource<Upstream>, Upstream> {
Expand Down
11 changes: 10 additions & 1 deletion include/rmm/mr/host/pinned_memory_resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ class pinned_memory_resource final : public host_memory_resource {
do_deallocate(ptr, rmm::detail::align_up(bytes, alignment));
}

/**
* @brief Enables the `cuda::mr::device_accessible` property
*
* This property declares that a `pinned_memory_resource` provides device accessible memory
*/
friend void get_property(pinned_memory_resource const&, cuda::mr::device_accessible) noexcept {}

private:
/**
* @brief Allocates pinned memory on the host of size at least `bytes` bytes.
Expand Down Expand Up @@ -172,6 +179,8 @@ class pinned_memory_resource final : public host_memory_resource {
ptr, bytes, alignment, [](void* ptr) { RMM_ASSERT_CUDA_SUCCESS(cudaFreeHost(ptr)); });
}
};
static_assert(cuda::mr::async_resource<pinned_memory_resource>);
static_assert(cuda::mr::async_resource_with<pinned_memory_resource,
cuda::mr::host_accessible,
cuda::mr::device_accessible>);
/** @} */ // end of group
} // namespace rmm::mr

0 comments on commit 76d55fa

Please sign in to comment.