From dc9bce3b9b22ae38b2c8a01d1975e2028a05cace Mon Sep 17 00:00:00 2001 From: David Yat Sin Date: Wed, 15 Nov 2023 16:14:36 +0000 Subject: [PATCH] Force t1_ update when profiling is enabled Fixes issue where t1_ counters may not be updated when doing dispatch profiling, causing a divide by 0. Change-Id: I91060ac3f9fd2183d277e6e7cd810398a453a87f [ROCm/ROCR-Runtime commit: 3d1563ee682893d2f8077620cb15c03097614eda] --- .../runtime/hsa-runtime/core/inc/amd_aql_queue.h | 5 ++++- .../runtime/hsa-runtime/core/inc/amd_gpu_agent.h | 7 +++++++ .../runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp | 7 +++++++ .../runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp | 6 +----- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_aql_queue.h b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_aql_queue.h index 156958d2b0..ddb8671c2d 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_aql_queue.h +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_aql_queue.h @@ -196,9 +196,12 @@ class AqlQueue : public core::Queue, private core::LocalSignal, public core::Doo /// @return hsa_status_t hsa_status_t GetCUMasking(uint32_t num_cu_mask_count, uint32_t* cu_mask) override; - // @brief Submits a block of PM4 and waits until it has been executed. + /// @brief Submits a block of PM4 and waits until it has been executed. void ExecutePM4(uint32_t* cmd_data, size_t cmd_size_b) override; + /// @brief Enables/Disables profiling overrides SetProfiling from core::Queue + void SetProfiling(bool enabled) override; + /// @brief Update signal value using Relaxed semantics void StoreRelaxed(hsa_signal_value_t value) override; diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_gpu_agent.h b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_gpu_agent.h index 49977229f3..cac4096760 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_gpu_agent.h +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_gpu_agent.h @@ -373,6 +373,13 @@ class GpuAgent : public GpuAgentInt { // @brief returns the libdrm device handle __forceinline amdgpu_device_handle libDrmDev() const { return ldrm_dev_; } + __forceinline void CheckClockTicks() { + // If we did not update t1 since agent initialization, force a SyncClock. Otherwise computing + // the SystemClockCounter to GPUClockCounter ratio in TranslateTime(tick) results to a division + // by 0. + if (t0_.GPUClockCounter == t1_.GPUClockCounter) SyncClocks(); + } + void ReserveScratch(); // @brief If agent supports it, release scratch memory for all AQL queues on this agent. diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp index 211535ab10..e8b8f3e80f 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp @@ -1339,6 +1339,13 @@ hsa_status_t AqlQueue::GetCUMasking(uint32_t num_cu_mask_count, uint32_t* cu_mas return HSA_STATUS_SUCCESS; } +void AqlQueue::SetProfiling(bool enabled) { + Queue::SetProfiling(enabled); + + if (enabled) agent_->CheckClockTicks(); + return; +} + void AqlQueue::ExecutePM4(uint32_t* cmd_data, size_t cmd_size_b) { // pm4_ib_buf_ is a shared resource, so mutually exclude here. ScopedAcquire lock(&pm4_ib_mutex_); diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp index 47d5ceaa01..a77cd65ed1 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp @@ -1191,11 +1191,7 @@ hsa_status_t GpuAgent::EnableDmaProfiling(bool enable) { } } - // If we did not update t1 since agent initialization, force a SyncClock. Otherwise computing - // the SystemClockCounter to GPUClockCounter ratio in TranslateTime(tick) results to a division - // by 0. We perform the check here because we do not want to check everytime there is a call to - // TranslateTime(tick) - if (enable && t0_.GPUClockCounter == t1_.GPUClockCounter) SyncClocks(); + if (enable) CheckClockTicks(); return HSA_STATUS_SUCCESS; }