diff --git a/projects/clr/hipamd/include/hip/amd_detail/amd_hip_gl_interop.h b/projects/clr/hipamd/include/hip/amd_detail/amd_hip_gl_interop.h index 72a8f63f61..db1b61f37b 100644 --- a/projects/clr/hipamd/include/hip/amd_detail/amd_hip_gl_interop.h +++ b/projects/clr/hipamd/include/hip/amd_detail/amd_hip_gl_interop.h @@ -81,7 +81,7 @@ hipError_t hipGLGetDevices(unsigned int* pHipDeviceCount, int* pHipDevices, * * @param [out] resource - Returns pointer of graphics resource. * @param [in] buffer - Buffer to be registered. - * @param [in] flags - Register flags. + * @param [in] flags - Register OpenGL flags. * * @returns #hipSuccess, #hipErrorInvalidValue, #hipErrorUnknown, #hipErrorInvalidResourceHandle * @@ -94,7 +94,7 @@ hipError_t hipGraphicsGLRegisterBuffer(hipGraphicsResource** resource, GLuint bu * @param [out] resource - Returns pointer of graphics resource. * @param [in] image - Image to be registered. * @param [in] target - Valid target value Id. - * @param [in] flags - Register flags. + * @param [in] flags - Register OpenGL flags. * * @returns #hipSuccess, #hipErrorInvalidValue, #hipErrorUnknown, #hipErrorInvalidResourceHandle * diff --git a/projects/clr/hipamd/src/hip_device.cpp b/projects/clr/hipamd/src/hip_device.cpp index 2869822c14..697c2be51f 100644 --- a/projects/clr/hipamd/src/hip_device.cpp +++ b/projects/clr/hipamd/src/hip_device.cpp @@ -325,6 +325,8 @@ Device::~Device() { default_mem_pool_->release(); } + clearAllTrackedObjects(); + if (graph_mem_pool_ != nullptr) { graph_mem_pool_->release(); } diff --git a/projects/clr/hipamd/src/hip_gl.cpp b/projects/clr/hipamd/src/hip_gl.cpp index d2037f478a..3648a1bd27 100644 --- a/projects/clr/hipamd/src/hip_gl.cpp +++ b/projects/clr/hipamd/src/hip_gl.cpp @@ -64,6 +64,35 @@ void setupGLInteropOnce() { } } +static inline hipError_t validateResources(hipGraphicsResource_t* resources, int count = 1) { + hip::Device* device = hip::getCurrentDevice(); + if (device == nullptr) { + return hipErrorNoDevice; + } + + if (count <= 0) { + LogError("invalid count"); + return hipErrorInvalidValue; + } + + if (resources == nullptr) { + return hipErrorInvalidValue; + } + + for (int i = 0; i < count; i++) { + if (resources[i] == nullptr) { + return hipErrorInvalidValue; + } + if (!device->registeredGraphics().isValid(resources[i])) { + return hipErrorInvalidHandle; + } + if (!device->mappedGraphics().isValid(resources[i])) { + return hipErrorNotMapped; + } + } + return hipSuccess; +} + static inline hipError_t hipSetInteropObjects(int num_objects, void** mem_objects, std::vector& interopObjects) { if ((num_objects == 0 && mem_objects != nullptr) || @@ -74,13 +103,13 @@ static inline hipError_t hipSetInteropObjects(int num_objects, void** mem_object while (num_objects-- > 0) { void* obj = *mem_objects++; if (obj == nullptr) { - return hipErrorInvalidResourceHandle; + return hipErrorInvalidHandle; } amd::Memory* mem = reinterpret_cast(obj); if (mem->getInteropObj() == nullptr) { - return hipErrorInvalidResourceHandle; + return hipErrorInvalidHandle; } interopObjects.push_back(mem); @@ -184,20 +213,39 @@ hipError_t hipGraphicsSubResourceGetMappedArray(hipArray_t* array, hipGraphicsRe HIP_INIT_API(hipGraphicsSubResourceGetMappedArray, array, resource, arrayIndex, mipLevel); amd::Context& amdContext = *(hip::getCurrentDevice()->asContext()); - if (array == nullptr || resource == nullptr) { - LogError("invalid array/resource"); + if (array == nullptr) { + LogError("invalid array"); HIP_RETURN(hipErrorInvalidValue); } + hipError_t status = validateResources(&resource); + if (status != hipSuccess) { + LogError("invalid resource"); + HIP_RETURN(status); + } + amd::Image* image = (reinterpret_cast(resource))->asImage(); if (image == nullptr) { LogError("invalid resource/image"); + HIP_RETURN(hipErrorNotMappedAsArray); + } + // arrayIndex higher than zero not implemented + if (arrayIndex > 0) { + LogError("invalid arrayIndex, arrayIndex higher than zero not implemented"); HIP_RETURN(hipErrorInvalidValue); } - // arrayIndex higher than zero not implmented - if (arrayIndex > 0) { - return hipErrorInvalidValue; + + size_t height = image->getHeight(); + size_t width = image->getWidth(); + size_t depth = image->getDepth(); + size_t max_dim = std::max({height, width, depth}); + unsigned int max_mipLevel = 1 + static_cast(std::floor(std::log2(max_dim))); + + if (mipLevel > max_mipLevel) { + LogError("invalid mipLevel"); + HIP_RETURN(hipErrorInvalidValue); } + amd::Image* view = image->createView(amdContext, image->getImageFormat(), nullptr, mipLevel, 0); hipArray* myarray = new hipArray(); @@ -225,16 +273,43 @@ hipError_t hipGraphicsSubResourceGetMappedArray(hipArray_t* array, hipGraphicsRe HIP_RETURN(hipSuccess); } +// Helper function to convert from OpenGL Flags to HIP Memory Flags +hipError_t HipToClMemoryFlags(uint32_t gl_flags, cl_mem_flags* cl_flags) { + if (cl_flags == nullptr) { + return hipErrorInvalidValue; + } + switch (gl_flags) { + case hipGraphicsRegisterFlagsNone: + *cl_flags = 0; + break; + case hipGraphicsRegisterFlagsReadOnly: + *cl_flags = CL_MEM_READ_ONLY; + break; + case hipGraphicsRegisterFlagsWriteDiscard: + *cl_flags = CL_MEM_WRITE_ONLY; + break; + case hipGraphicsRegisterFlagsSurfaceLoadStore: + *cl_flags = CL_MEM_READ_WRITE; + break; + case hipGraphicsRegisterFlagsTextureGather: + *cl_flags = CL_MEM_READ_WRITE | CL_MEM_READ_ONLY; + break; + default: + return hipErrorInvalidValue; + break; + } + return hipSuccess; +} + hipError_t hipGraphicsGLRegisterImage(hipGraphicsResource** resource, GLuint image, GLenum target, unsigned int flags) { HIP_INIT_API(hipGraphicsGLRegisterImage, resource, image, target, flags); - if (!((flags == hipGraphicsRegisterFlagsNone) || (flags & hipGraphicsRegisterFlagsReadOnly) || - (flags & hipGraphicsRegisterFlagsWriteDiscard) || - (flags & hipGraphicsRegisterFlagsSurfaceLoadStore) || - (flags & hipGraphicsRegisterFlagsTextureGather))) { - LogError("invalid parameter \"flags\""); - HIP_RETURN(hipErrorInvalidValue); + cl_mem_flags cl_flags = 0; + hipError_t status = HipToClMemoryFlags(flags, &cl_flags); + if (status != hipSuccess) { + LogPrintfError("invalid parameter \"flags\" %u, gl interop can not convert", flags); + HIP_RETURN(status); } if (resource == nullptr) { @@ -275,7 +350,7 @@ hipError_t hipGraphicsGLRegisterImage(hipGraphicsResource** resource, GLuint ima if ((GL_FALSE == amdContext.glenv()->glIsTexture_(image)) || (GL_NO_ERROR != (glErr = amdContext.glenv()->glGetError_()))) { LogWarning("\"texture\" is not a GL texture object"); - HIP_RETURN(hipErrorUnknown); + HIP_RETURN(hipErrorInvalidValue); } bool isImage = true; @@ -470,7 +545,7 @@ hipError_t hipGraphicsGLRegisterImage(hipGraphicsResource** resource, GLuint ima // Now get CL format from GL format and bytes per pixel int iBytesPerPixel = 0; if (!amd::getCLFormatFromGL(amdContext, glInternalFormat, &clImageFormat, &iBytesPerPixel, - flags)) { + cl_flags)) { LogWarning("\"texture\" format does not map to an appropriate CL image format"); HIP_RETURN(hipErrorInvalidValue); } @@ -496,7 +571,7 @@ hipError_t hipGraphicsGLRegisterImage(hipGraphicsResource** resource, GLuint ima target = (glTarget == GL_TEXTURE_CUBE_MAP) ? target : 0; pImageGL = new (amdContext) - amd::ImageGL(amdContext, clType, flags, clImageFormat, static_cast(gliTexWidth), + amd::ImageGL(amdContext, clType, cl_flags, clImageFormat, static_cast(gliTexWidth), static_cast(gliTexHeight), static_cast(gliTexDepth), glTarget, image, 0, glInternalFormat, clGLType, numSamples, target); @@ -530,6 +605,16 @@ hipError_t hipGraphicsGLRegisterImage(hipGraphicsResource** resource, GLuint ima mem->processGLResource(device::Memory::GLDecompressResource); *resource = reinterpret_cast(pImageGL); + + hip::Device* device = hip::getCurrentDevice(); + if (device == nullptr) { + return hipErrorNoDevice; + } + + if (!device->registeredGraphics().add(*resource)) { + LogError("duplicate resource"); + HIP_RETURN(hipErrorUnknown); + } HIP_RETURN(hipSuccess); } @@ -537,10 +622,11 @@ hipError_t hipGraphicsGLRegisterBuffer(hipGraphicsResource** resource, GLuint bu unsigned int flags) { HIP_INIT_API(hipGraphicsGLRegisterBuffer, resource, buffer, flags); - if (!((flags == hipGraphicsRegisterFlagsNone) || (flags & hipGraphicsRegisterFlagsReadOnly) || - (flags & hipGraphicsRegisterFlagsWriteDiscard))) { - LogError("invalid parameter \"flags\""); - HIP_RETURN(hipErrorInvalidValue); + cl_mem_flags cl_flags = 0; + hipError_t status = HipToClMemoryFlags(flags, &cl_flags); + if (status != hipSuccess) { + LogPrintfError("invalid parameter \"flags\" %u, gl interop can not convert", flags); + HIP_RETURN(status); } if (resource == nullptr) { @@ -574,7 +660,7 @@ hipError_t hipGraphicsGLRegisterBuffer(hipGraphicsResource** resource, GLuint bu if ((GL_FALSE == amdContext.glenv()->glIsBuffer_(buffer)) || (GL_NO_ERROR != (glErr = amdContext.glenv()->glGetError_()))) { LogWarning("\"buffer\" is not a GL buffer object"); - HIP_RETURN(hipErrorInvalidResourceHandle); + HIP_RETURN(hipErrorInvalidValue); } // Check if size is available - data store is created @@ -583,17 +669,17 @@ hipError_t hipGraphicsGLRegisterBuffer(hipGraphicsResource** resource, GLuint bu amdContext.glenv()->glGetBufferParameteriv_(glTarget, GL_BUFFER_SIZE, &gliSize); if (GL_NO_ERROR != (glErr = amdContext.glenv()->glGetError_())) { LogWarning("cannot get the GL buffer size"); - HIP_RETURN(hipErrorInvalidResourceHandle); + HIP_RETURN(hipErrorInvalidValue); } if (gliSize == 0) { LogWarning("the GL buffer's data store is not created"); - HIP_RETURN(hipErrorInvalidResourceHandle); + HIP_RETURN(hipErrorInvalidValue); } } // Release scoped lock // Now create BufferGL object - pBufferGL = new (amdContext) amd::BufferGL(amdContext, flags, gliSize, 0, buffer); + pBufferGL = new (amdContext) amd::BufferGL(amdContext, cl_flags, gliSize, 0, buffer); if (!pBufferGL) { LogWarning("cannot create object of class BufferGL"); @@ -627,12 +713,35 @@ hipError_t hipGraphicsGLRegisterBuffer(hipGraphicsResource** resource, GLuint bu *resource = reinterpret_cast(pBufferGL); + hip::Device* device = hip::getCurrentDevice(); + if (device == nullptr) { + return hipErrorNoDevice; + } + + if (!device->registeredGraphics().add(*resource)) { + LogError("duplicate resource"); + HIP_RETURN(hipErrorUnknown); + } HIP_RETURN(hipSuccess); } - + hipError_t hipGraphicsMapResources(int count, hipGraphicsResource_t* resources, hipStream_t stream) { HIP_INIT_API(hipGraphicsMapResources, count, resources, stream); + + if (!hip::isValid(stream)) { + HIP_RETURN(hipErrorContextIsDestroyed); + } + + hipError_t status = validateResources(resources, count); + if (status != hipErrorNotMapped) { + LogError("invalid resource(s)"); + if (status == hipSuccess) { + status = hipErrorAlreadyMapped; + } + HIP_RETURN(status); + } + amd::Context* amdContext = hip::getCurrentDevice()->asContext(); if (!amdContext || !amdContext->glenv()) { HIP_RETURN(hipErrorUnknown); @@ -645,12 +754,12 @@ hipError_t hipGraphicsMapResources(int count, hipGraphicsResource_t* resources, hip::Stream* hip_stream = hip::getStream(stream); if (nullptr == hip_stream) { - HIP_RETURN(hipErrorUnknown); + HIP_RETURN(hipErrorContextIsDestroyed); } if (!hip_stream->context().glenv() || !hip_stream->context().glenv()->isAssociated()) { LogWarning("\"amdContext\" is not created from GL context or share list"); - HIP_RETURN(hipErrorUnknown); + HIP_RETURN(hipErrorContextIsDestroyed); } std::vector memObjects; @@ -688,12 +797,39 @@ hipError_t hipGraphicsMapResources(int count, hipGraphicsResource_t* resources, amd::MemObjMap::AddMemObj(reinterpret_cast(mem->virtualAddress()), mobj); mobj->retain(); } + // Track mapping status + hip::Device* device = hip::getCurrentDevice(); + if (device == nullptr) { + return hipErrorNoDevice; + } + for (int i = 0; i < count; i++) { + if (!device->mappedGraphics().add(resources[i])) { + HIP_RETURN(hipErrorMapFailed); + } + } HIP_RETURN(hipSuccess); } hipError_t hipGraphicsResourceGetMappedPointer(void** devPtr, size_t* size, hipGraphicsResource_t resource) { HIP_INIT_API(hipGraphicsResourceGetMappedPointer, devPtr, size, resource); + + if (devPtr == nullptr) { + LogError("invalid device pointer"); + HIP_RETURN(hipErrorInvalidValue); + } + + if (size == nullptr) { + LogError("invalid size"); + HIP_RETURN(hipErrorInvalidValue); + } + + hipError_t status = validateResources(&resource); + if (status != hipSuccess) { + LogError("invalid resource"); + HIP_RETURN(status); + } + amd::Context* amdContext = hip::getCurrentDevice()->asContext(); if (!amdContext || !amdContext->glenv()) { HIP_RETURN(hipErrorUnknown); @@ -707,12 +843,27 @@ hipError_t hipGraphicsResourceGetMappedPointer(void** devPtr, size_t* size, amd::Device* curDev = *it; amd::Memory* amdMem = reinterpret_cast(resource); + + // Check if not a buffer + amd::Buffer* buffer = amdMem->asBuffer(); + if (buffer == nullptr) { + LogError("resource not mapped as pointer"); + HIP_RETURN(hipErrorNotMappedAsPointer); + } + *size = amdMem->getSize(); // Interop resources don't have svm allocations they are added to // amd::MemObjMap using device virtual address during creation. - device::Memory* mem = reinterpret_cast(amdMem->getDeviceMemory(*curDev)); - *devPtr = reinterpret_cast(static_cast(mem->virtualAddress())); + device::Memory* devMem = reinterpret_cast(amdMem->getDeviceMemory(*curDev)); + + // Not mapped + if (devMem == nullptr) { + LogError("resource not mapped"); + HIP_RETURN(hipErrorNotMapped); + } + + *devPtr = reinterpret_cast(static_cast(devMem->virtualAddress())); HIP_RETURN(hipSuccess); } @@ -726,9 +877,15 @@ hipError_t hipGraphicsUnmapResources(int count, hipGraphicsResource_t* resources // Wait for the current host queue hip::getStream(stream)->finish(); + hipError_t status = validateResources(resources, count); + if (status != hipSuccess) { + LogError("resource(s) not mapped"); + HIP_RETURN(status); + } + hip::Stream* hip_stream = hip::getStream(stream); if (nullptr == hip_stream) { - HIP_RETURN(hipErrorUnknown); + HIP_RETURN(hipErrorContextIsDestroyed); } std::vector memObjects; @@ -760,6 +917,18 @@ hipError_t hipGraphicsUnmapResources(int count, hipGraphicsResource_t* resources for (auto& mobj : memObjects) { mobj->release(); } + + // Remove mapping from registry + hip::Device* device = hip::getCurrentDevice(); + if (device == nullptr) { + return hipErrorNoDevice; + } + for (uint8_t i = 0; i < count; i++) { + if (!device->mappedGraphics().remove(resources[i])) { + LogError("failed to unmap resource"); + return hipErrorUnknown; + } + } HIP_RETURN(hipSuccess); } @@ -769,6 +938,28 @@ hipError_t hipGraphicsUnregisterResource(hipGraphicsResource_t resource) { if (resource == nullptr) { HIP_RETURN(hipErrorInvalidValue); } + hip::Device* device = hip::getCurrentDevice(); + if (device == nullptr) { + LogError("no device"); + HIP_RETURN(hipErrorNoDevice); + } + + if (device->mappedGraphics().isValid(resource)) { + LogError("resource still mapped"); + HIP_RETURN(hipErrorArrayIsMapped); + } + + if (!device->registeredGraphics().isValid(resource)) { + LogError("resource not registered"); + HIP_RETURN(hipErrorInvalidHandle); + } + + // Safe to remove from registered list + if (!device->registeredGraphics().remove(resource)) { + LogError("failed to unregister resource"); + HIP_RETURN(hipErrorUnknown); + } + reinterpret_cast(resource)->release(); HIP_RETURN(hipSuccess); diff --git a/projects/clr/hipamd/src/hip_internal.hpp b/projects/clr/hipamd/src/hip_internal.hpp index 7b93b2a166..047160d3b6 100644 --- a/projects/clr/hipamd/src/hip_internal.hpp +++ b/projects/clr/hipamd/src/hip_internal.hpp @@ -32,6 +32,7 @@ #include #include #include +#include #ifdef _WIN32 #include #else @@ -480,6 +481,40 @@ public: ~Stream() {}; }; + // Generic object tracking class of type T + template + class ObjectRegistry { + public: + // Adds the object to the set. Returns true if successful + bool add(T object) { + if (object == nullptr) return false; + + std::lock_guard lock(mtx_); + auto result = objects_.insert(object); + return result.second; + } + // Removes the object from the set. Returns true if successful + bool remove(T object) { + std::lock_guard lock(mtx_); + return objects_.erase(object) > 0; + } + // Returns true if set contains object + bool isValid(T object) const { + if (object == nullptr) return false; + std::lock_guard lock(mtx_); + return objects_.find(object) != objects_.end(); + } + // Clears the set + void clear() { + std::lock_guard lock(mtx_); + objects_.clear(); + } + + private: + mutable std::mutex mtx_; + std::unordered_set objects_; + }; + /// HIP Device class class Device : public amd::ReferenceCountedObject { // Device lock @@ -508,6 +543,9 @@ public: MemoryPool* graph_mem_pool_; //!< Memory pool, associated with graphs for this device std::set mem_pools_; + // Tracking Objects + ObjectRegistry registeredGraphicsResources_; //!< Track registered graphics resources + ObjectRegistry mappedGraphicsResources_; //!< Track mapped graphics resources public: Device(amd::Context* ctx, int devId): context_(ctx), @@ -617,6 +655,19 @@ public: /// Wait all active streams on the blocking queue. The method enqueues a wait command and /// doesn't stall the current thread void WaitActiveStreams(hip::Stream* blocking_stream, bool wait_null_stream = false); + + ObjectRegistry& registeredGraphics() { + return registeredGraphicsResources_; + }; + + ObjectRegistry& mappedGraphics() { + return mappedGraphicsResources_; + }; + + void clearAllTrackedObjects() { + registeredGraphicsResources_.clear(); + mappedGraphicsResources_.clear(); + } }; /// Thread Local Storage Variables Aggregator Class diff --git a/projects/hip-tests/catch/unit/gl_interop/hipGLGetDevices.cc b/projects/hip-tests/catch/unit/gl_interop/hipGLGetDevices.cc index 40d4f0860c..f822064ee7 100644 --- a/projects/hip-tests/catch/unit/gl_interop/hipGLGetDevices.cc +++ b/projects/hip-tests/catch/unit/gl_interop/hipGLGetDevices.cc @@ -96,4 +96,4 @@ TEST_CASE("Unit_hipGLGetDevices_Negative_Parameters") { REQUIRE(gl_device_count == 0); REQUIRE(gl_devices.at(0) == -1); } -} \ No newline at end of file +} diff --git a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsGLRegisterBuffer.cc b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsGLRegisterBuffer.cc index 531ca1152b..415b67154e 100644 --- a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsGLRegisterBuffer.cc +++ b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsGLRegisterBuffer.cc @@ -113,4 +113,4 @@ TEST_CASE("Unit_hipGraphicsGLRegisterBuffer_Negative_Parameters") { hipGraphicsGLRegisterBuffer(&vbo_resource, vbo, hipGraphicsRegisterFlagsTextureGather), hipErrorInvalidValue); } -} \ No newline at end of file +} diff --git a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsGLRegisterImage.cc b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsGLRegisterImage.cc index f2c62ca25a..cc372172e7 100644 --- a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsGLRegisterImage.cc +++ b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsGLRegisterImage.cc @@ -116,4 +116,4 @@ TEST_CASE("Unit_hipGraphicsGLRegisterImage_Negative_Parameters") { std::numeric_limits::max()), hipErrorInvalidValue); } -} \ No newline at end of file +} diff --git a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsMapResources.cc b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsMapResources.cc index 285f35cff4..183a6a575e 100644 --- a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsMapResources.cc +++ b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsMapResources.cc @@ -108,4 +108,4 @@ TEST_CASE("Unit_hipGraphicsMapResources_Negative_Parameters") { } HIP_CHECK(hipGraphicsUnregisterResource(vbo_resource)); -} \ No newline at end of file +} diff --git a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsResourceGetMappedPointer.cc b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsResourceGetMappedPointer.cc index 8061d4bf41..b12203eba4 100644 --- a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsResourceGetMappedPointer.cc +++ b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsResourceGetMappedPointer.cc @@ -60,7 +60,7 @@ TEST_CASE("Unit_hipGraphicsResourceGetMappedPointer_Positive_Basic") { HIP_CHECK(hipGraphicsUnregisterResource(vbo_resource)); } -TEST_CASE("Unit_hipGraphicsResourceGetMappedPointer_Positive_Parameters") { +TEST_CASE("Unit_hipGraphicsResourceGetMappedPointer_Null_Parameters") { GLContextScopeGuard gl_context; const int device_count = HipTest::getDeviceCount(); @@ -84,14 +84,23 @@ TEST_CASE("Unit_hipGraphicsResourceGetMappedPointer_Positive_Parameters") { size_t size = 0; SECTION("devPtr == nullptr") { - HIP_CHECK(hipGraphicsResourceGetMappedPointer(nullptr, &size, vbo_resource)); - REQUIRE(size == vbo.kSize); + HIP_CHECK_ERROR(hipGraphicsResourceGetMappedPointer(nullptr, &size, vbo_resource), hipErrorInvalidValue); } SECTION("size == nullptr") { - HIP_CHECK(hipGraphicsResourceGetMappedPointer(reinterpret_cast(&buffer_devptr), nullptr, - vbo_resource)); - REQUIRE(buffer_devptr != nullptr); + HIP_CHECK_ERROR(hipGraphicsResourceGetMappedPointer(reinterpret_cast(&buffer_devptr), nullptr, + vbo_resource), hipErrorInvalidValue); + } + + SECTION("resource == nullptr") { + hipGraphicsResource* null_resource = nullptr; + HIP_CHECK_ERROR(hipGraphicsResourceGetMappedPointer(reinterpret_cast(&buffer_devptr), &size, + null_resource), hipErrorInvalidValue); + } + + SECTION("devPtr == nullptr && size == nullptr") { + HIP_CHECK_ERROR(hipGraphicsResourceGetMappedPointer(nullptr, nullptr, + vbo_resource), hipErrorInvalidValue); } HIP_CHECK(hipGraphicsUnmapResources(1, &vbo_resource, 0)); @@ -145,7 +154,7 @@ TEST_CASE("Unit_hipGraphicsResourceGetMappedPointer_Negative_Parameters") { HIP_CHECK(hipGraphicsUnregisterResource(unregistered_resource)); HIP_CHECK_ERROR(hipGraphicsResourceGetMappedPointer(reinterpret_cast(&buffer_devptr), &size, unregistered_resource), - hipErrorContextIsDestroyed); + hipErrorInvalidHandle); } SECTION("not mapped resource") { @@ -175,4 +184,4 @@ TEST_CASE("Unit_hipGraphicsResourceGetMappedPointer_Negative_Parameters") { HIP_CHECK(hipGraphicsUnmapResources(1, &vbo_resource, 0)); HIP_CHECK(hipGraphicsUnregisterResource(vbo_resource)); -} \ No newline at end of file +} diff --git a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsSubResourceGetMappedArray.cc b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsSubResourceGetMappedArray.cc index ef2fd0808f..e477916e31 100644 --- a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsSubResourceGetMappedArray.cc +++ b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsSubResourceGetMappedArray.cc @@ -81,7 +81,7 @@ TEST_CASE("Unit_hipGraphicsSubResourceGetMappedArray_Negative_Parameters") { hipArray_t image_devptr = nullptr; SECTION("array == nullptr") { - HIP_CHECK(hipGraphicsSubResourceGetMappedArray(nullptr, tex_resource, 0, 0)); + HIP_CHECK_ERROR(hipGraphicsSubResourceGetMappedArray(nullptr, tex_resource, 0, 0), hipErrorInvalidValue); } SECTION("non-texture resource") { @@ -105,7 +105,7 @@ TEST_CASE("Unit_hipGraphicsSubResourceGetMappedArray_Negative_Parameters") { HIP_CHECK(hipGraphicsUnregisterResource(unregistered_resource)); HIP_CHECK_ERROR( hipGraphicsSubResourceGetMappedArray(&image_devptr, unregistered_resource, 0, 0), - hipErrorContextIsDestroyed); + hipErrorInvalidHandle); } SECTION("not mapped resource") { @@ -147,4 +147,4 @@ TEST_CASE("Unit_hipGraphicsSubResourceGetMappedArray_Negative_Parameters") { HIP_CHECK(hipGraphicsUnmapResources(1, &tex_resource, 0)); HIP_CHECK(hipGraphicsUnregisterResource(tex_resource)); -} \ No newline at end of file +} diff --git a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsUnmapResources.cc b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsUnmapResources.cc index 48d3add85c..080a81f895 100644 --- a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsUnmapResources.cc +++ b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsUnmapResources.cc @@ -51,7 +51,7 @@ TEST_CASE("Unit_hipGraphicsUnmapResources_Negative_Parameters") { } SECTION("resources == nullptr") { - HIP_CHECK_ERROR(hipGraphicsUnmapResources(1, nullptr, 0), hipErrorUnknown); + HIP_CHECK_ERROR(hipGraphicsUnmapResources(1, nullptr, 0), hipErrorInvalidValue); } SECTION("not mapped resource") { @@ -72,4 +72,4 @@ TEST_CASE("Unit_hipGraphicsUnmapResources_Negative_Parameters") { HIP_CHECK(hipGraphicsUnmapResources(1, &vbo_resource, 0)); HIP_CHECK(hipGraphicsUnregisterResource(vbo_resource)); -} \ No newline at end of file +} diff --git a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsUnregisterResource.cc b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsUnregisterResource.cc index 7a74042a91..714bde40fc 100644 --- a/projects/hip-tests/catch/unit/gl_interop/hipGraphicsUnregisterResource.cc +++ b/projects/hip-tests/catch/unit/gl_interop/hipGraphicsUnregisterResource.cc @@ -40,18 +40,23 @@ TEST_CASE("Unit_hipGraphicsUnregisterResource_Negative_Parameters") { GLBufferObject vbo; - SECTION("already unregistered resource") { + SECTION("null resource") { + hipGraphicsResource* null_resource = nullptr; + HIP_CHECK_ERROR(hipGraphicsUnregisterResource(null_resource), hipErrorInvalidValue); + } + + SECTION("already unregistered resource") { hipGraphicsResource* unregistered_resource; HIP_CHECK( hipGraphicsGLRegisterBuffer(&unregistered_resource, vbo, hipGraphicsRegisterFlagsNone)); HIP_CHECK(hipGraphicsUnregisterResource(unregistered_resource)); - HIP_CHECK_ERROR(hipGraphicsUnregisterResource(unregistered_resource), hipErrorInvalidContext); + HIP_CHECK_ERROR(hipGraphicsUnregisterResource(unregistered_resource), hipErrorInvalidHandle); } SECTION("mapped resource") { hipGraphicsResource* mapped_resource; HIP_CHECK(hipGraphicsGLRegisterBuffer(&mapped_resource, vbo, hipGraphicsRegisterFlagsNone)); HIP_CHECK(hipGraphicsMapResources(1, &mapped_resource, 0)); - HIP_CHECK_ERROR(hipGraphicsUnregisterResource(mapped_resource), hipErrorAlreadyMapped); + HIP_CHECK_ERROR(hipGraphicsUnregisterResource(mapped_resource), hipErrorArrayIsMapped); } -} \ No newline at end of file +} diff --git a/projects/hip/include/hip/hip_runtime_api.h b/projects/hip/include/hip/hip_runtime_api.h index 6f6aee86fe..2833810619 100644 --- a/projects/hip/include/hip/hip_runtime_api.h +++ b/projects/hip/include/hip/hip_runtime_api.h @@ -1446,12 +1446,12 @@ void __hipGetPCH(const char** pch, unsigned int* size); */ typedef enum hipGraphicsRegisterFlags { hipGraphicsRegisterFlagsNone = 0, - hipGraphicsRegisterFlagsReadOnly = 1, ///< HIP will not write to this registered resource + hipGraphicsRegisterFlagsReadOnly = 1, ///< HIP will not write to this registered resource, read only hipGraphicsRegisterFlagsWriteDiscard = - 2, ///< HIP will only write and will not read from this registered resource - hipGraphicsRegisterFlagsSurfaceLoadStore = 4, ///< HIP will bind this resource to a surface + 2, ///< HIP will only write and will not read from this registered resource, write only + hipGraphicsRegisterFlagsSurfaceLoadStore = 4, ///< HIP will bind this resource to a surface, read and write hipGraphicsRegisterFlagsTextureGather = - 8 ///< HIP will perform texture gather operations on this registered resource + 8 ///< HIP will perform texture gather operations on this registered resource, read and write or read only } hipGraphicsRegisterFlags; typedef struct _hipGraphicsResource hipGraphicsResource;