From a030ebfe49c9e87dc5e8b71aef6b5f1987d6d182 Mon Sep 17 00:00:00 2001 From: foreman Date: Thu, 8 Sep 2016 20:34:24 -0400 Subject: [PATCH] P4 to Git Change 1312587 by cpaquot@hog-ocl on 2016/09/08 19:46:03 SWDEV-96354 - Wrong usage of hsaImageData_ and deviceMemory_. Use hsaImageData_ as the original pointer before alignment and only for that purpose. The deviceMemory_ is where the data is located. No one ever needs to use hsaImageData_ really. This is only an issue with tiled images ReviewBoardURL = http://ocltc.amd.com/reviews/r/11331/diff/ Affected files ... ... //depot/stg/opencl/drivers/opencl/runtime/device/rocm/rocdevice.cpp#14 edit ... //depot/stg/opencl/drivers/opencl/runtime/device/rocm/rocdevice.hpp#6 edit ... //depot/stg/opencl/drivers/opencl/runtime/device/rocm/rocmemory.cpp#4 edit ... //depot/stg/opencl/drivers/opencl/runtime/device/rocm/rocmemory.hpp#4 edit [ROCm/clr commit: 51948f577c3d8fea2d650559978fde8a0b6b6c50] --- .../rocclr/runtime/device/rocm/rocdevice.cpp | 11 ++--- .../rocclr/runtime/device/rocm/rocdevice.hpp | 2 +- .../rocclr/runtime/device/rocm/rocmemory.cpp | 49 +++++++++---------- .../rocclr/runtime/device/rocm/rocmemory.hpp | 2 +- 4 files changed, 29 insertions(+), 35 deletions(-) diff --git a/projects/clr/rocclr/runtime/device/rocm/rocdevice.cpp b/projects/clr/rocclr/runtime/device/rocm/rocdevice.cpp index c01413ba18..a95d632526 100644 --- a/projects/clr/rocclr/runtime/device/rocm/rocdevice.cpp +++ b/projects/clr/rocclr/runtime/device/rocm/rocdevice.cpp @@ -1246,12 +1246,7 @@ Device::hostAlloc(size_t size, size_t alignment, bool atomics) const { void Device::hostFree(void* ptr, size_t size) const { - hsa_status_t stat = - hsa_amd_memory_pool_free(ptr); - if (stat != HSA_STATUS_SUCCESS) { - LogError("Fail freeing host memory"); - assert(stat == HSA_STATUS_SUCCESS); - } + memFree(ptr, size); } void * @@ -1272,7 +1267,7 @@ Device::deviceLocalAlloc(size_t size) const stat = hsa_memory_assign_agent(ptr, _bkendDevice, HSA_ACCESS_PERMISSION_RW); if (stat != HSA_STATUS_SUCCESS) { LogError("Fail assigning local memory to agent"); - deviceLocalFree(ptr, size); + memFree(ptr, size); return NULL; } @@ -1280,7 +1275,7 @@ Device::deviceLocalAlloc(size_t size) const } void -Device::deviceLocalFree(void *ptr, size_t size) const +Device::memFree(void *ptr, size_t size) const { hsa_status_t stat = hsa_amd_memory_pool_free(ptr); diff --git a/projects/clr/rocclr/runtime/device/rocm/rocdevice.hpp b/projects/clr/rocclr/runtime/device/rocm/rocdevice.hpp index 96b0e29339..3aff6e1458 100644 --- a/projects/clr/rocclr/runtime/device/rocm/rocdevice.hpp +++ b/projects/clr/rocclr/runtime/device/rocm/rocdevice.hpp @@ -320,7 +320,7 @@ public: void *deviceLocalAlloc(size_t size) const; - void deviceLocalFree(void *ptr, size_t size) const; + void memFree(void *ptr, size_t size) const; virtual void* svmAlloc(amd::Context& context, size_t size, size_t alignment, cl_svm_mem_flags flags = CL_MEM_READ_WRITE, void* svmPtr = NULL) const; diff --git a/projects/clr/rocclr/runtime/device/rocm/rocmemory.cpp b/projects/clr/rocclr/runtime/device/rocm/rocmemory.cpp index e1eb87602e..588208ae47 100644 --- a/projects/clr/rocclr/runtime/device/rocm/rocmemory.cpp +++ b/projects/clr/rocclr/runtime/device/rocm/rocmemory.cpp @@ -274,7 +274,7 @@ Buffer::destroy() } } else { - dev_.deviceLocalFree(deviceMemory_, size()); + dev_.memFree(deviceMemory_, size()); } } @@ -359,7 +359,7 @@ Buffer::create() amd::Coord3D(size()), true); if (!ret) { - dev_.deviceLocalFree(deviceMemory_, size()); + dev_.memFree(deviceMemory_, size()); deviceMemory_ = NULL; } @@ -484,7 +484,7 @@ Image::Image(const roc::Device& dev, amd::Memory& owner) : flags_ &= (~HostMemoryDirectAccess & ~HostMemoryRegistered); populateImageDescriptor(); hsaImageObject_.handle = 0; - hsaImageData_ = NULL; + originalDeviceMemory_ = NULL; } void @@ -594,9 +594,9 @@ Image::createInteropImage() if (obj->getGLTarget()==GL_TEXTURE_CUBE_MAP) desc.setFace(obj->getCubemapFace()); - hsaImageData_=deviceMemory_; + originalDeviceMemory_=deviceMemory_; - hsa_status_t err=hsa_amd_image_create(dev_.getBackendDevice(), &imageDescriptor_, amdImageDesc_, hsaImageData_, permission_, &hsaImageObject_); + hsa_status_t err=hsa_amd_image_create(dev_.getBackendDevice(), &imageDescriptor_, amdImageDesc_, originalDeviceMemory_, permission_, &hsaImageObject_); if(err!=HSA_STATUS_SUCCESS) return false; @@ -644,23 +644,23 @@ Image::create() : deviceImageInfo_.size + deviceImageInfo_.alignment; if (!(owner()->getMemFlags() & CL_MEM_ALLOC_HOST_PTR)) { - deviceMemory_ = dev_.deviceLocalAlloc(alloc_size); + originalDeviceMemory_ = dev_.deviceLocalAlloc(alloc_size); } - if (deviceMemory_ == NULL) { - deviceMemory_ = + if (originalDeviceMemory_ == NULL) { + originalDeviceMemory_ = dev_.hostAlloc(alloc_size, 1, false); } - hsaImageData_ = reinterpret_cast( - amd::alignUp(reinterpret_cast(deviceMemory_), + deviceMemory_ = reinterpret_cast( + amd::alignUp(reinterpret_cast(originalDeviceMemory_), deviceImageInfo_.alignment)); assert(amd::isMultipleOf( - hsaImageData_, static_cast(deviceImageInfo_.alignment))); + deviceMemory_, static_cast(deviceImageInfo_.alignment))); status = hsa_ext_image_create( - dev_.getBackendDevice(), &imageDescriptor_, hsaImageData_, + dev_.getBackendDevice(), &imageDescriptor_, deviceMemory_, permission_, &hsaImageObject_); if (status != HSA_STATUS_SUCCESS) { @@ -676,17 +676,17 @@ Image::createView(Memory &parent) { deviceMemory_ = parent.getDeviceMemory(); - hsaImageData_ = (parent.owner()->asBuffer() != NULL) + originalDeviceMemory_ = (parent.owner()->asBuffer() != NULL) ? deviceMemory_ - : static_cast(parent).hsaImageData_; + : static_cast(parent).originalDeviceMemory_; kind_=parent.getKind(); hsa_status_t status; if(kind_==MEMORY_KIND_INTEROP) - status = hsa_amd_image_create(dev_.getBackendDevice(), &imageDescriptor_, amdImageDesc_, hsaImageData_, permission_, &hsaImageObject_); + status = hsa_amd_image_create(dev_.getBackendDevice(), &imageDescriptor_, amdImageDesc_, deviceMemory_, permission_, &hsaImageObject_); else - status= hsa_ext_image_create(dev_.getBackendDevice(), &imageDescriptor_, hsaImageData_, permission_, &hsaImageObject_); + status= hsa_ext_image_create(dev_.getBackendDevice(), &imageDescriptor_, deviceMemory_, permission_, &hsaImageObject_); if (status != HSA_STATUS_SUCCESS) { LogError("[OCL] Fail to allocate image memory"); @@ -756,27 +756,26 @@ Image::~Image() void Image::destroy() { + if (hsaImageObject_.handle != 0) { + hsa_status_t status = + hsa_ext_image_destroy(dev_.getBackendDevice(), hsaImageObject_); + assert(status == HSA_STATUS_SUCCESS); + } + if (owner()->parent() != NULL) { return; } if(kind_==MEMORY_KIND_INTEROP) { - hsa_ext_image_destroy(dev_.getBackendDevice(), hsaImageObject_); free(amdImageDesc_); amdImageDesc_=NULL; destroyInteropBuffer(); return; } - if (deviceMemory_ != NULL) { - dev_.hostFree(deviceMemory_, deviceImageInfo_.size); - } - - if (hsaImageObject_.handle != 0) { - hsa_status_t status = - hsa_ext_image_destroy(dev_.getBackendDevice(), hsaImageObject_); - assert(status == HSA_STATUS_SUCCESS); + if (originalDeviceMemory_ != NULL) { + dev_.memFree(originalDeviceMemory_, deviceImageInfo_.size); } } } diff --git a/projects/clr/rocclr/runtime/device/rocm/rocmemory.hpp b/projects/clr/rocclr/runtime/device/rocm/rocmemory.hpp index a561ad3f86..c2d77f6201 100644 --- a/projects/clr/rocclr/runtime/device/rocm/rocmemory.hpp +++ b/projects/clr/rocclr/runtime/device/rocm/rocmemory.hpp @@ -180,7 +180,7 @@ private: hsa_ext_image_t hsaImageObject_; hsa_amd_image_descriptor_t* amdImageDesc_; - const void* hsaImageData_; + void* originalDeviceMemory_; }; }