From 3a4d69def2835b3499d228666f7db86e9be5172c Mon Sep 17 00:00:00 2001 From: German Andryeyev Date: Wed, 3 Mar 2021 17:28:58 -0500 Subject: [PATCH] SWDEV-272496 - Detect callbacks and force AQL barrier HIP tests require HIP callbacks to be processed in another thread. This change will use a thread from HSA signal callbacks to make sure a HIP callback was done asynchronously. Also process the callback before changing the status of command Change-Id: Icef85d0e0f808663882cf6881ff1be3e5eca29ac [ROCm/clr commit: 7f32d0b4256f00b35a0eec4fae74740492f50686] --- .../clr/rocclr/device/rocm/rocvirtual.cpp | 9 +++++++ projects/clr/rocclr/platform/command.cpp | 27 ++++++++++++++----- projects/clr/rocclr/platform/command.hpp | 3 +++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/projects/clr/rocclr/device/rocm/rocvirtual.cpp b/projects/clr/rocclr/device/rocm/rocvirtual.cpp index 2ff4c3483b..2b58a4715c 100644 --- a/projects/clr/rocclr/device/rocm/rocvirtual.cpp +++ b/projects/clr/rocclr/device/rocm/rocvirtual.cpp @@ -2853,9 +2853,18 @@ void VirtualGPU::flush(amd::Command* list, bool wait) { if (skip_cpu_wait) { // Search for the last command in the batch to track GPU state amd::Command* current = list; + // HIP tests expect callbacks processed from another thread, hence force AQL barrier always, so + // HSA signal callback will process HIP callback asynchronously + if (list->Callback() != nullptr) { + hasPendingDispatch_ = true; + } while (current->getNext() != nullptr) { current = current->getNext(); + if (current->Callback() != nullptr) { + hasPendingDispatch_ = true; + } } + // Enable profiling, so runtime can track TS profilingBegin(*current); diff --git a/projects/clr/rocclr/platform/command.cpp b/projects/clr/rocclr/platform/command.cpp index c5af772111..2f5cffa535 100644 --- a/projects/clr/rocclr/platform/command.cpp +++ b/projects/clr/rocclr/platform/command.cpp @@ -105,13 +105,26 @@ bool Event::setStatus(int32_t status, uint64_t timeStamp) { } } - if (!status_.compare_exchange_strong(currentStatus, status, std::memory_order_relaxed)) { - // Somebody else beat us to it, let them deal with the release/signal. - return false; - } - - if (callbacks_ != (CallBackEntry*)0) { - processCallbacks(status); + if (amd::IS_HIP) { + // HIP API doesn't have any event, associated with a callback. Hence the SW status of + // the event is irrelevant, during the actual callback. At the same time HIP API requires + // to finish the callback before HIP stream can continue. Hence runtime has to process + // the callback first and then update the status. + if (callbacks_ != (CallBackEntry*)0) { + processCallbacks(status); + } + if (!status_.compare_exchange_strong(currentStatus, status, std::memory_order_relaxed)) { + // Somebody else beat us to it, let them deal with the release/signal. + return false; + } + } else { + if (!status_.compare_exchange_strong(currentStatus, status, std::memory_order_relaxed)) { + // Somebody else beat us to it, let them deal with the release/signal. + return false; + } + if (callbacks_ != (CallBackEntry*)0) { + processCallbacks(status); + } } if (Agent::shouldPostEventEvents() && command().type() != 0) { diff --git a/projects/clr/rocclr/platform/command.hpp b/projects/clr/rocclr/platform/command.hpp index 4ae045c724..01ae5c6324 100644 --- a/projects/clr/rocclr/platform/command.hpp +++ b/projects/clr/rocclr/platform/command.hpp @@ -204,6 +204,9 @@ class Event : public RuntimeObject { //! RTTI internal implementation virtual ObjectType objectType() const { return ObjectTypeEvent; } + + //! Returns the callback for this event + const CallBackEntry* Callback() const { return callbacks_; } }; /*! \brief An operation that is submitted to a command queue.