From d2e70bb99978a179343f1aef0fa87b01cdb2bd66 Mon Sep 17 00:00:00 2001 From: Sean Keely Date: Tue, 28 Nov 2017 22:10:07 -0600 Subject: [PATCH] Cleanup Signal interfaces for doorbells. Create an interface for doorbell signals to reduce code duplication. No functional changes. Change-Id: I101a8997dd582ff99e1537758c804b21fe3bb6af --- runtime/hsa-runtime/core/inc/amd_aql_queue.h | 151 +---------------- .../hsa-runtime/core/inc/intercept_queue.h | 148 +--------------- runtime/hsa-runtime/core/inc/signal.h | 160 ++++++++++++++++++ .../core/runtime/amd_aql_queue.cpp | 2 +- .../core/runtime/intercept_queue.cpp | 2 +- runtime/hsa-runtime/inc/hsa_ext_amd.h | 5 +- 6 files changed, 166 insertions(+), 302 deletions(-) diff --git a/runtime/hsa-runtime/core/inc/amd_aql_queue.h b/runtime/hsa-runtime/core/inc/amd_aql_queue.h index 104a0272cb..8126f9f996 100644 --- a/runtime/hsa-runtime/core/inc/amd_aql_queue.h +++ b/runtime/hsa-runtime/core/inc/amd_aql_queue.h @@ -53,7 +53,7 @@ namespace amd { /// @brief Encapsulates HW Aql Command Processor functionality. It /// provide the interface for things such as Doorbell register, read, /// write pointers and a buffer. -class AqlQueue : public core::Queue, private core::LocalSignal, public core::Signal { +class AqlQueue : public core::Queue, private core::LocalSignal, public core::DoorbellSignal { public: static __forceinline bool IsType(core::Signal* signal) { return signal->IsType(&rtti_id_); @@ -183,164 +183,15 @@ class AqlQueue : public core::Queue, private core::LocalSignal, public core::Sig // @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 This operation is illegal - hsa_signal_value_t LoadRelaxed() override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t LoadAcquire() override { - assert(false); - return 0; - } - /// @brief Update signal value using Relaxed semantics void StoreRelaxed(hsa_signal_value_t value) override; /// @brief Update signal value using Release semantics void StoreRelease(hsa_signal_value_t value) override; - /// @brief This operation is illegal - hsa_signal_value_t WaitRelaxed(hsa_signal_condition_t condition, hsa_signal_value_t compare_value, - uint64_t timeout, hsa_wait_state_t wait_hint) override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t WaitAcquire(hsa_signal_condition_t condition, hsa_signal_value_t compare_value, - uint64_t timeout, hsa_wait_state_t wait_hint) override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - void AndRelaxed(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void AndAcquire(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void AndRelease(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void AndAcqRel(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void OrRelaxed(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void OrAcquire(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void OrRelease(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void OrAcqRel(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void XorRelaxed(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void XorAcquire(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void XorRelease(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void XorAcqRel(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void AddRelaxed(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void AddAcquire(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void AddRelease(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void AddAcqRel(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void SubRelaxed(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void SubAcquire(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void SubRelease(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - void SubAcqRel(hsa_signal_value_t value) override { assert(false); } - - /// @brief This operation is illegal - hsa_signal_value_t ExchRelaxed(hsa_signal_value_t value) override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t ExchAcquire(hsa_signal_value_t value) override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t ExchRelease(hsa_signal_value_t value) override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t ExchAcqRel(hsa_signal_value_t value) override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t CasRelaxed(hsa_signal_value_t expected, hsa_signal_value_t value) override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t CasAcquire(hsa_signal_value_t expected, hsa_signal_value_t value) override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t CasRelease(hsa_signal_value_t expected, hsa_signal_value_t value) override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t CasAcqRel(hsa_signal_value_t expected, hsa_signal_value_t value) override { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t* ValueLocation() const override { - assert(false); - return NULL; - } - - /// @brief This operation is illegal - HsaEvent* EopEvent() override { - assert(false); - return NULL; - } - protected: bool _IsA(Queue::rtti_t id) const override { return id == &rtti_id_; } - /// @brief Disallow destroying doorbell apart from its queue. - void doDestroySignal() override { assert(false); } - private: uint32_t ComputeRingBufferMinPkts(); uint32_t ComputeRingBufferMaxPkts(); diff --git a/runtime/hsa-runtime/core/inc/intercept_queue.h b/runtime/hsa-runtime/core/inc/intercept_queue.h index 82ec222fe4..3f1cd956ef 100644 --- a/runtime/hsa-runtime/core/inc/intercept_queue.h +++ b/runtime/hsa-runtime/core/inc/intercept_queue.h @@ -183,7 +183,7 @@ class QueueProxy : public QueueWrapper { // @brief Provides packet intercept and rewrite capability for a queue. // Host-side dispatches are processed during doorbell ring. // Device-side dispatches are processed as an asynchronous signal event. -class InterceptQueue : public QueueProxy, private LocalSignal, public Signal { +class InterceptQueue : public QueueProxy, private LocalSignal, public DoorbellSignal { public: explicit InterceptQueue(std::unique_ptr queue); ~InterceptQueue(); @@ -250,152 +250,6 @@ class InterceptQueue : public QueueProxy, private LocalSignal, public Signal { StoreRelaxed(value); } - /// @brief This operation is illegal - hsa_signal_value_t LoadRelaxed() { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t LoadAcquire() { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t WaitRelaxed(hsa_signal_condition_t condition, hsa_signal_value_t compare_value, - uint64_t timeout, hsa_wait_state_t wait_hint) { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t WaitAcquire(hsa_signal_condition_t condition, hsa_signal_value_t compare_value, - uint64_t timeout, hsa_wait_state_t wait_hint) { - assert(false); - return 0; - } - - /// @brief This operation is illegal - void AndRelaxed(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void AndAcquire(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void AndRelease(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void AndAcqRel(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void OrRelaxed(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void OrAcquire(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void OrRelease(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void OrAcqRel(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void XorRelaxed(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void XorAcquire(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void XorRelease(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void XorAcqRel(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void AddRelaxed(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void AddAcquire(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void AddRelease(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void AddAcqRel(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void SubRelaxed(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void SubAcquire(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void SubRelease(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - void SubAcqRel(hsa_signal_value_t value) { assert(false); } - - /// @brief This operation is illegal - hsa_signal_value_t ExchRelaxed(hsa_signal_value_t value) { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t ExchAcquire(hsa_signal_value_t value) { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t ExchRelease(hsa_signal_value_t value) { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t ExchAcqRel(hsa_signal_value_t value) { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t CasRelaxed(hsa_signal_value_t expected, hsa_signal_value_t value) { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t CasAcquire(hsa_signal_value_t expected, hsa_signal_value_t value) { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t CasRelease(hsa_signal_value_t expected, hsa_signal_value_t value) { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t CasAcqRel(hsa_signal_value_t expected, hsa_signal_value_t value) { - assert(false); - return 0; - } - - /// @brief This operation is illegal - hsa_signal_value_t* ValueLocation() const { - assert(false); - return NULL; - } - - /// @brief This operation is illegal - HsaEvent* EopEvent() { - assert(false); - return NULL; - } - static __forceinline bool IsType(core::Signal* signal) { return signal->IsType(&rtti_id_); } static __forceinline bool IsType(core::Queue* queue) { return queue->IsType(&rtti_id_); } diff --git a/runtime/hsa-runtime/core/inc/signal.h b/runtime/hsa-runtime/core/inc/signal.h index ddc31ebd73..9c0d25618c 100644 --- a/runtime/hsa-runtime/core/inc/signal.h +++ b/runtime/hsa-runtime/core/inc/signal.h @@ -333,6 +333,166 @@ class Signal { DISALLOW_COPY_AND_ASSIGN(Signal); }; +/// @brief Handle signal operations which are not for use on doorbells. +class DoorbellSignal : public Signal { + public: + using Signal::Signal; + + /// @brief This operation is illegal + hsa_signal_value_t LoadRelaxed() final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t LoadAcquire() final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t WaitRelaxed(hsa_signal_condition_t condition, hsa_signal_value_t compare_value, + uint64_t timeout, hsa_wait_state_t wait_hint) final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t WaitAcquire(hsa_signal_condition_t condition, hsa_signal_value_t compare_value, + uint64_t timeout, hsa_wait_state_t wait_hint) final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + void AndRelaxed(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void AndAcquire(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void AndRelease(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void AndAcqRel(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void OrRelaxed(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void OrAcquire(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void OrRelease(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void OrAcqRel(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void XorRelaxed(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void XorAcquire(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void XorRelease(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void XorAcqRel(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void AddRelaxed(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void AddAcquire(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void AddRelease(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void AddAcqRel(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void SubRelaxed(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void SubAcquire(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void SubRelease(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + void SubAcqRel(hsa_signal_value_t value) final override { assert(false); } + + /// @brief This operation is illegal + hsa_signal_value_t ExchRelaxed(hsa_signal_value_t value) final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t ExchAcquire(hsa_signal_value_t value) final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t ExchRelease(hsa_signal_value_t value) final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t ExchAcqRel(hsa_signal_value_t value) final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t CasRelaxed(hsa_signal_value_t expected, + hsa_signal_value_t value) final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t CasAcquire(hsa_signal_value_t expected, + hsa_signal_value_t value) final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t CasRelease(hsa_signal_value_t expected, + hsa_signal_value_t value) final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t CasAcqRel(hsa_signal_value_t expected, + hsa_signal_value_t value) final override { + assert(false); + return 0; + } + + /// @brief This operation is illegal + hsa_signal_value_t* ValueLocation() const final override { + assert(false); + return NULL; + } + + /// @brief This operation is illegal + HsaEvent* EopEvent() final override { + assert(false); + return NULL; + } + + protected: + /// @brief Disallow destroying doorbell apart from its queue. + void doDestroySignal() final override { assert(false); } +}; + struct hsa_signal_handle { hsa_signal_t signal; diff --git a/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp b/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp index ab8fee67a1..0847a9f5d8 100644 --- a/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp +++ b/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp @@ -81,7 +81,7 @@ AqlQueue::AqlQueue(GpuAgent* agent, size_t req_size_pkts, HSAuint32 node_id, Scr core::HsaEventCallback callback, void* err_data, bool is_kv) : Queue(), LocalSignal(0), - Signal(signal()), + DoorbellSignal(signal()), ring_buf_(nullptr), ring_buf_alloc_bytes_(0), queue_id_(HSA_QUEUEID(-1)), diff --git a/runtime/hsa-runtime/core/runtime/intercept_queue.cpp b/runtime/hsa-runtime/core/runtime/intercept_queue.cpp index 0beaddab0d..4761a8f4f9 100644 --- a/runtime/hsa-runtime/core/runtime/intercept_queue.cpp +++ b/runtime/hsa-runtime/core/runtime/intercept_queue.cpp @@ -70,7 +70,7 @@ int InterceptQueue::rtti_id_ = 0; InterceptQueue::InterceptQueue(std::unique_ptr queue) : QueueProxy(std::move(queue)), LocalSignal(0), - Signal(signal()), + DoorbellSignal(signal()), next_packet_(0), retry_index_(0), quit_(false), diff --git a/runtime/hsa-runtime/inc/hsa_ext_amd.h b/runtime/hsa-runtime/inc/hsa_ext_amd.h index 78db49608f..78328b307a 100755 --- a/runtime/hsa-runtime/inc/hsa_ext_amd.h +++ b/runtime/hsa-runtime/inc/hsa_ext_amd.h @@ -299,9 +299,8 @@ hsa_status_t HSA_API * @brief Retrieve packet processing time stamps. * * @param[in] agent The agent with which the signal was last used. For - *instance, - * if the profiled dispatch packet is dispatched on to queue Q, which was - * created on agent A, then this parameter must be A. + * instance, if the profiled dispatch packet is dispatched onto queue Q, + * which was created on agent A, then this parameter must be A. * * @param[in] signal A signal used as the completion signal of the dispatch * packet to retrieve time stamps from. This dispatch packet must have been