csullivan commented on code in PR #13181:
URL: https://github.com/apache/tvm/pull/13181#discussion_r1005016552
##########
src/runtime/hexagon/hexagon_hvx.cc:
##########
@@ -31,25 +31,30 @@ namespace tvm {
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;
+HexagonHvx::HexagonHvx() { Acquire(); }
- // Lock HVX.
+HexagonHvx::~HexagonHvx() { Release(); }
+
+void HexagonHvx::Acquire() {
+ reserved_count_ = qurt_hvx_reserve(QURT_HVX_RESERVE_ALL);
Review Comment:
Oh wait, does every thread main call
`qurt_hvx_reserve(QURT_HVX_RESERVE_ALL);` because each owns an HexagonHvx?
##########
src/runtime/hexagon/hexagon_thread_manager.cc:
##########
@@ -168,7 +180,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());
Review Comment:
So if we are binding resources to the thread contexts, will they no longer
be available to the global thread? Because we don't have lowering support to
the HTM yet, we still need these resources availble to the global thread. Do
you have thoughts on how to transition the runtime from one model to the other
(global thread -> stream based backed by the HTM)?
##########
src/runtime/hexagon/hexagon_hvx.cc:
##########
@@ -31,25 +31,30 @@ namespace tvm {
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;
+HexagonHvx::HexagonHvx() { Acquire(); }
- // Lock HVX.
+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))
Review Comment:
Yeah I agree that this should probably be an error if `reserved_count_ ==
QURT_HVX_RESERVE_ALREADY_MADE`.
##########
src/runtime/hexagon/hexagon_hvx.cc:
##########
@@ -31,25 +31,30 @@ namespace tvm {
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;
+HexagonHvx::HexagonHvx() { Acquire(); }
- // Lock HVX.
+HexagonHvx::~HexagonHvx() { Release(); }
+
+void HexagonHvx::Acquire() {
+ reserved_count_ = qurt_hvx_reserve(QURT_HVX_RESERVE_ALL);
Review Comment:
I think we'll always want to own the set of available HVX coprocessors and
so this is good.
##########
src/runtime/hexagon/hexagon_thread_manager.cc:
##########
@@ -168,7 +180,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());
Review Comment:
Oh, I think I see, right now the global thread is acquiring them, and then
you are passing in a pointer to the HexagonHtp or HexagonHvx. And later on you
plan to acquire them (construct) directly in thread_main?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
