From 776dfefde696ff0b5334a73bb17b399d22fdf83d Mon Sep 17 00:00:00 2001 From: Georgi Mirazchiyski Date: Mon, 15 Apr 2024 09:29:00 +0100 Subject: [PATCH] [HIP] Refactor HIP pointer type detection --- source/adapters/hip/common.hpp | 17 +++++ source/adapters/hip/enqueue.cpp | 115 +++++++++++++++++--------------- source/adapters/hip/usm.cpp | 15 +---- 3 files changed, 80 insertions(+), 67 deletions(-) diff --git a/source/adapters/hip/common.hpp b/source/adapters/hip/common.hpp index be332c280b..2fad48d6f3 100644 --- a/source/adapters/hip/common.hpp +++ b/source/adapters/hip/common.hpp @@ -19,6 +19,23 @@ #include #include +static inline unsigned int +getHipMemoryType(const hipPointerAttribute_t &Attributes) { +#if HIP_VERSION >= 50600000 + return Attributes.type; +#else + return Attributes.memoryType; +#endif +} + +// Error code hipErrorInvalidValue returned from hipPointerGetAttributes +// for a non-null pointer refers to an OS-allocation, hence we can work +// with the assumption that this is a pointer to a pageable host memory. +// NOTE Since ROCm version 6.0.0, the enum hipMemoryType can also be marked +// hipMemoryTypeUnregistered explicitly but until then that does not exist. +ur::Result +getMemoryTypePointerAttributeOrInvalidValue(const void *Ptr); + // Before ROCm 6, hipify doesn't support cuArrayGetDescriptor, on AMD the // hipArray can just be indexed, but on NVidia it is an opaque type and needs to // go through cuArrayGetDescriptor so implement a utility function to get the diff --git a/source/adapters/hip/enqueue.cpp b/source/adapters/hip/enqueue.cpp index 79522d4c93..9990c1b607 100644 --- a/source/adapters/hip/enqueue.cpp +++ b/source/adapters/hip/enqueue.cpp @@ -75,6 +75,18 @@ void guessLocalWorkSize(ur_device_handle_t Device, size_t *ThreadsPerBlock, MaxBlockDim, MaxThreadsPerBlock[0]); } +ur::Result +getMemoryTypePointerAttributeOrInvalidValue(const void *Ptr) { + hipPointerAttribute_t Attributes{}; + hipError_t Ret = hipPointerGetAttributes(&Attributes, Ptr); + if (Ret == hipErrorInvalidValue && Ptr) + return mapErrorUR(Ret); + // Direct usage of the function, instead of UR_CHECK_ERROR, to get line + // offset. + checkErrorUR(Ret, __func__, __LINE__ - 4, __FILE__); + return getHipMemoryType(Attributes); +} + namespace { ur_result_t setHipMemAdvise(const void *DevPtr, const size_t Size, @@ -1614,6 +1626,24 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueUSMFill2D( return UR_RESULT_ERROR_UNSUPPORTED_FEATURE; } +// Determine the memory type for shared allocations. +// +// ROCm 5.7.1 finally started updating the type attribute member to +// hipMemoryTypeManaged for shared memory allocations(hipMallocManaged). +// Hence, we use a separate query that verifies the pointer use via flags. +static inline void +getManagedPointerTypeLocation([[maybe_unused]] unsigned int &memType, + [[maybe_unused]] void *pMem) { +#if HIP_VERSION_MAJOR >= 5 + UR_CHECK_ERROR( + hipPointerGetAttribute(&memType, HIP_POINTER_ATTRIBUTE_MEMORY_TYPE, + reinterpret_cast(pMem))); + // Verify the memory location; can only be device or host. + UR_ASSERT(memType == hipMemoryTypeHost || memType == hipMemoryTypeDevice, + UR_RESULT_ERROR_INVALID_MEM_OBJECT); +#endif +} + /// 2D Memcpy API /// /// \param hQueue is the queue to submit to @@ -1654,65 +1684,40 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueUSMMemcpy2D( // the copies since ROCm 5.6.0+. See: https://github.com/ROCm/clr/issues/40 // TODO: Add maximum HIP_VERSION when bug has been fixed. #if HIP_VERSION >= 50600000 - hipPointerAttribute_t srcAttribs{}; - hipPointerAttribute_t dstAttribs{}; - - // Determine if pSrc and/or pDst are system allocated pageable host memory. - bool srcIsSystemAlloc{false}; - bool dstIsSystemAlloc{false}; - - hipError_t hipRes{}; - // Error code hipErrorInvalidValue returned from hipPointerGetAttributes - // for a non-null pointer refers to an OS-allocation, hence we can work - // with the assumption that this is a pointer to a pageable host memory. - // Since ROCm version 6.0.0, the enum hipMemoryType can also be marked as - // hipMemoryTypeUnregistered explicitly to relay that information better. - // This means we cannot rely on any attribute result, hence we just mark - // the pointer handle as system allocated pageable host memory. - // The HIP runtime can handle the registering/unregistering of the memory - // as long as the right copy-kind (direction) is provided to hipMemcpy2D*. - hipRes = hipPointerGetAttributes(&srcAttribs, pSrc); - if (hipRes == hipErrorInvalidValue && pSrc) - srcIsSystemAlloc = true; - hipRes = hipPointerGetAttributes(&dstAttribs, (const void *)pDst); - if (hipRes == hipErrorInvalidValue && pDst) - dstIsSystemAlloc = true; + bool srcIsHost{false}; + bool srcIsDevice{false}; + bool dstIsHost{false}; + bool dstIsDevice{false}; + + static const auto isHostMemoryType = [](unsigned int Type) -> bool { #if HIP_VERSION_MAJOR >= 6 - srcIsSystemAlloc |= srcAttribs.type == hipMemoryTypeUnregistered; - dstIsSystemAlloc |= dstAttribs.type == hipMemoryTypeUnregistered; + return Type == hipMemoryTypeHost || Type == hipMemoryTypeUnregistered; +#else + return Type == hipMemoryTypeHost; #endif + }; - unsigned int srcMemType{srcAttribs.type}; - unsigned int dstMemType{dstAttribs.type}; - - // ROCm 5.7.1 finally started updating the type attribute member to - // hipMemoryTypeManaged for shared memory allocations(hipMallocManaged). - // Hence, we use a separate query that verifies the pointer use via flags. -#if HIP_VERSION >= 50700001 - // Determine the source/destination memory type for shared allocations. - // - // NOTE: The hipPointerGetAttribute API is marked as [BETA] and fails with - // exit code -11 when passing a system allocated pointer to it. - if (!srcIsSystemAlloc && srcAttribs.isManaged) { - UR_ASSERT(srcAttribs.hostPointer && srcAttribs.devicePointer, - UR_RESULT_ERROR_INVALID_VALUE); - UR_CHECK_ERROR(hipPointerGetAttribute( - &srcMemType, HIP_POINTER_ATTRIBUTE_MEMORY_TYPE, - reinterpret_cast(const_cast(pSrc)))); - } - if (!dstIsSystemAlloc && dstAttribs.isManaged) { - UR_ASSERT(dstAttribs.hostPointer && dstAttribs.devicePointer, - UR_RESULT_ERROR_INVALID_VALUE); - UR_CHECK_ERROR( - hipPointerGetAttribute(&dstMemType, HIP_POINTER_ATTRIBUTE_MEMORY_TYPE, - reinterpret_cast(pDst))); + auto srcTypeRes = getMemoryTypePointerAttributeOrInvalidValue(pSrc); + if (srcTypeRes.get_error() == UR_RESULT_ERROR_INVALID_VALUE) + srcIsHost = true; + if (srcTypeRes) { + unsigned int srcMemType = *srcTypeRes.get_value(); + if (srcMemType == hipMemoryTypeManaged) + getManagedPointerTypeLocation(srcMemType, const_cast(pSrc)); + srcIsHost = IsHostMemoryType(srcMemType); + srcIsDevice = srcMemType == hipMemoryTypeDevice; + } + + auto dstTypeRes = getMemoryTypePointerAttributeOrInvalidValue((const void *)pDst); + if (dstTypeRes.get_error() == UR_RESULT_ERROR_INVALID_VALUE) + dstIsHost = true; + if (dstTypeRes) { + unsigned int dstMemType = *dstTypeRes.get_value(); + if (dstMemType == hipMemoryTypeManaged) + getManagedPointerTypeLocation(dstMemType, pDst); + dstIsHost = IsHostMemoryType(dstMemType); + dstIsDevice = dstMemType == hipMemoryTypeDevice; } -#endif - - const bool srcIsHost{(srcMemType == hipMemoryTypeHost) || srcIsSystemAlloc}; - const bool srcIsDevice{srcMemType == hipMemoryTypeDevice}; - const bool dstIsHost{(dstMemType == hipMemoryTypeHost) || dstIsSystemAlloc}; - const bool dstIsDevice{dstMemType == hipMemoryTypeDevice}; unsigned int cpyKind{}; if (srcIsHost && dstIsHost) diff --git a/source/adapters/hip/usm.cpp b/source/adapters/hip/usm.cpp index f29fab7b92..bd9d986f2d 100644 --- a/source/adapters/hip/usm.cpp +++ b/source/adapters/hip/usm.cpp @@ -73,11 +73,7 @@ USMFreeImpl([[maybe_unused]] ur_context_handle_t hContext, void *pMem) { try { hipPointerAttribute_t hipPointerAttributeType; UR_CHECK_ERROR(hipPointerGetAttributes(&hipPointerAttributeType, pMem)); -#if HIP_VERSION >= 50600000 - const auto Type = hipPointerAttributeType.type; -#else - const auto Type = hipPointerAttributeType.memoryType; -#endif + const unsigned int Type = getHipMemoryType(hipPointerAttributeType); UR_ASSERT(Type == hipMemoryTypeDevice || Type == hipMemoryTypeHost || Type == hipMemoryTypeManaged, UR_RESULT_ERROR_INVALID_MEM_OBJECT); @@ -169,19 +165,14 @@ urUSMGetMemAllocInfo(ur_context_handle_t hContext, const void *pMem, // Direct usage of the function, instead of UR_CHECK_ERROR, so we can get // the line offset. checkErrorUR(Ret, __func__, __LINE__ - 5, __FILE__); + const unsigned int Value = getHipMemoryType(hipPointerAttributeType); // ROCm 6.0.0 introduces hipMemoryTypeUnregistered in the hipMemoryType // enum to mark unregistered allocations (i.e., via system allocators). #if HIP_VERSION_MAJOR >= 6 - if (hipPointerAttributeType.type == hipMemoryTypeUnregistered) { + if (Value == hipMemoryTypeUnregistered) { // pointer not known to the HIP subsystem return ReturnValue(UR_USM_TYPE_UNKNOWN); } -#endif - unsigned int Value; -#if HIP_VERSION >= 50600000 - Value = hipPointerAttributeType.type; -#else - Value = hipPointerAttributeType.memoryType; #endif UR_ASSERT(Value == hipMemoryTypeDevice || Value == hipMemoryTypeHost || Value == hipMemoryTypeManaged,