From 1f53841421183bc623cd0775e5ae277f20e3a874 Mon Sep 17 00:00:00 2001 From: foreman Date: Tue, 20 Mar 2018 12:01:19 -0400 Subject: [PATCH] P4 to Git Change 1529531 by gandryey@gera-w8 on 2018/03/20 11:50:12 SWDEV-79445 - OCL generic changes and code clean-up - Fix a regression in the AMF test and reenable the suballoc optimization. Rearrange the locks around cache field access only to avoid calling memory release under the cache lock. Affected files ... ... //depot/stg/opencl/drivers/opencl/runtime/device/pal/palprogram.cpp#57 edit ... //depot/stg/opencl/drivers/opencl/runtime/device/pal/palresource.cpp#53 edit ... //depot/stg/opencl/drivers/opencl/runtime/device/pal/palresource.hpp#18 edit ... //depot/stg/opencl/drivers/opencl/runtime/utils/flags.hpp#287 edit --- rocclr/runtime/device/pal/palprogram.cpp | 2 +- rocclr/runtime/device/pal/palresource.cpp | 68 ++++++++++++----------- rocclr/runtime/device/pal/palresource.hpp | 7 ++- rocclr/runtime/utils/flags.hpp | 2 +- 4 files changed, 41 insertions(+), 38 deletions(-) diff --git a/rocclr/runtime/device/pal/palprogram.cpp b/rocclr/runtime/device/pal/palprogram.cpp index 9e9bb5d356..179b990bf2 100644 --- a/rocclr/runtime/device/pal/palprogram.cpp +++ b/rocclr/runtime/device/pal/palprogram.cpp @@ -86,12 +86,12 @@ void Segment::copy(size_t offset, const void* src, size_t size) { if (cpuAccess_ != nullptr) { amd::Os::fastMemcpy(cpuAddress(offset), src, size); } else { - amd::ScopedLock k(gpuAccess_->dev().xferMgr().lockXfer()); VirtualGPU& gpu = *gpuAccess_->dev().xferQueue(); Memory& xferBuf = gpuAccess_->dev().xferWrite().acquire(); size_t tmpSize = std::min(static_cast(xferBuf.size()), size); size_t srcOffs = 0; while (size != 0) { + amd::ScopedLock k(gpuAccess_->dev().xferMgr().lockXfer()); xferBuf.hostWrite(&gpu, reinterpret_cast(src) + srcOffs, 0, tmpSize); xferBuf.partialMemCopyTo(gpu, 0, (offset + srcOffs), tmpSize, *gpuAccess_, false, true); size -= tmpSize; diff --git a/rocclr/runtime/device/pal/palresource.cpp b/rocclr/runtime/device/pal/palresource.cpp index 09dee8b7fb..6f3f581ffa 100644 --- a/rocclr/runtime/device/pal/palresource.cpp +++ b/rocclr/runtime/device/pal/palresource.cpp @@ -561,7 +561,7 @@ bool Resource::CreateImage(CreateParams* params) desc_.pitch_ = rowPitch; // Make sure the row pitch is aligned to pixels imgCreateInfo.rowPitch = - elementSize() * amd::alignUp(rowPitch, dev().info().imagePitchAlignment_); + amd::alignUp(elementSize() * rowPitch, dev().info().imagePitchAlignment_); imgCreateInfo.depthPitch = imgCreateInfo.rowPitch * desc().height_; imgCreateInfo.tiling = tiling; @@ -1983,20 +1983,27 @@ GpuMemoryReference* MemorySubAllocator::Allocate( } // ================================================================================================ -bool MemorySubAllocator::Free(GpuMemoryReference* ref, Pal::gpusize offset) +bool MemorySubAllocator::Free(amd::Monitor* monitor, GpuMemoryReference* ref, Pal::gpusize offset) { - // Find if current memory reference is a chunk allocation - auto it = mem_heap_.find(ref); - if (it == mem_heap_.end()) { - return false; + bool releaseMem = false; + { + amd::ScopedLock l(monitor); + // Find if current memory reference is a chunk allocation + auto it = mem_heap_.find(ref); + if (it == mem_heap_.end()) { + return false; + } + // Free suballocation at the specified offset + it->second->Free(offset); + // If this suballocator empty, then release memory chunk + if (it->second->IsEmpty()) { + delete it->second; + mem_heap_.erase(it); + releaseMem = true; + } } - // Free suballocation at the specified offset - it->second->Free(offset); - // If this suballocator empty, then release memory chunk - if (it->second->IsEmpty()) { - delete it->second; - it->first->release(); - mem_heap_.erase(it); + if (releaseMem) { + ref->release(); } return true; } @@ -2013,9 +2020,8 @@ bool ResourceCache::addGpuMemory(Resource::Descriptor* desc, size_t size = ref->iMem()->Desc().size; if (desc->type_ == Resource::Local) { - amd::ScopedLock l(&lockCacheOps_); // Check if runtime can free suballocation in local memory - if (memSubAllocLocal_.Free(ref, offset)) { + if (memSubAllocLocal_.Free(&lockCacheOps_, ref, offset)) { return true; } } @@ -2024,16 +2030,17 @@ bool ResourceCache::addGpuMemory(Resource::Descriptor* desc, if (((desc->type_ == Resource::Local) || (desc->type_ == Resource::Persistent) || (desc->type_ == Resource::Remote) || (desc->type_ == Resource::RemoteUSWC)) && (size < cacheSizeLimit_) && !desc->SVMRes_) { - amd::ScopedLock l(&lockCacheOps_); // Validate the cache size limit. Loop until we have enough space while ((cacheSize_ + size) > cacheSizeLimit_) { removeLast(); } + Resource::Descriptor* descCached = new Resource::Descriptor; if (descCached != nullptr) { // Copy the original desc to the cached version memcpy(descCached, desc, sizeof(Resource::Descriptor)); + amd::ScopedLock l(&lockCacheOps_); // Add the current resource to the cache resCache_.push_front(std::make_pair(descCached, ref)); ref->gpu_ = nullptr; @@ -2085,37 +2092,32 @@ GpuMemoryReference* ResourceCache::findGpuMemory(Resource::Descriptor* desc, Pal } // ================================================================================================ -bool ResourceCache::free(size_t minCacheEntries) { - amd::ScopedLock l(&lockCacheOps_); - bool result = false; - +void ResourceCache::free(size_t minCacheEntries) { if (minCacheEntries < resCache_.size()) { - if (static_cast(cacheSize_) > 0) { - result = true; - } // Clear the cache while (static_cast(cacheSize_) > 0) { removeLast(); } CondLog((cacheSize_ != 0), "Incorrect size for cache release!"); } - return result; } // ================================================================================================ -void ResourceCache::removeLast() { +void ResourceCache::removeLast() +{ std::pair entry; - entry = resCache_.back(); - resCache_.pop_back(); - - size_t size = entry.second->iMem()->Desc().size; - - // Delete Descriptor - delete entry.first; + { + // Protect access to the global data + amd::ScopedLock l(&lockCacheOps_); + entry = resCache_.back(); + resCache_.pop_back(); + // Delete Descriptor + delete entry.first; + cacheSize_ -= entry.second->iMem()->Desc().size; + } // Destroy PAL resource entry.second->release(); - cacheSize_ -= size; } } // namespace pal diff --git a/rocclr/runtime/device/pal/palresource.hpp b/rocclr/runtime/device/pal/palresource.hpp index 3329ee077b..b7fd9ac341 100644 --- a/rocclr/runtime/device/pal/palresource.hpp +++ b/rocclr/runtime/device/pal/palresource.hpp @@ -258,7 +258,8 @@ class Resource : public amd::HeapObject { uint64_t vmAddress() const { return iMem()->Desc().gpuVirtAddr + offset_; } //! Returns global memory offset - uint64_t vmSize() const { return desc_.width_ * elementSize(); } + uint64_t vmSize() const + { return desc_.width_ * desc_.height_ * desc_.depth_ * elementSize(); } //! Returns global memory offset bool mipMapped() const { return (desc().mipLevels_ > 1) ? true : false; } @@ -484,7 +485,7 @@ public: GpuMemoryReference* Allocate(Pal::gpusize size, Pal::gpusize alignment, Pal::gpusize* offset); - bool Free(GpuMemoryReference* ref, Pal::gpusize offset); + bool Free(amd::Monitor* monitor, GpuMemoryReference* ref, Pal::gpusize offset); private: Device* device_; @@ -515,7 +516,7 @@ class ResourceCache : public amd::HeapObject { Pal::gpusize size, Pal::gpusize alignment, Pal::gpusize* offset); //! Destroys cache - bool free(size_t minCacheEntries = 0); + void free(size_t minCacheEntries = 0); private: //! Disable copy constructor diff --git a/rocclr/runtime/utils/flags.hpp b/rocclr/runtime/utils/flags.hpp index 9f68fe004a..eb29a04ed4 100644 --- a/rocclr/runtime/utils/flags.hpp +++ b/rocclr/runtime/utils/flags.hpp @@ -86,7 +86,7 @@ release(size_t, GPU_PINNED_MIN_XFER_SIZE, 512, \ "The minimal buffer size for pinned read/write transfers in KBytes") \ release(size_t, GPU_RESOURCE_CACHE_SIZE, 64, \ "The resource cache size in MB") \ -release(size_t, GPU_MAX_SUBALLOC_SIZE, 0, \ +release(size_t, GPU_MAX_SUBALLOC_SIZE, 4096, \ "The maximum size accepted for suballocaitons in KB") \ release(uint, GPU_ASYNC_MEM_COPY, 0, \ "Enables async memory transfers with DRM engine") \