From ccc1476e8724ab1b99507aaee4baded8acdaca39 Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Fri, 21 Oct 2022 20:26:07 -0700 Subject: [PATCH 01/12] Acquire/lock resources per thread --- src/runtime/hexagon/hexagon_htp.cc | 4 +- src/runtime/hexagon/hexagon_htp.h | 6 +- src/runtime/hexagon/hexagon_hvx.cc | 23 +++--- src/runtime/hexagon/hexagon_hvx.h | 10 +++ src/runtime/hexagon/hexagon_thread_manager.cc | 81 ++++++++++++++++--- src/runtime/hexagon/hexagon_thread_manager.h | 13 ++- .../hexagon/hexagon_thread_manager_tests.cc | 13 +-- 7 files changed, 115 insertions(+), 35 deletions(-) diff --git a/src/runtime/hexagon/hexagon_htp.cc b/src/runtime/hexagon/hexagon_htp.cc index 01344ccf4a79..b98a59e9f64c 100644 --- a/src/runtime/hexagon/hexagon_htp.cc +++ b/src/runtime/hexagon/hexagon_htp.cc @@ -35,9 +35,9 @@ namespace tvm { namespace runtime { namespace hexagon { -HexagonHtp::HexagonHtp() { Acquire(); } +HexagonHtp::HexagonHtp() {} -HexagonHtp::~HexagonHtp() { Release(); } +HexagonHtp::~HexagonHtp() {} void HexagonHtp::Acquire() { compute_res_attr_t compute_res_attr; diff --git a/src/runtime/hexagon/hexagon_htp.h b/src/runtime/hexagon/hexagon_htp.h index b3f0c0b5f71f..4048c9037395 100644 --- a/src/runtime/hexagon/hexagon_htp.h +++ b/src/runtime/hexagon/hexagon_htp.h @@ -44,12 +44,12 @@ class HexagonHtp { //! \brief Prevent move assignment. HexagonHtp& operator=(HexagonHtp&&) = delete; + void Acquire(); + void Release(); + private: //! \brief Acquisition context ID unsigned int context_id_; - - void Acquire(); - void Release(); }; } // namespace hexagon diff --git a/src/runtime/hexagon/hexagon_hvx.cc b/src/runtime/hexagon/hexagon_hvx.cc index 0c3160a7d89b..13ced85e4236 100644 --- a/src/runtime/hexagon/hexagon_hvx.cc +++ b/src/runtime/hexagon/hexagon_hvx.cc @@ -32,24 +32,27 @@ namespace runtime { namespace hexagon { HexagonHvx::HexagonHvx() { - // Reserve HVX. - int res = qurt_hvx_reserve(QURT_HVX_RESERVE_ALL_AVAILABLE); - CHECK((res != QURT_HVX_RESERVE_NOT_SUPPORTED) && (res != QURT_HVX_RESERVE_NOT_SUCCESSFUL)) - << "error reserving HVX: " << res; + reserved_count_ = qurt_hvx_reserve(QURT_HVX_RESERVE_ALL); + CHECK((reserved_count_ == QURT_HVX_RESERVE_ALL) || + (reserved_count_ == QURT_HVX_RESERVE_ALREADY_MADE)) + << "error reserving HVX: " << reserved_count_; +} + +HexagonHvx::~HexagonHvx() { + // Release HVX. + int rel = qurt_hvx_cancel_reserve(); + CHECK(rel == 0) << "error releasing HVX: " << rel; +} - // Lock HVX. +void HexagonHvx::Lock() { int lck = qurt_hvx_lock(QURT_HVX_MODE_128B); CHECK(lck == 0) << "error locking HVX: " << lck; } -HexagonHvx::~HexagonHvx() { +void HexagonHvx::Unlock() { // Unlock HVX. int unl = qurt_hvx_unlock(); CHECK(unl == 0) << "error unlocking HVX: " << unl; - - // Release HVX. - int rel = qurt_hvx_cancel_reserve(); - CHECK(rel == 0) << "error releasing HVX: " << rel; } } // namespace hexagon diff --git a/src/runtime/hexagon/hexagon_hvx.h b/src/runtime/hexagon/hexagon_hvx.h index 042977981c99..c5a281191079 100644 --- a/src/runtime/hexagon/hexagon_hvx.h +++ b/src/runtime/hexagon/hexagon_hvx.h @@ -45,7 +45,17 @@ class HexagonHvx { //! \brief Prevent move assignment. HexagonHvx& operator=(HexagonHvx&&) = delete; + //! \brief Lock one HVX to the calling thread. + void Lock(); + + //! \brief Unlock the HVX for the calling thread. + void Unlock(); + + //! \brief Number of HVX units reservered. + int ReservedCount() { return reserved_count_; } + private: + int reserved_count_; }; } // namespace hexagon diff --git a/src/runtime/hexagon/hexagon_thread_manager.cc b/src/runtime/hexagon/hexagon_thread_manager.cc index cf64cdc8b2d0..09d4e8f6af7d 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.cc +++ b/src/runtime/hexagon/hexagon_thread_manager.cc @@ -38,14 +38,8 @@ HexagonThreadManager::HexagonThreadManager(unsigned num_threads, unsigned thread CHECK_GE(thread_pipe_size_words, MIN_PIPE_SIZE_WORDS); CHECK_LE(thread_pipe_size_words, MAX_PIPE_SIZE_WORDS); - // Support either no resources or a specific set of hardware resources for now. - if (!hw_resources.empty()) { - CHECK((hw_resources.size() == nthreads_) && (nthreads_ == 6) && (hw_resources[0] == DMA_0) && - (hw_resources[1] == HTP_0) && (hw_resources[2] == HVX_0) && (hw_resources[3] == HVX_1) && - (hw_resources[4] == HVX_2) && (hw_resources[5] == HVX_3)) - << "Unsupported hardware resource set"; - } hw_resources_ = hw_resources; + CheckResources(); if (!hw_resources_.empty()) { DLOG(INFO) << "Initialize hardware resource managers"; @@ -74,9 +68,9 @@ HexagonThreadManager::~HexagonThreadManager() { // dispatch a command to each thread to exit with status 0 for (unsigned i = 0; i < nthreads_; i++) { - bool success = Dispatch(reinterpret_cast(i), thread_exit, nullptr); + bool success = Dispatch(reinterpret_cast(i), thread_exit, contexts_[i]); while (!success) { - success = Dispatch(reinterpret_cast(i), thread_exit, nullptr); + success = Dispatch(reinterpret_cast(i), thread_exit, contexts_[i]); } } @@ -121,6 +115,22 @@ HexagonThreadManager::~HexagonThreadManager() { DLOG(INFO) << "Hardware resources released"; } +void HexagonThreadManager::CheckResources() { + CHECK(hw_resources_.empty() || hw_resources_.size() == nthreads_) + << "Thread count must match resource count"; + if (!hw_resources_.empty()) { + // Ensure that no more than one of each hardware resource is specified + for (int i = 0; i < hw_resources_.size(); i++) { + if (hw_resources_[i] != NONE) { + for (int j = i + 1; j < hw_resources_.size(); j++) { + CHECK(hw_resources_[i] != hw_resources_[j]) + << "No more than one of each resource type may be specified " << hw_resources_[i]; + } + } + } + } +} + void HexagonThreadManager::SpawnThreads(unsigned thread_stack_size_bytes, unsigned thread_pipe_size_words) { // allocate all stack space for threads @@ -168,7 +178,8 @@ void HexagonThreadManager::SpawnThreads(unsigned thread_stack_size_bytes, next_stack_start += thread_stack_size_bytes; // create the thread - contexts_[i] = new ThreadContext(&pipes_[i], i); + contexts_[i] = new ThreadContext(&pipes_[i], i, hw_resources_.empty() ? NONE : hw_resources_[i], + hvx_.get(), htp_.get()); int rc = qurt_thread_create(&threads_[i], &thread_attr, thread_main, contexts_[i]); CHECK_EQ(rc, QURT_EOK); } @@ -284,18 +295,62 @@ void HexagonThreadManager::thread_wait_free(void* semaphore) { free(semaphore); } -void HexagonThreadManager::thread_exit(void* status) { - DLOG(INFO) << "thread exiting"; - qurt_thread_exit((uint64_t)status); +void HexagonThreadManager::thread_exit(void* context) { + ThreadContext* tc = static_cast(context); + unsigned index = tc->index; + HardwareResourceType resource_type = tc->resource_type; + + if ((resource_type == HVX_0) || (resource_type == HVX_1) || (resource_type == HVX_2) || + (resource_type == HVX_3)) { + CHECK(tc->hvx) << "Malformed thread context, missing hvx pointer for HVX_x resource"; + tc->hvx->Unlock(); + DLOG(INFO) << "Resource " << resource_type << " unlocked an HVX instance"; + } else if (resource_type == HTP_0) { + CHECK(tc->htp) << "Malformed thread context, missing htp pointer for HTP_0 resource"; + tc->htp->Release(); + DLOG(INFO) << "Resource " << resource_type << " released the HTP"; + } + + DLOG(INFO) << "Thread " << index << " exiting"; + qurt_thread_exit((uint64_t)tc->status); } void HexagonThreadManager::thread_main(void* context) { ThreadContext* tc = static_cast(context); unsigned index = tc->index; qurt_pipe_t* mypipe = tc->pipe; + HardwareResourceType resource_type = tc->resource_type; DLOG(INFO) << "Thread " << index << " spawned"; + switch (resource_type) { + case NONE: + DLOG(INFO) << "No hardware resource"; + break; + + case DMA_0: + DLOG(INFO) << "Resource " << resource_type << " assigned to DMA"; + break; + + case HTP_0: + CHECK(tc->htp) << "Malformed thread context, missing htp pointer for HTP_0 resource"; + tc->htp->Acquire(); + DLOG(INFO) << "Resource " << resource_type << " acquired the HTP"; + break; + + case HVX_0: + case HVX_1: + case HVX_2: + case HVX_3: + CHECK(tc->hvx) << "Malformed thread context, missing hvx pointer for HVX_x resource"; + tc->hvx->Lock(); + DLOG(INFO) << "Resource " << resource_type << " locked an HVX instance"; + break; + + default: + CHECK(false) << "Invalid HardwareResourceType: " << resource_type; + } + while (true) { // loop, executing commands from pipe DLOG(INFO) << "Thread " << index << " receiving command"; qurt_pipe_data_t msg = qurt_pipe_receive(mypipe); // blocks if empty diff --git a/src/runtime/hexagon/hexagon_thread_manager.h b/src/runtime/hexagon/hexagon_thread_manager.h index a263cf42dc58..cafa153ffeff 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.h +++ b/src/runtime/hexagon/hexagon_thread_manager.h @@ -137,9 +137,18 @@ class HexagonThreadManager { struct ThreadContext { qurt_pipe_t* pipe; unsigned index; - ThreadContext(qurt_pipe_t* pipe, unsigned index) : pipe(pipe), index(index) {} + HardwareResourceType resource_type; + HexagonHvx* hvx; + HexagonHtp* htp; + uint64_t status; + ThreadContext(qurt_pipe_t* pipe, unsigned index, HardwareResourceType resource_type, + HexagonHvx* hvx, HexagonHtp* htp) + : pipe(pipe), index(index), resource_type(resource_type), hvx(hvx), htp(htp), status(0) {} }; + //! \brief Helper function to ensure the set of requested resources is valid. + void CheckResources(); + //! \brief Helper function for the constructor to spawn threads. void SpawnThreads(unsigned thread_stack_size_bytes, unsigned thread_pipe_size_words); @@ -157,7 +166,7 @@ class HexagonThreadManager { static void thread_wait_free(void* semaphore); //! \brief Void function executed by a thread to exit at time of destruction. - static void thread_exit(void* status); + static void thread_exit(void* context); //! \brief Void function executed by each thread as `main`. static void thread_main(void* context); diff --git a/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc b/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc index d7bf0afed906..f9a66f3089c0 100644 --- a/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc @@ -42,7 +42,7 @@ class HexagonThreadManagerTest : public ::testing::Test { const unsigned stack_size{0x4000}; // 16KB }; -TEST_F(HexagonThreadManagerTest, ctor_errors) { +TEST_F(HexagonThreadManagerTest, ctor_edge_cases) { // zero threads ASSERT_THROW(HexagonThreadManager(0, stack_size, pipe_size), InternalError); // too many threads @@ -57,13 +57,16 @@ TEST_F(HexagonThreadManagerTest, ctor_errors) { ASSERT_THROW(HexagonThreadManager(6, stack_size, 0x10000000), InternalError); // hw resources count doesn't match thread count ASSERT_THROW(HexagonThreadManager(6, stack_size, pipe_size, {DMA_0}), InternalError); - // hw resources doesn't match specific supported configuration + // no more than one of each hw resource may be specified + ASSERT_THROW(HexagonThreadManager(4, stack_size, pipe_size, {DMA_0, HTP_0, HVX_0, HVX_0}), + InternalError); + // no more than one of each hw resource may be specified ASSERT_THROW( HexagonThreadManager(6, stack_size, pipe_size, {DMA_0, HTP_0, HVX_0, HVX_1, HVX_2, DMA_0}), InternalError); - // hw resources doesn't match specific supported configuration - ASSERT_THROW(HexagonThreadManager(5, stack_size, pipe_size, {DMA_0, HTP_0, HVX_0, HVX_1, HVX_2}), - InternalError); + // multiple entries for no resource is allowed. + HexagonThreadManager* htm_none = new HexagonThreadManager(2, stack_size, pipe_size, {NONE, NONE}); + delete htm_none; } TEST_F(HexagonThreadManagerTest, init) { From bf811f852ebbfa1d613b890c85b64fd8d506350e Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Fri, 21 Oct 2022 20:45:50 -0700 Subject: [PATCH 02/12] Get threads by types and types by threads --- src/runtime/hexagon/hexagon_thread_manager.cc | 13 +++++++++++++ src/runtime/hexagon/hexagon_thread_manager.h | 12 ++++++++++++ .../hexagon/hexagon_device_api_tests.cc | 19 +++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/src/runtime/hexagon/hexagon_thread_manager.cc b/src/runtime/hexagon/hexagon_thread_manager.cc index 09d4e8f6af7d..9b45759d7d8d 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.cc +++ b/src/runtime/hexagon/hexagon_thread_manager.cc @@ -196,6 +196,19 @@ const std::vector HexagonThreadManager::GetStreamHandles() { return out; } +TVMStreamHandle HexagonThreadManager::GetStreamHandleByResourceType(HardwareResourceType type) { + for (unsigned i = 0; i < hw_resources_.size(); i++) { + if (hw_resources_[i] == type) { + return reinterpret_cast(i); + } + } + CHECK(false) << "Thread for resource type " << type << " not found"; +} + +HardwareResourceType HexagonThreadManager::GetResourceTypeForStreamHandle(TVMStreamHandle thread) { + return hw_resources_[reinterpret_cast(thread)]; +} + bool HexagonThreadManager::Dispatch(TVMStreamHandle stream, voidfunc f, void* args) { unsigned thread = reinterpret_cast(stream); DLOG(INFO) << "Dispatching to stream " << thread; diff --git a/src/runtime/hexagon/hexagon_thread_manager.h b/src/runtime/hexagon/hexagon_thread_manager.h index cafa153ffeff..90a60a2ae0ba 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.h +++ b/src/runtime/hexagon/hexagon_thread_manager.h @@ -87,6 +87,18 @@ class HexagonThreadManager { */ const std::vector GetStreamHandles(); + /*! + * \brief Get the spawned threads as stream handles for a resource type. + * \returns stream handle. + */ + TVMStreamHandle GetStreamHandleByResourceType(HardwareResourceType type); + + /*! + * \brief Get the resource type for a stream handle + * \returns stream handle. + */ + HardwareResourceType GetResourceTypeForStreamHandle(TVMStreamHandle thread); + /*! * \brief Non-blocking dispatch of a void function and args on a given thread. * \param thread Stream handle of the thread on which to dispatch the void function. diff --git a/tests/cpp-runtime/hexagon/hexagon_device_api_tests.cc b/tests/cpp-runtime/hexagon/hexagon_device_api_tests.cc index 0d193042a950..197c4296a9f6 100644 --- a/tests/cpp-runtime/hexagon/hexagon_device_api_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_device_api_tests.cc @@ -190,3 +190,22 @@ TEST_F(HexagonDeviceAPITest, vtcm_pool) { EXPECT_THROW(hexapi->VtcmPool(), InternalError); hexapi->AcquireResources(); } + +// Validate threads created for hw resources +TEST_F(HexagonDeviceAPITest, threads_for_resource_types) { + HexagonThreadManager* thread_manager = hexapi->ThreadManager(); + TVMStreamHandle thread; + + thread = thread_manager->GetStreamHandleByResourceType(DMA_0); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == DMA_0); + thread = thread_manager->GetStreamHandleByResourceType(HTP_0); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HTP_0); + thread = thread_manager->GetStreamHandleByResourceType(HVX_0); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_0); + thread = thread_manager->GetStreamHandleByResourceType(HVX_1); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_1); + thread = thread_manager->GetStreamHandleByResourceType(HVX_2); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_2); + thread = thread_manager->GetStreamHandleByResourceType(HVX_3); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_3); +} From d1188c2414c8cbecf2efb1654e7d5fc606232a00 Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Sat, 22 Oct 2022 08:16:42 -0700 Subject: [PATCH 03/12] Test acquire HTP and lock/unlock HVX --- src/runtime/hexagon/hexagon_hvx.cc | 2 - .../hexagon/hexagon_device_api_tests.cc | 19 ---------- .../hexagon/hexagon_thread_manager_tests.cc | 37 +++++++++++++++++++ 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/runtime/hexagon/hexagon_hvx.cc b/src/runtime/hexagon/hexagon_hvx.cc index 13ced85e4236..afa64d189037 100644 --- a/src/runtime/hexagon/hexagon_hvx.cc +++ b/src/runtime/hexagon/hexagon_hvx.cc @@ -39,7 +39,6 @@ HexagonHvx::HexagonHvx() { } HexagonHvx::~HexagonHvx() { - // Release HVX. int rel = qurt_hvx_cancel_reserve(); CHECK(rel == 0) << "error releasing HVX: " << rel; } @@ -50,7 +49,6 @@ void HexagonHvx::Lock() { } void HexagonHvx::Unlock() { - // Unlock HVX. int unl = qurt_hvx_unlock(); CHECK(unl == 0) << "error unlocking HVX: " << unl; } diff --git a/tests/cpp-runtime/hexagon/hexagon_device_api_tests.cc b/tests/cpp-runtime/hexagon/hexagon_device_api_tests.cc index 197c4296a9f6..0d193042a950 100644 --- a/tests/cpp-runtime/hexagon/hexagon_device_api_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_device_api_tests.cc @@ -190,22 +190,3 @@ TEST_F(HexagonDeviceAPITest, vtcm_pool) { EXPECT_THROW(hexapi->VtcmPool(), InternalError); hexapi->AcquireResources(); } - -// Validate threads created for hw resources -TEST_F(HexagonDeviceAPITest, threads_for_resource_types) { - HexagonThreadManager* thread_manager = hexapi->ThreadManager(); - TVMStreamHandle thread; - - thread = thread_manager->GetStreamHandleByResourceType(DMA_0); - CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == DMA_0); - thread = thread_manager->GetStreamHandleByResourceType(HTP_0); - CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HTP_0); - thread = thread_manager->GetStreamHandleByResourceType(HVX_0); - CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_0); - thread = thread_manager->GetStreamHandleByResourceType(HVX_1); - CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_1); - thread = thread_manager->GetStreamHandleByResourceType(HVX_2); - CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_2); - thread = thread_manager->GetStreamHandleByResourceType(HVX_3); - CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_3); -} diff --git a/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc b/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc index f9a66f3089c0..6d0b2b7e295c 100644 --- a/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc @@ -337,3 +337,40 @@ TEST_F(HexagonThreadManagerTest, dispatch_writes) { CHECK_EQ(array[i], truth[i]); } } + +// Validate threads created for hw resources on global manager +TEST_F(HexagonThreadManagerTest, threads_for_resource_types) { + HexagonThreadManager* thread_manager = HexagonDeviceAPI::Global()->ThreadManager(); + TVMStreamHandle thread; + + thread = thread_manager->GetStreamHandleByResourceType(DMA_0); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == DMA_0); + thread = thread_manager->GetStreamHandleByResourceType(HTP_0); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HTP_0); + thread = thread_manager->GetStreamHandleByResourceType(HVX_0); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_0); + thread = thread_manager->GetStreamHandleByResourceType(HVX_1); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_1); + thread = thread_manager->GetStreamHandleByResourceType(HVX_2); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_2); + thread = thread_manager->GetStreamHandleByResourceType(HVX_3); + CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_3); +} + +// Ensure proper behavior of hardware resources managed by global thread manager +TEST_F(HexagonThreadManagerTest, hardware_resources_locked) { + CHECK(htm != nullptr); + CHECK_EQ(streams.size(), threads); + HexagonHvx* hvx = new HexagonHvx(); + HexagonHtp* htp = new HexagonHtp(); + + // These should succeed + hvx->Lock(); + hvx->Unlock(); + + // This should throw + EXPECT_THROW(htp->Acquire(), InternalError); + + delete hvx; + delete htp; +} From 715c3f8671bad54338c1d699fc6774232c6c8ba1 Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Sat, 22 Oct 2022 08:33:38 -0700 Subject: [PATCH 04/12] Cleanup --- src/runtime/hexagon/hexagon_thread_manager.cc | 39 +++++-------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/src/runtime/hexagon/hexagon_thread_manager.cc b/src/runtime/hexagon/hexagon_thread_manager.cc index 9b45759d7d8d..f73a33d4e373 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.cc +++ b/src/runtime/hexagon/hexagon_thread_manager.cc @@ -315,13 +315,11 @@ void HexagonThreadManager::thread_exit(void* context) { if ((resource_type == HVX_0) || (resource_type == HVX_1) || (resource_type == HVX_2) || (resource_type == HVX_3)) { - CHECK(tc->hvx) << "Malformed thread context, missing hvx pointer for HVX_x resource"; tc->hvx->Unlock(); - DLOG(INFO) << "Resource " << resource_type << " unlocked an HVX instance"; + DLOG(INFO) << "Thread " << index << " unlocked an HVX instance"; } else if (resource_type == HTP_0) { - CHECK(tc->htp) << "Malformed thread context, missing htp pointer for HTP_0 resource"; tc->htp->Release(); - DLOG(INFO) << "Resource " << resource_type << " released the HTP"; + DLOG(INFO) << "Thread " << index << " released the HTP"; } DLOG(INFO) << "Thread " << index << " exiting"; @@ -336,32 +334,13 @@ void HexagonThreadManager::thread_main(void* context) { DLOG(INFO) << "Thread " << index << " spawned"; - switch (resource_type) { - case NONE: - DLOG(INFO) << "No hardware resource"; - break; - - case DMA_0: - DLOG(INFO) << "Resource " << resource_type << " assigned to DMA"; - break; - - case HTP_0: - CHECK(tc->htp) << "Malformed thread context, missing htp pointer for HTP_0 resource"; - tc->htp->Acquire(); - DLOG(INFO) << "Resource " << resource_type << " acquired the HTP"; - break; - - case HVX_0: - case HVX_1: - case HVX_2: - case HVX_3: - CHECK(tc->hvx) << "Malformed thread context, missing hvx pointer for HVX_x resource"; - tc->hvx->Lock(); - DLOG(INFO) << "Resource " << resource_type << " locked an HVX instance"; - break; - - default: - CHECK(false) << "Invalid HardwareResourceType: " << resource_type; + if ((resource_type == HVX_0) || (resource_type == HVX_1) || (resource_type == HVX_2) || + (resource_type == HVX_3)) { + tc->hvx->Lock(); + DLOG(INFO) << "Thread " << index << " locked an HVX instance"; + } else if (resource_type == HTP_0) { + tc->htp->Acquire(); + DLOG(INFO) << "Thread " << index << " acquired the HTP"; } while (true) { // loop, executing commands from pipe From f5abfc3e6a244b92307f26a29efd5e2baa1b912e Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Sat, 22 Oct 2022 08:34:21 -0700 Subject: [PATCH 05/12] clean up --- src/runtime/hexagon/hexagon_thread_manager.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/runtime/hexagon/hexagon_thread_manager.h b/src/runtime/hexagon/hexagon_thread_manager.h index 90a60a2ae0ba..649b1d32be8e 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.h +++ b/src/runtime/hexagon/hexagon_thread_manager.h @@ -155,7 +155,11 @@ class HexagonThreadManager { uint64_t status; ThreadContext(qurt_pipe_t* pipe, unsigned index, HardwareResourceType resource_type, HexagonHvx* hvx, HexagonHtp* htp) - : pipe(pipe), index(index), resource_type(resource_type), hvx(hvx), htp(htp), status(0) {} + : pipe(pipe), index(index), resource_type(resource_type), hvx(hvx), htp(htp), status(0) { + CHECK(resource_type == NONE || (hvx && htp)) + << "Missing resource manager pointer, type: " << resource_type << " hvx: " << hvx + << " htp: " << htp; + } }; //! \brief Helper function to ensure the set of requested resources is valid. From 24abd18da8c7b28b4d67789f1e8902f815d945ed Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Sat, 22 Oct 2022 08:51:53 -0700 Subject: [PATCH 06/12] Make hvx and htp interfaces uniform --- src/runtime/hexagon/hexagon_htp.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/runtime/hexagon/hexagon_htp.cc b/src/runtime/hexagon/hexagon_htp.cc index b98a59e9f64c..f6c1d2f01ffb 100644 --- a/src/runtime/hexagon/hexagon_htp.cc +++ b/src/runtime/hexagon/hexagon_htp.cc @@ -35,9 +35,9 @@ namespace tvm { namespace runtime { namespace hexagon { -HexagonHtp::HexagonHtp() {} +HexagonHtp::HexagonHtp() { Acquire(); } -HexagonHtp::~HexagonHtp() {} +HexagonHtp::~HexagonHtp() { Release(); } void HexagonHtp::Acquire() { compute_res_attr_t compute_res_attr; @@ -54,15 +54,19 @@ void HexagonHtp::Acquire() { if (!context_id_) { LOG(FATAL) << "InternalError: HAP_compute_res_acquire failed\n"; } +} + +void HexagonHtp::Release() { HAP_compute_res_release((unsigned int)context_id_); } + +void HexagonHtp::Lock() { + int nErr; + if ((nErr = HAP_compute_res_hmx_lock(context_id_))) { LOG(FATAL) << "InternalError: Unable to lock HTP!"; } } -void HexagonHtp::Release() { - HAP_compute_res_hmx_unlock((unsigned int)context_id_); - HAP_compute_res_release((unsigned int)context_id_); -} +void HexagonHtp::Unlock() { HAP_compute_res_hmx_unlock((unsigned int)context_id_); } } // namespace hexagon } // namespace runtime From f4ee7a584e16f0253f1178efd8a6371ee3ef0b43 Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Sat, 22 Oct 2022 08:52:36 -0700 Subject: [PATCH 07/12] Make hvx and htp interfaces uniform --- src/runtime/hexagon/hexagon_htp.h | 7 +++++-- src/runtime/hexagon/hexagon_hvx.cc | 8 ++++++-- src/runtime/hexagon/hexagon_hvx.h | 3 +++ src/runtime/hexagon/hexagon_thread_manager.cc | 12 +++++++----- src/runtime/hexagon/hexagon_thread_manager.h | 3 +++ .../hexagon/hexagon_thread_manager_tests.cc | 14 ++++---------- 6 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/runtime/hexagon/hexagon_htp.h b/src/runtime/hexagon/hexagon_htp.h index 4048c9037395..928936133dd8 100644 --- a/src/runtime/hexagon/hexagon_htp.h +++ b/src/runtime/hexagon/hexagon_htp.h @@ -44,12 +44,15 @@ class HexagonHtp { //! \brief Prevent move assignment. HexagonHtp& operator=(HexagonHtp&&) = delete; - void Acquire(); - void Release(); + void Lock(); + void Unlock(); private: //! \brief Acquisition context ID unsigned int context_id_; + + void Acquire(); + void Release(); }; } // namespace hexagon diff --git a/src/runtime/hexagon/hexagon_hvx.cc b/src/runtime/hexagon/hexagon_hvx.cc index afa64d189037..e90a0c801828 100644 --- a/src/runtime/hexagon/hexagon_hvx.cc +++ b/src/runtime/hexagon/hexagon_hvx.cc @@ -31,14 +31,18 @@ namespace tvm { namespace runtime { namespace hexagon { -HexagonHvx::HexagonHvx() { +HexagonHvx::HexagonHvx() { Acquire(); } + +HexagonHvx::~HexagonHvx() { Release(); } + +void HexagonHvx::Acquire() { reserved_count_ = qurt_hvx_reserve(QURT_HVX_RESERVE_ALL); CHECK((reserved_count_ == QURT_HVX_RESERVE_ALL) || (reserved_count_ == QURT_HVX_RESERVE_ALREADY_MADE)) << "error reserving HVX: " << reserved_count_; } -HexagonHvx::~HexagonHvx() { +void HexagonHvx::Release() { int rel = qurt_hvx_cancel_reserve(); CHECK(rel == 0) << "error releasing HVX: " << rel; } diff --git a/src/runtime/hexagon/hexagon_hvx.h b/src/runtime/hexagon/hexagon_hvx.h index c5a281191079..b09cf2611833 100644 --- a/src/runtime/hexagon/hexagon_hvx.h +++ b/src/runtime/hexagon/hexagon_hvx.h @@ -56,6 +56,9 @@ class HexagonHvx { private: int reserved_count_; + + void Acquire(); + void Release(); }; } // namespace hexagon diff --git a/src/runtime/hexagon/hexagon_thread_manager.cc b/src/runtime/hexagon/hexagon_thread_manager.cc index f73a33d4e373..f5ee9cea8dda 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.cc +++ b/src/runtime/hexagon/hexagon_thread_manager.cc @@ -41,7 +41,7 @@ HexagonThreadManager::HexagonThreadManager(unsigned num_threads, unsigned thread hw_resources_ = hw_resources; CheckResources(); - if (!hw_resources_.empty()) { + if (create_resource_managers_) { DLOG(INFO) << "Initialize hardware resource managers"; // Acquisition/locks will be performed on specific threads htp_ = std::make_unique(); @@ -116,12 +116,14 @@ HexagonThreadManager::~HexagonThreadManager() { } void HexagonThreadManager::CheckResources() { + create_resource_managers_ = false; CHECK(hw_resources_.empty() || hw_resources_.size() == nthreads_) << "Thread count must match resource count"; if (!hw_resources_.empty()) { // Ensure that no more than one of each hardware resource is specified for (int i = 0; i < hw_resources_.size(); i++) { if (hw_resources_[i] != NONE) { + create_resource_managers_ = true; for (int j = i + 1; j < hw_resources_.size(); j++) { CHECK(hw_resources_[i] != hw_resources_[j]) << "No more than one of each resource type may be specified " << hw_resources_[i]; @@ -318,8 +320,8 @@ void HexagonThreadManager::thread_exit(void* context) { tc->hvx->Unlock(); DLOG(INFO) << "Thread " << index << " unlocked an HVX instance"; } else if (resource_type == HTP_0) { - tc->htp->Release(); - DLOG(INFO) << "Thread " << index << " released the HTP"; + tc->htp->Unlock(); + DLOG(INFO) << "Thread " << index << " unlocked the HTP"; } DLOG(INFO) << "Thread " << index << " exiting"; @@ -339,8 +341,8 @@ void HexagonThreadManager::thread_main(void* context) { tc->hvx->Lock(); DLOG(INFO) << "Thread " << index << " locked an HVX instance"; } else if (resource_type == HTP_0) { - tc->htp->Acquire(); - DLOG(INFO) << "Thread " << index << " acquired the HTP"; + tc->htp->Lock(); + DLOG(INFO) << "Thread " << index << " locked the HTP"; } while (true) { // loop, executing commands from pipe diff --git a/src/runtime/hexagon/hexagon_thread_manager.h b/src/runtime/hexagon/hexagon_thread_manager.h index 649b1d32be8e..c911d1326a39 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.h +++ b/src/runtime/hexagon/hexagon_thread_manager.h @@ -228,6 +228,9 @@ class HexagonThreadManager { //! \brief List of hardware resources std::vector hw_resources_; + //! \brief Whether or not resource managers should be created + bool create_resource_managers_{false}; + //! \brief HTP hardware resource. // TODO(HWE): Move binding of HTP to a specific thread std::unique_ptr htp_; diff --git a/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc b/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc index 6d0b2b7e295c..85e7edc8806e 100644 --- a/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc @@ -359,18 +359,12 @@ TEST_F(HexagonThreadManagerTest, threads_for_resource_types) { // Ensure proper behavior of hardware resources managed by global thread manager TEST_F(HexagonThreadManagerTest, hardware_resources_locked) { - CHECK(htm != nullptr); - CHECK_EQ(streams.size(), threads); + // HVX can share HexagonHvx* hvx = new HexagonHvx(); - HexagonHtp* htp = new HexagonHtp(); - - // These should succeed hvx->Lock(); hvx->Unlock(); - - // This should throw - EXPECT_THROW(htp->Acquire(), InternalError); - delete hvx; - delete htp; + + // HTP cannot + EXPECT_THROW(HexagonHtp* htp = new HexagonHtp(), InternalError); } From 971ba00b10612f6694a02522a3abc08682fadc1b Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Sat, 22 Oct 2022 09:06:24 -0700 Subject: [PATCH 08/12] Bug fixes --- src/runtime/hexagon/hexagon_thread_manager.cc | 2 ++ tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/runtime/hexagon/hexagon_thread_manager.cc b/src/runtime/hexagon/hexagon_thread_manager.cc index f5ee9cea8dda..f2327d3329a8 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.cc +++ b/src/runtime/hexagon/hexagon_thread_manager.cc @@ -208,6 +208,8 @@ TVMStreamHandle HexagonThreadManager::GetStreamHandleByResourceType(HardwareReso } HardwareResourceType HexagonThreadManager::GetResourceTypeForStreamHandle(TVMStreamHandle thread) { + CHECK(hw_resources_.size() > reinterpret_cast(thread)) + << "No thread for handle id exists " << thread; return hw_resources_[reinterpret_cast(thread)]; } diff --git a/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc b/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc index 85e7edc8806e..3ea3537aa98c 100644 --- a/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc @@ -355,6 +355,9 @@ TEST_F(HexagonThreadManagerTest, threads_for_resource_types) { CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_2); thread = thread_manager->GetStreamHandleByResourceType(HVX_3); CHECK(thread_manager->GetResourceTypeForStreamHandle(thread) == HVX_3); + EXPECT_THROW(thread_manager->GetStreamHandleByResourceType(NONE), InternalError); + thread = reinterpret_cast(6); + EXPECT_THROW(thread_manager->GetResourceTypeForStreamHandle(thread), InternalError); } // Ensure proper behavior of hardware resources managed by global thread manager @@ -366,5 +369,5 @@ TEST_F(HexagonThreadManagerTest, hardware_resources_locked) { delete hvx; // HTP cannot - EXPECT_THROW(HexagonHtp* htp = new HexagonHtp(), InternalError); + EXPECT_THROW(new HexagonHtp(), InternalError); } From b86d68d5556b7248f6ac8238838b152dc387981b Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Tue, 25 Oct 2022 11:52:39 -0700 Subject: [PATCH 09/12] Remove test that hangs on lock on sim --- .../hexagon/hexagon_thread_manager_tests.cc | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc b/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc index 3ea3537aa98c..af29a428bc69 100644 --- a/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_thread_manager_tests.cc @@ -359,15 +359,3 @@ TEST_F(HexagonThreadManagerTest, threads_for_resource_types) { thread = reinterpret_cast(6); EXPECT_THROW(thread_manager->GetResourceTypeForStreamHandle(thread), InternalError); } - -// Ensure proper behavior of hardware resources managed by global thread manager -TEST_F(HexagonThreadManagerTest, hardware_resources_locked) { - // HVX can share - HexagonHvx* hvx = new HexagonHvx(); - hvx->Lock(); - hvx->Unlock(); - delete hvx; - - // HTP cannot - EXPECT_THROW(new HexagonHtp(), InternalError); -} From bd74d13d39a15d9838b6b11e092670cc30eeedf6 Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Tue, 25 Oct 2022 15:48:00 -0700 Subject: [PATCH 10/12] PR feedback --- src/runtime/hexagon/hexagon_hvx.cc | 4 +--- src/runtime/hexagon/hexagon_hvx.h | 2 +- src/runtime/hexagon/hexagon_thread_manager.cc | 5 ++++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/runtime/hexagon/hexagon_hvx.cc b/src/runtime/hexagon/hexagon_hvx.cc index e90a0c801828..4fc97bf95475 100644 --- a/src/runtime/hexagon/hexagon_hvx.cc +++ b/src/runtime/hexagon/hexagon_hvx.cc @@ -37,9 +37,7 @@ HexagonHvx::~HexagonHvx() { Release(); } void HexagonHvx::Acquire() { reserved_count_ = qurt_hvx_reserve(QURT_HVX_RESERVE_ALL); - CHECK((reserved_count_ == QURT_HVX_RESERVE_ALL) || - (reserved_count_ == QURT_HVX_RESERVE_ALREADY_MADE)) - << "error reserving HVX: " << reserved_count_; + CHECK(reserved_count_ == QURT_HVX_RESERVE_ALL) << "error reserving HVX: " << reserved_count_; } void HexagonHvx::Release() { diff --git a/src/runtime/hexagon/hexagon_hvx.h b/src/runtime/hexagon/hexagon_hvx.h index b09cf2611833..06394d7c5c7d 100644 --- a/src/runtime/hexagon/hexagon_hvx.h +++ b/src/runtime/hexagon/hexagon_hvx.h @@ -51,7 +51,7 @@ class HexagonHvx { //! \brief Unlock the HVX for the calling thread. void Unlock(); - //! \brief Number of HVX units reservered. + //! \brief Number of HVX units reserved. int ReservedCount() { return reserved_count_; } private: diff --git a/src/runtime/hexagon/hexagon_thread_manager.cc b/src/runtime/hexagon/hexagon_thread_manager.cc index f2327d3329a8..5f405c73d5fb 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.cc +++ b/src/runtime/hexagon/hexagon_thread_manager.cc @@ -43,7 +43,10 @@ HexagonThreadManager::HexagonThreadManager(unsigned num_threads, unsigned thread if (create_resource_managers_) { DLOG(INFO) << "Initialize hardware resource managers"; - // Acquisition/locks will be performed on specific threads + // This creates the manager objects, which reserves (acquires) the resources. + // Calls to lock/unlock will be performed on threads dedicated to instances. + // This must be done before spawing threads so we can pass pointers to the + // objects in the thread context. htp_ = std::make_unique(); hvx_ = std::make_unique(); } From 2ce5b881b32b5b0b5f36d4b342cc3c8efdbd00d3 Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Wed, 26 Oct 2022 08:29:50 -0700 Subject: [PATCH 11/12] Spelling --- src/runtime/hexagon/hexagon_thread_manager.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/runtime/hexagon/hexagon_thread_manager.cc b/src/runtime/hexagon/hexagon_thread_manager.cc index 5f405c73d5fb..b8e5701fb1e5 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.cc +++ b/src/runtime/hexagon/hexagon_thread_manager.cc @@ -45,7 +45,7 @@ HexagonThreadManager::HexagonThreadManager(unsigned num_threads, unsigned thread DLOG(INFO) << "Initialize hardware resource managers"; // This creates the manager objects, which reserves (acquires) the resources. // Calls to lock/unlock will be performed on threads dedicated to instances. - // This must be done before spawing threads so we can pass pointers to the + // This must be done before spawning threads so we can pass pointers to the // objects in the thread context. htp_ = std::make_unique(); hvx_ = std::make_unique(); @@ -322,10 +322,10 @@ void HexagonThreadManager::thread_exit(void* context) { if ((resource_type == HVX_0) || (resource_type == HVX_1) || (resource_type == HVX_2) || (resource_type == HVX_3)) { - tc->hvx->Unlock(); + //tc->hvx->Unlock(); DLOG(INFO) << "Thread " << index << " unlocked an HVX instance"; } else if (resource_type == HTP_0) { - tc->htp->Unlock(); + //tc->htp->Unlock(); DLOG(INFO) << "Thread " << index << " unlocked the HTP"; } From 41068ef5074c41cc840cd44c4b55be3b5e0ea3dc Mon Sep 17 00:00:00 2001 From: Janet Schneider Date: Wed, 26 Oct 2022 08:31:25 -0700 Subject: [PATCH 12/12] unlock --- src/runtime/hexagon/hexagon_thread_manager.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/runtime/hexagon/hexagon_thread_manager.cc b/src/runtime/hexagon/hexagon_thread_manager.cc index b8e5701fb1e5..2fbc231e5781 100644 --- a/src/runtime/hexagon/hexagon_thread_manager.cc +++ b/src/runtime/hexagon/hexagon_thread_manager.cc @@ -322,10 +322,10 @@ void HexagonThreadManager::thread_exit(void* context) { if ((resource_type == HVX_0) || (resource_type == HVX_1) || (resource_type == HVX_2) || (resource_type == HVX_3)) { - //tc->hvx->Unlock(); + tc->hvx->Unlock(); DLOG(INFO) << "Thread " << index << " unlocked an HVX instance"; } else if (resource_type == HTP_0) { - //tc->htp->Unlock(); + tc->htp->Unlock(); DLOG(INFO) << "Thread " << index << " unlocked the HTP"; }