From 6c304e4027a1350f3e71eac9e44f54c55135ac7e Mon Sep 17 00:00:00 2001 From: Saleel Kudchadker Date: Mon, 5 Apr 2021 15:23:07 -0700 Subject: [PATCH] SWDEV-276120 - Remove support for barrier sync ROC_BARRIER_SYNC will not work with direct dispatch. Remove and cleanup. Change-Id: I81368b2e65039477bd0343bb92708dab48867db6 [ROCm/clr commit: aa38af8c963b36afd679f90d21d568e292c76663] --- projects/clr/rocclr/device/device.hpp | 5 -- projects/clr/rocclr/device/rocm/rocblit.cpp | 47 ++++++------------- projects/clr/rocclr/device/rocm/rocdefs.hpp | 1 - projects/clr/rocclr/device/rocm/rocdevice.cpp | 12 ++--- .../clr/rocclr/device/rocm/rocsettings.cpp | 1 - .../clr/rocclr/device/rocm/rocsettings.hpp | 3 +- .../clr/rocclr/device/rocm/rocvirtual.cpp | 36 ++++---------- .../clr/rocclr/device/rocm/rocvirtual.hpp | 2 +- 8 files changed, 30 insertions(+), 77 deletions(-) diff --git a/projects/clr/rocclr/device/device.hpp b/projects/clr/rocclr/device/device.hpp index 8bfbba872b..5864cde3e5 100644 --- a/projects/clr/rocclr/device/device.hpp +++ b/projects/clr/rocclr/device/device.hpp @@ -1209,9 +1209,6 @@ class VirtualDevice : public amd::HeapObject { //! Returns true if device has active wait setting bool ActiveWait() const; - bool isLastCommandSDMA() const { return isLastCommandSDMA_; } - void setLastCommandSDMA(bool s) { isLastCommandSDMA_ = s; } - private: //! Disable default copy constructor VirtualDevice& operator=(const VirtualDevice&); @@ -1225,8 +1222,6 @@ class VirtualDevice : public amd::HeapObject { protected: device::BlitManager* blitMgr_; //!< Blit manager - //!< Keep track if the last command was SDMA and not send Barrier packets if barrier_sync is 0 - std::atomic_bool isLastCommandSDMA_; amd::Monitor execution_; //!< Lock to serialise access to all device objects uint index_; //!< The virtual device unique index }; diff --git a/projects/clr/rocclr/device/rocm/rocblit.cpp b/projects/clr/rocclr/device/rocm/rocblit.cpp index 39c8feab29..54a048d13c 100644 --- a/projects/clr/rocclr/device/rocm/rocblit.cpp +++ b/projects/clr/rocclr/device/rocm/rocblit.cpp @@ -63,7 +63,7 @@ bool DmaBlitManager::readBuffer(device::Memory& srcMemory, void* dstHost, const amd::Coord3D& origin, const amd::Coord3D& size, bool entire) const { // HSA copy functionality with a possible async operation - gpu().releaseGpuMemoryFence(kIgnoreBarrier, kSkipCpuWait); + gpu().releaseGpuMemoryFence(kSkipCpuWait); // Use host copy if memory has direct access if (setup_.disableReadBuffer_ || @@ -231,7 +231,7 @@ bool DmaBlitManager::writeBuffer(const void* srcHost, device::Memory& dstMemory, return HostBlitManager::writeBuffer(srcHost, dstMemory, origin, size, entire); } else { // HSA copy functionality with a possible async operation - gpu().releaseGpuMemoryFence(kIgnoreBarrier, kSkipCpuWait); + gpu().releaseGpuMemoryFence(kSkipCpuWait); size_t dstSize = size[0]; size_t tmpSize = 0; @@ -395,8 +395,7 @@ bool DmaBlitManager::copyBufferRect(device::Memory& srcMemory, device::Memory& d gpu().releaseGpuMemoryFence(); return HostBlitManager::copyBufferRect(srcMemory, dstMemory, srcRect, dstRect, size, entire); } else { - bool force_barrier = !dev().settings().barrier_sync_ && !gpu().isLastCommandSDMA(); - gpu().releaseGpuMemoryFence(force_barrier, kSkipCpuWait); + gpu().releaseGpuMemoryFence(kSkipCpuWait); void* src = gpuMem(srcMemory).getDeviceMemory(); void* dst = gpuMem(dstMemory).getDeviceMemory(); @@ -491,8 +490,6 @@ bool DmaBlitManager::copyBufferRect(device::Memory& srcMemory, device::Memory& d gpu().Barriers().ResetCurrentSignal(); LogPrintfError("DMA buffer failed with code %d", status); return false; - } else { - gpu().setLastCommandSDMA(true); } } } @@ -508,8 +505,7 @@ bool DmaBlitManager::copyImageToBuffer(device::Memory& srcMemory, device::Memory const amd::Coord3D& size, bool entire, size_t rowPitch, size_t slicePitch) const { // HSA copy functionality with a possible async operation, hence make sure GPU is done - bool force_barrier = !dev().settings().barrier_sync_ && !gpu().isLastCommandSDMA(); - gpu().releaseGpuMemoryFence(force_barrier); + gpu().releaseGpuMemoryFence(); bool result = false; @@ -553,8 +549,7 @@ bool DmaBlitManager::copyBufferToImage(device::Memory& srcMemory, device::Memory const amd::Coord3D& size, bool entire, size_t rowPitch, size_t slicePitch) const { // HSA copy functionality with a possible async operation, hence make sure GPU is done - bool force_barrier = !dev().settings().barrier_sync_ && !gpu().isLastCommandSDMA(); - gpu().releaseGpuMemoryFence(force_barrier); + gpu().releaseGpuMemoryFence(); bool result = false; @@ -619,8 +614,7 @@ bool DmaBlitManager::hsaCopy(const Memory& srcMemory, const Memory& dstMemory, address src = reinterpret_cast
(srcMemory.getDeviceMemory()); address dst = reinterpret_cast
(dstMemory.getDeviceMemory()); - bool force_barrier = !dev().settings().barrier_sync_ && !gpu().isLastCommandSDMA(); - gpu().releaseGpuMemoryFence(force_barrier, kSkipCpuWait); + gpu().releaseGpuMemoryFence(kSkipCpuWait); src += srcOrigin[0]; dst += dstOrigin[0]; @@ -681,7 +675,6 @@ bool DmaBlitManager::hsaCopy(const Memory& srcMemory, const Memory& dstMemory, status = hsa_amd_memory_async_copy(dst, dstAgent, src, srcAgent, size[0], num_wait_events, wait_event, active); if (status == HSA_STATUS_SUCCESS) { - gpu().setLastCommandSDMA(true); gpu().addSystemScope(); } else { gpu().Barriers().ResetCurrentSignal(); @@ -695,8 +688,7 @@ bool DmaBlitManager::hsaCopy(const Memory& srcMemory, const Memory& dstMemory, bool DmaBlitManager::hsaCopyStaged(const_address hostSrc, address hostDst, size_t size, address staging, bool hostToDev) const { // Stall GPU, sicne CPU copy is possible - bool force_barrier = !dev().settings().barrier_sync_ && !gpu().isLastCommandSDMA(); - gpu().releaseGpuMemoryFence(force_barrier); + gpu().releaseGpuMemoryFence(); // No allocation is necessary for Full Profile hsa_status_t status; @@ -742,8 +734,6 @@ bool DmaBlitManager::hsaCopyStaged(const_address hostSrc, address hostDst, size_ gpu().Barriers().ResetCurrentSignal(); LogPrintfError("Hsa copy from host to device failed with code %d", status); return false; - } else { - gpu().setLastCommandSDMA(true); } gpu().Barriers().WaitCurrent(); totalSize -= size; @@ -771,7 +761,6 @@ bool DmaBlitManager::hsaCopyStaged(const_address hostSrc, address hostDst, size_ status = hsa_amd_memory_async_copy(hsaBuffer, dstAgent, hostSrc + offset, dev().getBackendDevice(), size, 0, nullptr, active); if (status == HSA_STATUS_SUCCESS) { - gpu().setLastCommandSDMA(true); gpu().Barriers().WaitCurrent(); memcpy(hostDst + offset, hsaBuffer, size); } else { @@ -1118,8 +1107,7 @@ bool KernelBlitManager::copyBufferToImageKernel(device::Memory& srcMemory, releaseArguments(parameters); if (releaseView) { // todo SRD programming could be changed to avoid a stall - bool force_barrier = !dev().settings().barrier_sync_ && !gpu().isLastCommandSDMA(); - gpu().releaseGpuMemoryFence(force_barrier); + gpu().releaseGpuMemoryFence(); dstView->owner()->release(); } @@ -1317,8 +1305,7 @@ bool KernelBlitManager::copyImageToBufferKernel(device::Memory& srcMemory, releaseArguments(parameters); if (releaseView) { // todo SRD programming could be changed to avoid a stall - bool force_barrier = !dev().settings().barrier_sync_ && !gpu().isLastCommandSDMA(); - gpu().releaseGpuMemoryFence(force_barrier); + gpu().releaseGpuMemoryFence(); srcView->owner()->release(); } @@ -1449,8 +1436,7 @@ bool KernelBlitManager::copyImage(device::Memory& srcMemory, device::Memory& dst releaseArguments(parameters); if (releaseView) { // todo SRD programming could be changed to avoid a stall - bool force_barrier = !dev().settings().barrier_sync_ && !gpu().isLastCommandSDMA(); - gpu().releaseGpuMemoryFence(force_barrier); + gpu().releaseGpuMemoryFence(); srcView->owner()->release(); dstView->owner()->release(); } @@ -1717,8 +1703,7 @@ bool KernelBlitManager::readBuffer(device::Memory& srcMemory, void* dstHost, if ((srcMemory.owner()->getHostMem() == nullptr) && (srcMemory.owner()->getSvmPtr() != nullptr)) { // CPU read ahead, hence release GPU memory and force barrier to make sure L2 flush - constexpr bool ForceBarrier = true; - gpu().releaseGpuMemoryFence(ForceBarrier); + gpu().releaseGpuMemoryFence(); char* src = reinterpret_cast(srcMemory.owner()->getSvmPtr()); std::memcpy(dstHost, src + origin[0], size[0]); // Force HDP Read cache invalidation somewhere in the AQL barrier flags... @@ -1838,13 +1823,12 @@ bool KernelBlitManager::writeBuffer(const void* srcHost, device::Memory& dstMemo if (dev().info().largeBar_ && size[0] <= kMaxH2dMemcpySize) { if ((dstMemory.owner()->getHostMem() == nullptr) && (dstMemory.owner()->getSvmPtr() != nullptr)) { // CPU read ahead, hence release GPU memory - constexpr bool ForceBarrier = true; - gpu().releaseGpuMemoryFence(ForceBarrier); + gpu().releaseGpuMemoryFence(); char* dst = reinterpret_cast(dstMemory.owner()->getSvmPtr()); std::memcpy(dst + origin[0], srcHost, size[0]); - // Set HASPENDINGDISPATCH_ FLAG. Then releaseGpuMemoryFence() will use barrier to invalidate cache + // Set hasPendingDispatch_ flag. Then releaseGpuMemoryFence() will use barrier to invalidate cache gpu().hasPendingDispatch(); - gpu().releaseGpuMemoryFence(ForceBarrier); + gpu().releaseGpuMemoryFence(); return true; } } @@ -2273,8 +2257,7 @@ bool KernelBlitManager::fillImage(device::Memory& memory, const void* pattern, releaseArguments(parameters); if (releaseView) { // todo SRD programming could be changed to avoid a stall - bool force_barrier = !dev().settings().barrier_sync_ && !gpu().isLastCommandSDMA(); - gpu().releaseGpuMemoryFence(force_barrier); + gpu().releaseGpuMemoryFence(); memView->owner()->release(); } diff --git a/projects/clr/rocclr/device/rocm/rocdefs.hpp b/projects/clr/rocclr/device/rocm/rocdefs.hpp index 44827de45d..8e9712fb62 100644 --- a/projects/clr/rocclr/device/rocm/rocdefs.hpp +++ b/projects/clr/rocclr/device/rocm/rocdefs.hpp @@ -34,7 +34,6 @@ static constexpr uint DeviceQueueMaskSize = 32; static constexpr uint kMaxAsyncQueues = 8; constexpr bool kSkipCpuWait = true; -constexpr bool kIgnoreBarrier = false; enum HwQueueEngine : uint32_t { Compute = 0, diff --git a/projects/clr/rocclr/device/rocm/rocdevice.cpp b/projects/clr/rocclr/device/rocm/rocdevice.cpp index b26dc68562..ca2f976efa 100644 --- a/projects/clr/rocclr/device/rocm/rocdevice.cpp +++ b/projects/clr/rocclr/device/rocm/rocdevice.cpp @@ -1606,11 +1606,6 @@ device::VirtualDevice* Device::createVirtualDevice(amd::CommandQueue* queue) { cooperative = amd::IS_HIP && settings().enableCoopGroups_; profiling = amd::IS_HIP; } - // If barrier is disabled, then profiling should be enabled to make sure HSA signal is - // attached for every dispatch - else if (!settings().barrier_sync_) { - queue->properties().set(CL_QUEUE_PROFILING_ENABLE); - } // Initialization of heap and other resources occur during the command // queue creation time. const std::vector defaultCuMask = {}; @@ -1872,7 +1867,7 @@ void* Device::hostAlloc(size_t size, size_t alignment, MemorySegment mem_seg) co } case kNoAtomics : // If runtime disables barrier, then all host allocations must have L2 disabled - if ((settings().barrier_sync_) && (system_coarse_segment_.handle != 0)) { + if (system_coarse_segment_.handle != 0) { segment = system_coarse_segment_; break; } @@ -1908,10 +1903,9 @@ void* Device::hostAgentAlloc(size_t size, const AgentInfo& agentInfo, bool atomi void* ptr = nullptr; const hsa_amd_memory_pool_t segment = // If runtime disables barrier, then all host allocations must have L2 disabled - (!atomics && settings().barrier_sync_) ? - (agentInfo.coarse_grain_pool.handle != 0) ? + !atomics ? (agentInfo.coarse_grain_pool.handle != 0) ? agentInfo.coarse_grain_pool : agentInfo.fine_grain_pool - : agentInfo.fine_grain_pool; + : agentInfo.fine_grain_pool; assert(segment.handle != 0); hsa_status_t stat = hsa_amd_memory_pool_allocate(segment, size, 0, &ptr); ClPrint(amd::LOG_DEBUG, amd::LOG_MEM, "Allocate hsa host memory %p, size 0x%zx", ptr, size); diff --git a/projects/clr/rocclr/device/rocm/rocsettings.cpp b/projects/clr/rocclr/device/rocm/rocsettings.cpp index b2c0cc772d..0396b8a811 100644 --- a/projects/clr/rocclr/device/rocm/rocsettings.cpp +++ b/projects/clr/rocclr/device/rocm/rocsettings.cpp @@ -91,7 +91,6 @@ Settings::Settings() { hmmFlags_ = (!flagIsDefault(ROC_HMM_FLAGS)) ? ROC_HMM_FLAGS : Hmm::EnableSvmTracking; rocr_backend_ = true; - barrier_sync_ = (!flagIsDefault(ROC_BARRIER_SYNC)) ? ROC_BARRIER_SYNC : true; cpu_wait_for_signal_ = !AMD_DIRECT_DISPATCH; cpu_wait_for_signal_ = (!flagIsDefault(ROC_CPU_WAIT_FOR_SIGNAL)) ? diff --git a/projects/clr/rocclr/device/rocm/rocsettings.hpp b/projects/clr/rocclr/device/rocm/rocsettings.hpp index 5abace8b85..96a80b7e8c 100644 --- a/projects/clr/rocclr/device/rocm/rocsettings.hpp +++ b/projects/clr/rocclr/device/rocm/rocsettings.hpp @@ -51,11 +51,10 @@ class Settings : public device::Settings { uint stagedXferRead_ : 1; //!< Uses a staged buffer read uint stagedXferWrite_ : 1; //!< Uses a staged buffer write uint imageBufferWar_ : 1; //!< Image buffer workaround for Gfx10 - uint barrier_sync_ : 1; //!< Use AQL barrier command to sync with CPU uint cpu_wait_for_signal_ : 1; //!< Wait for HSA signal on CPU uint system_scope_signal_ : 1; //!< HSA signal is visibile to the entire system uint skip_copy_sync_ : 1; //!< Ignore explicit HSA signal waits for copy functionality - uint reserved_ : 20; + uint reserved_ : 21; }; uint value_; }; diff --git a/projects/clr/rocclr/device/rocm/rocvirtual.cpp b/projects/clr/rocclr/device/rocm/rocvirtual.cpp index 6a4e8ef9b2..048ccf35fa 100644 --- a/projects/clr/rocclr/device/rocm/rocvirtual.cpp +++ b/projects/clr/rocclr/device/rocm/rocvirtual.cpp @@ -864,16 +864,15 @@ void VirtualGPU::ResetQueueStates() { // Release all memory dependencies memoryDependency().clear(); - if (dev().settings().barrier_sync_) { - // Release the pool, since runtime just completed a barrier - // @note: Runtime can reset kernel arg pool only if the barrier with L2 invalidation was issued - resetKernArgPool(); - } + // Release the pool, since runtime just completed a barrier + // @note: Runtime can reset kernel arg pool only if the barrier with L2 invalidation was issued + resetKernArgPool(); + } // ================================================================================================ -bool VirtualGPU::releaseGpuMemoryFence(bool force_barrier, bool skip_cpu_wait) { - if (hasPendingDispatch_ && (dev().settings().barrier_sync_ || force_barrier)) { +bool VirtualGPU::releaseGpuMemoryFence(bool skip_cpu_wait) { + if (hasPendingDispatch_) { // Dispatch barrier packet into the queue dispatchBarrierPacket(&barrier_packet_, kBarrierPacketHeader); hasPendingDispatch_ = false; @@ -2461,7 +2460,6 @@ bool VirtualGPU::submitKernelInternal(const amd::NDRangeContainer& sizes, const bool imageBufferWrtBack = false; // Image buffer write back is required std::vector wrtBackImageBuffer; // Array of images for write back - setLastCommandSDMA(false); // Check memory dependency and SVM objects bool coopGroups = (vcmd != nullptr) ? vcmd->cooperativeGroups() : false; if (!processMemObjects(kernel, parameters, ldsUsage, coopGroups, @@ -2740,8 +2738,7 @@ bool VirtualGPU::submitKernelInternal(const amd::NDRangeContainer& sizes, const void VirtualGPU::submitKernel(amd::NDRangeKernelCommand& vcmd) { if (vcmd.cooperativeGroups() || vcmd.cooperativeMultiDeviceGroups()) { // Wait for the execution on the current queue, since the coop groups will use the device queue - bool force_barrier = !dev().settings().barrier_sync_ && !isLastCommandSDMA(); - releaseGpuMemoryFence(force_barrier, kSkipCpuWait); + releaseGpuMemoryFence(kSkipCpuWait); // Get device queue for exclusive GPU access VirtualGPU* queue = dev().xferQueue(); @@ -2781,7 +2778,7 @@ void VirtualGPU::submitKernel(amd::NDRangeKernelCommand& vcmd) { vcmd.setStatus(CL_INVALID_OPERATION); } // Wait for the execution on the device queue. Keep the current queue in-order - queue->releaseGpuMemoryFence(kIgnoreBarrier, kSkipCpuWait); + queue->releaseGpuMemoryFence(kSkipCpuWait); // Add a dependency into the current queue on the coop queue Barriers().SetExternalSignal(queue->Barriers().GetLastSignal()); @@ -2844,11 +2841,6 @@ void VirtualGPU::submitReleaseExtObjects(amd::ReleaseExtObjectsCommand& vcmd) { // Make sure VirtualGPU has an exclusive access to the resources amd::ScopedLock lock(execution()); profilingBegin(vcmd); - if (!dev().settings().barrier_sync_) { - // Force barrier to make sure L2 flush, since interop can be in sysmem - constexpr bool kForceBarrier = true; - releaseGpuMemoryFence(kForceBarrier); - } profilingEnd(vcmd); } @@ -2856,7 +2848,6 @@ void VirtualGPU::submitReleaseExtObjects(amd::ReleaseExtObjectsCommand& vcmd) { void VirtualGPU::flush(amd::Command* list, bool wait) { // Direct dispatch relies on HSA signal callback bool skip_cpu_wait = AMD_DIRECT_DISPATCH; - bool force_barrier = !dev().settings().barrier_sync_ && !isLastCommandSDMA(); if (skip_cpu_wait) { // Search for the last command in the batch to track GPU state @@ -2885,25 +2876,18 @@ void VirtualGPU::flush(amd::Command* list, bool wait) { // then the host thread can't update it also, otherwise double free may occur skip_cpu_wait &= hasPendingDispatch_; - releaseGpuMemoryFence(force_barrier, skip_cpu_wait); + releaseGpuMemoryFence(skip_cpu_wait); profilingEnd(*current); } else { // If barrier is requested, then wait for everything, otherwise // a per disaptch wait will occur later in updateCommandsState() - if (dev().settings().barrier_sync_) { - releaseGpuMemoryFence(); - } + releaseGpuMemoryFence(); } // If CPU waited for GPU, then the queue is idle if (!skip_cpu_wait) { updateCommandsState(list); - // Add extra clean up for resources if releaseGpuMemoryFence() was skipped - if (!dev().settings().barrier_sync_) { - ResetQueueStates(); - } - // Release all pinned memory releasePinnedMem(); } diff --git a/projects/clr/rocclr/device/rocm/rocvirtual.hpp b/projects/clr/rocclr/device/rocm/rocvirtual.hpp index d8b2a98a8b..3799713e1b 100644 --- a/projects/clr/rocclr/device/rocm/rocvirtual.hpp +++ b/projects/clr/rocclr/device/rocm/rocvirtual.hpp @@ -290,7 +290,7 @@ class VirtualGPU : public device::VirtualDevice { * * @return bool true if Wait returned successfully, false otherwise */ - bool releaseGpuMemoryFence(bool force_barrier = false, bool skip_copy_wait = false); + bool releaseGpuMemoryFence(bool skip_copy_wait = false); hsa_agent_t gpu_device() const { return gpu_device_; } hsa_queue_t* gpu_queue() { return gpu_queue_; }