From e448ed564e6f7146477a99c5d03acdda88254f9a Mon Sep 17 00:00:00 2001 From: koparasy Date: Mon, 10 Jun 2024 09:05:59 -0700 Subject: [PATCH] Fix comments --- src/AMSlib/AMS.cpp | 6 +- src/AMSlib/wf/device.hpp | 16 +++-- src/AMSlib/wf/resource_manager.cpp | 102 +++++++++++++++-------------- src/AMSlib/wf/resource_manager.hpp | 24 +++---- tests/AMSlib/CMakeLists.txt | 1 + 5 files changed, 83 insertions(+), 66 deletions(-) diff --git a/src/AMSlib/AMS.cpp b/src/AMSlib/AMS.cpp index d78538d3..5c5d3066 100644 --- a/src/AMSlib/AMS.cpp +++ b/src/AMSlib/AMS.cpp @@ -389,8 +389,9 @@ class AMSWrap } else if (log_prefix.find("") != std::string::npos) { pattern = std::string(""); id = getpid(); - } // Combine hostname and pid + } + // Combine hostname and pid std::ostringstream combined; combined << "." << hostname << "." << id; @@ -665,7 +666,8 @@ const char *AMSGetAllocatorName(AMSResourceType device) void AMSSetAllocator(AMSResourceType resource, const char *alloc_name) { auto &rm = ams::ResourceManager::getInstance(); - rm.setAllocator(std::string(alloc_name), resource); + std::string alloc(alloc_name); + rm.setAllocator(alloc, resource); } AMSCAbstrModel AMSRegisterAbstractModel(const char *domain_name, diff --git a/src/AMSlib/wf/device.hpp b/src/AMSlib/wf/device.hpp index a7dd7bdf..d197bd21 100644 --- a/src/AMSlib/wf/device.hpp +++ b/src/AMSlib/wf/device.hpp @@ -16,6 +16,8 @@ #include "AMS.h" #include "wf/debug.h" +#define UNDEFINED_FUNC -1 + #ifdef __ENABLE_CUDA__ namespace ams { @@ -254,6 +256,7 @@ inline void computePredicate(float *data, const size_t kneigh, float threshold) { + FATAL(Device, "Called device code when CUDA disabled"); return; } @@ -265,6 +268,7 @@ inline void linearize(TypeOutValue *output, size_t dims, size_t elements) { + FATAL(Device, "Called device code when CUDA disabled"); return; } @@ -277,7 +281,8 @@ inline int pack(bool cond, TypeValue **dense, int dims) { - return -1; + FATAL(Device, "Called device code when CUDA disabled"); + return UNDEFINED_FUNC; } template @@ -290,7 +295,8 @@ inline int pack(bool cond, int *sparse_indices, int dims) { - return -1; + FATAL(Device, "Called device code when CUDA disabled"); + return UNDEFINED_FUNC; } template @@ -302,7 +308,8 @@ inline int unpack(bool cond, TypeValue **dense, int dims) { - return -1; + FATAL(Device, "Called device code when CUDA disabled"); + return UNDEFINED_FUNC; } template @@ -314,7 +321,8 @@ inline int unpack(bool cond, int *sparse_indices, int dims) { - return -1; + FATAL(Device, "Called device code when CUDA disabled"); + return UNDEFINED_FUNC; } } // namespace Device diff --git a/src/AMSlib/wf/resource_manager.cpp b/src/AMSlib/wf/resource_manager.cpp index a19deac7..8b9da265 100644 --- a/src/AMSlib/wf/resource_manager.cpp +++ b/src/AMSlib/wf/resource_manager.cpp @@ -7,9 +7,6 @@ #include #include -#include -#include -#include #include "debug.h" #include "device.hpp" @@ -18,13 +15,13 @@ namespace ams { -template -static T roundUp(T num_to_round, int multiple) +template +static T roundUp(T num_to_round, int multiple) { - return ((num_to_round + multiple- 1) / multiple) * multiple; + return ((num_to_round + multiple - 1) / multiple) * multiple; } -std::string AMSAllocator::getName() { return name; } +const std::string AMSAllocator::getName() const { return name; } struct AMSDefaultDeviceAllocator final : AMSAllocator { @@ -40,7 +37,10 @@ struct AMSDefaultHostAllocator final : AMSAllocator { AMSDefaultHostAllocator(std::string name) : AMSAllocator(name) {} ~AMSDefaultHostAllocator() = default; - void *allocate(size_t num_bytes) { return aligned_alloc(8, roundUp(num_bytes, 8)); } + void *allocate(size_t num_bytes) + { + return aligned_alloc(8, roundUp(num_bytes, 8)); + } void deallocate(void *ptr) { free(ptr); } }; @@ -63,51 +63,55 @@ void _raw_copy(void *src, AMSResourceType dest_dev, size_t num_bytes) { - if (src_dev == AMSResourceType::AMS_HOST) { - if (dest_dev == AMSResourceType::AMS_HOST) { - std::memcpy(dest, src, num_bytes); - } else if (dest_dev == AMSResourceType::AMS_DEVICE) { - HtoDMemcpy(dest, src, num_bytes); - } else if (dest_dev == AMSResourceType::AMS_PINNED) { - std::memcpy(dest, src, num_bytes); - } else { - FATAL(AMSResource, "Unknown copy dest") - } - } else if (src_dev == AMSResourceType::AMS_DEVICE) { - if (dest_dev == AMSResourceType::AMS_HOST) { - DtoHMemcpy(dest, src, num_bytes); - } else if (dest_dev == AMSResourceType::AMS_DEVICE) { - DtoDMemcpy(dest, src, num_bytes); - } else if (dest_dev == AMSResourceType::AMS_PINNED) { - DtoHMemcpy(dest, src, num_bytes); - } else { - FATAL(AMSResource, "Unknown copy dest") - } - } else if (src_dev == AMSResourceType::AMS_PINNED) { - if (dest_dev == AMSResourceType::AMS_HOST) { - std::memcpy(dest, src, num_bytes); - } else if (dest_dev == AMSResourceType::AMS_DEVICE) { - HtoDMemcpy(dest, src, num_bytes); - } else if (dest_dev == AMSResourceType::AMS_PINNED) { - std::memcpy(dest, src, num_bytes); - } else { - FATAL(AMSResource, "Unknown copy dest") - } + switch (src_dev) { + case AMSResourceType::AMS_HOST: + case AMSResourceType::AMS_PINNED: + switch (dest_dev) { + case AMSResourceType::AMS_HOST: + case AMSResourceType::AMS_PINNED: + std::memcpy(dest, src, num_bytes); + break; + case AMSResourceType::AMS_DEVICE: + HtoDMemcpy(dest, src, num_bytes); + break; + default: + FATAL(ResourceManager, "Unknown device type to copy to from HOST"); + break; + } + break; + case AMSResourceType::AMS_DEVICE: + switch (dest_dev) { + case AMSResourceType::AMS_DEVICE: + DtoDMemcpy(dest, src, num_bytes); + break; + case AMSResourceType::AMS_HOST: + case AMSResourceType::AMS_PINNED: + DtoHMemcpy(dest, src, num_bytes); + break; + default: + FATAL(ResourceManager, "Unknown device type to copy to from DEVICE"); + break; + } + default: + FATAL(ResourceManager, "Unknown device type to copy from"); } } -AMSAllocator *_get_allocator(std::string alloc_name, AMSResourceType resource) +AMSAllocator *_get_allocator(std::string &alloc_name, AMSResourceType resource) { - if (resource == AMSResourceType::AMS_DEVICE) { - return new AMSDefaultDeviceAllocator(alloc_name); - } else if (resource == AMSResourceType::AMS_HOST) { - return new AMSDefaultHostAllocator(alloc_name); - } else if (resource == AMSResourceType::AMS_PINNED) { - return new AMSDefaultPinnedAllocator(alloc_name); - } else { - FATAL(ResourceManager, - "Requested allocator %s for Unknown resource type", - alloc_name.c_str()); + switch (resource) { + case AMSResourceType::AMS_DEVICE: + return new AMSDefaultDeviceAllocator(alloc_name); + break; + case AMSResourceType::AMS_HOST: + return new AMSDefaultHostAllocator(alloc_name); + break; + case AMSResourceType::AMS_PINNED: + return new AMSDefaultPinnedAllocator(alloc_name); + break; + default: + FATAL(ResourceManager, + "Unknown resource type to create an allocator for"); } } diff --git a/src/AMSlib/wf/resource_manager.hpp b/src/AMSlib/wf/resource_manager.hpp index 88d44447..b86fdb6b 100644 --- a/src/AMSlib/wf/resource_manager.hpp +++ b/src/AMSlib/wf/resource_manager.hpp @@ -9,6 +9,7 @@ #define __AMS_ALLOCATOR__ #include +#include #include #include "AMS.h" @@ -27,7 +28,7 @@ void _raw_copy(void* src, AMSResourceType dest_dev, size_t num_bytes); -AMSAllocator* _get_allocator(std::string alloc_name, AMSResourceType resource); +AMSAllocator* _get_allocator(std::string& alloc_name, AMSResourceType resource); void _release_allocator(AMSAllocator* allocator); } // namespace internal @@ -40,16 +41,15 @@ void _release_allocator(AMSAllocator* allocator); struct AMSAllocator { std::string name; - AMSAllocator(std::string alloc_name) : name(alloc_name) {} + AMSAllocator(std::string& alloc_name) : name(alloc_name) {} virtual ~AMSAllocator() = default; virtual void* allocate(size_t num_bytes) = 0; virtual void deallocate(void* ptr) = 0; - std::string getName(); + const std::string getName() const; }; - class ResourceManager { private: @@ -77,7 +77,7 @@ class ResourceManager } /** @brief return the name of an allocator */ - std::string getAllocatorName(AMSResourceType resource) + const std::string getAllocatorName(AMSResourceType resource) const { return RMAllocators[resource]->getName(); } @@ -113,7 +113,7 @@ class ResourceManager * @tparam TypeInValue type of pointers * @param[in] src Source memory pointer. * @param[out] dest destination memory pointer. - * @param[in] size number of values to copy. + * @param[in] size number of values to copy (It performs a shallow copy of nested pointers). * @return void. */ template @@ -146,18 +146,21 @@ class ResourceManager void init() { DBG(ResourceManager, "Initialization of allocators"); + std::string host_alloc("HOST"); + std::string device_alloc("DEVICE"); + std::string pinned_alloc("PINNED"); if (!RMAllocators[AMSResourceType::AMS_HOST]) - setAllocator("HOST", AMSResourceType::AMS_HOST); + setAllocator(host_alloc, AMSResourceType::AMS_HOST); #ifdef __ENABLE_CUDA__ if (!RMAllocators[AMSResourceType::AMS_DEVICE]) - setAllocator("DEVICE", AMSResourceType::AMS_DEVICE); + setAllocator(host_alloc, AMSResourceType::AMS_DEVICE); if (!RMAllocators[AMSResourceType::AMS_PINNED]) - setAllocator("PINNED", AMSResourceType::AMS_PINNED); + setAllocator(pinned_alloc, AMSResourceType::AMS_PINNED); #endif } - void setAllocator(std::string alloc_name, AMSResourceType resource) + void setAllocator(std::string& alloc_name, AMSResourceType resource) { if (RMAllocators[resource]) { delete RMAllocators[resource]; @@ -194,7 +197,6 @@ class ResourceManager //! ------------------------------------------------------------------------ }; - } // namespace ams #endif diff --git a/tests/AMSlib/CMakeLists.txt b/tests/AMSlib/CMakeLists.txt index 1c85e22a..5571a922 100644 --- a/tests/AMSlib/CMakeLists.txt +++ b/tests/AMSlib/CMakeLists.txt @@ -156,6 +156,7 @@ function(ADDTEST exe test_name) endfunction() # This test requires Allocate +# TODO: Include tests once we re-instate a pool #BUILD_TEST(ams_allocator_test ams_allocate.cpp) #ADDTEST(ams_allocator_test AMSAllocate) BUILD_TEST(ams_packing_test cpu_packing_test.cpp AMSPack)