You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/04/14 21:57:53 UTC
[GitHub] [tvm] supersat opened a new pull request, #11018: [Hexagon][Runtime] Add QuRT thread pool backend
supersat opened a new pull request, #11018:
URL: https://github.com/apache/tvm/pull/11018
pthreads (and thus std::thread) does not work correctly on older versions of QuRT. This PR modifies threading_backend.cc to use the QuRT thread APIs directly when building for Hexagon devices.
@kparzysz-quic @areusch @csullivan
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] supersat commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
supersat commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r857817525
##########
src/runtime/threading_backend.cc:
##########
@@ -276,7 +334,17 @@ int ThreadGroup::Configure(AffinityMode mode, int nthreads, bool exclude_worker0
return impl_->Configure(mode, nthreads, exclude_worker0, cpus);
}
-void Yield() { std::this_thread::yield(); }
+void Yield() {
+#ifdef __hexagon__
+ // QuRT doesn't have a yield API, so instead we sleep for the minimum amount
+ // of time to let the OS schedule another thread. std::this_thread::yield()
+ // compiles down to an empty function.
+ qurt_sleep(1);
Review Comment:
std::this_thread::yield() is broken on hexagon -- it just immediately returns. The documentation for qurt_sleep() is silent on what happens if a 0 is passed in -- qurt could simply short-circuit the function call and immediately return.
Passing in a minimum amount of time (1 microsecond) guarantees the thread will yield.
I am waiting on Qualcomm to clarify what the behavior of qurt_sleep(0) is.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r856596366
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
Review Comment:
I think that abstracting thread control would be the best approach here. The runtime built for a particular target could then contain the implementation provided by that target (e.g. via template specialization), and there wouldn't be any run-time dependency on device kind.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r853614410
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
Review Comment:
wondering if it's possible to do this with templating instead of with `#ifdef __hexagon__`?
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] supersat commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
supersat commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r859118107
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f_(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ int ret = posix_memalign(&stack_, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ CHECK_EQ(ret, 0);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack_);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ ret = qurt_thread_create(&thread_, &attr, (void (*)(void*))RunFunction, this);
+ CHECK_EQ(ret, QURT_EOK);
+ }
+ QuRTThread(QuRTThread&& other) : thread_(other.thread_), f_(other.f_), stack_(other.stack_) {
+ other.thread_ = 0;
Review Comment:
free(NULL) is defined to be a no-op, but I added an explicit check anyway.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
huajsj commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r861363102
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
+ void join() {
+ int status;
+ qurt_thread_join(thread, &status);
Review Comment:
get it, following the existing std::join usage is make sense.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
csullivan commented on PR #11018:
URL: https://github.com/apache/tvm/pull/11018#issuecomment-1116640732
Thanks @supersat @huajsj @areusch @kparzysz-quic, this PR is merged.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
huajsj commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r861362247
##########
src/runtime/threading_backend.cc:
##########
@@ -276,7 +334,17 @@ int ThreadGroup::Configure(AffinityMode mode, int nthreads, bool exclude_worker0
return impl_->Configure(mode, nthreads, exclude_worker0, cpus);
}
-void Yield() { std::this_thread::yield(); }
+void Yield() {
+#ifdef __hexagon__
+ // QuRT doesn't have a yield API, so instead we sleep for the minimum amount
+ // of time to let the OS schedule another thread. std::this_thread::yield()
+ // compiles down to an empty function.
+ qurt_sleep(1);
Review Comment:
get it, seems like this is a trade off due to platform limitation, I think we can keep qurt_sleep(1) in this patch then use a following patch to address qurt_sleep(0) issue after qualcomm reaching back.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] supersat commented on pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
supersat commented on PR #11018:
URL: https://github.com/apache/tvm/pull/11018#issuecomment-1114623056
After discussing with @csullivan, I've reverted the refactoring changes. Those changes relied on separate source files being built for separate targets, but PR #11090 introduces changes that build Hexagon sources for all targets, necessitating more #ifdefs.
Eventually the build system should only build target-specific sources when building for that target. There's a lot of cruft already in TVM because this isn't the case (e.g., #ifdef WIN32 in hexagon_buffer.cc and others). However, fixing that is a non-trivial task, so we plan to leave that to a separate PR.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] supersat commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
supersat commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r853445087
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
+ void join() {
+ int status;
+ qurt_thread_join(thread, &status);
+ }
+
+ private:
+ static void run_func(QuRTThread* t) {
+ t->f();
+ qurt_thread_exit(QURT_EOK);
+ }
+ qurt_thread_t thread;
+ Callback f;
+ void* stack;
Review Comment:
thread_, f_, and stack_ have been renamed.
Now that the return value of posix_memalign is checked, stack_ is guaranteed to be initialized.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r861191428
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
Review Comment:
@areusch @kparzysz-quic PTAL. I'm inclined to take @supersat's recommendation if this approach is too ugly. But we'd like to keep forward progress on this one. Thanks!
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan merged pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
csullivan merged PR #11018:
URL: https://github.com/apache/tvm/pull/11018
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r856400269
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
Review Comment:
Then the only other options for dispatch I see are
(1) TVM compile-time dispatch -- during codegen, do something specific based on the target
(2) Build time dispatch via either preprocessor or through changes to the build system to conditionally include one translation unit over another.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
huajsj commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r862277313
##########
include/tvm/runtime/threading_backend.h:
##########
@@ -110,6 +110,37 @@ class ThreadGroup {
Impl* impl_;
};
+class ThreadGroup::Impl {
+ public:
+ virtual void Join() {}
+ virtual int Configure(AffinityMode mode, int nthreads, bool exclude_worker0,
+ std::vector<unsigned int> cpus) = 0;
+ virtual ~Impl() { Join(); }
+};
+
+template <class ThreadType>
+class ThreadGroupImplTemplate : public ThreadGroup::Impl {
+ public:
+ virtual void Join() {
+ for (auto& t : threads_) {
+ if (t.joinable()) t.join();
+ }
+ }
+
Review Comment:
~ThreadGroupImplTemplate() { Join();}
##########
include/tvm/runtime/threading_backend.h:
##########
@@ -110,6 +110,37 @@ class ThreadGroup {
Impl* impl_;
};
+class ThreadGroup::Impl {
+ public:
+ virtual void Join() {}
+ virtual int Configure(AffinityMode mode, int nthreads, bool exclude_worker0,
+ std::vector<unsigned int> cpus) = 0;
+ virtual ~Impl() { Join(); }
Review Comment:
seems like here is to expect ThreadGroupImplTemplate:Join get triggered, but that may not happen, because the calling is in a deconstruct function.
##########
include/tvm/runtime/threading_backend.h:
##########
@@ -110,6 +110,37 @@ class ThreadGroup {
Impl* impl_;
};
+class ThreadGroup::Impl {
+ public:
+ virtual void Join() {}
Review Comment:
"virtual void Join() = 0" or {assert(0);} to make sure inherit class have a dedicated "Join" and get called.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
huajsj commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r853493512
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
Review Comment:
I see, seems like keep original name is better than put more #ifdef, I am ok for this part.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] supersat commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
supersat commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r853524382
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
+ void join() {
+ int status;
+ qurt_thread_join(thread, &status);
Review Comment:
qurt_thread_join can return QURT_ENOTHREAD if you already called join() on the thread. This could either be ignored (since the particular thread you're waiting on has already exited) or there could be a CHECK/crash (since you're not supposed to join() a thread multiple times). I could see arguments either way. std::thread::join() throws a std::system_error in this case.
qurt_thread_join can also raise a QuRT exception, which I don't think we handle at all.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] supersat commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
supersat commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r859117807
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
Review Comment:
I've refactored this PR to split up pthread and qurt threading implementations. This introduces the following changes:
- ThreadGroup::Impl is now an abstract class.
- A ThreadGroupImplTemplate class is introduced with common code between pthread and qurt implementations. It is a subclass of ThreadGroup::Impl. (AFAICT, you can't have a pointer to an unspecialized template class. We need a pointer to a concrete type for ThreadGroup to call.)
- ThreadGroupPosixImpl now contains the bulk of the code from ThreadGroup::Impl. It inherits from ThreadGroupImplTemplate<std::thread>. This is in a new file src/runtime/posix/threading_posix.cc.
- ThreadGroupHexagonImpl inherits from ThreadGroupImplTemplate<QuRTThread>, which are both defined in a new file, src/runtime/hexagon/threading_hexagon.cc.
- There are now two different versions of Yield()
- CMakeLists.txt has been modified to either include src/runtime/posix/threading_posix.cc or src/runtime/hexagon/threading_hexagon.cc, depending on whether we're building for Hexagon.
Honestly, this seems like a pretty ugly solution, especially when threading_backend.cc is already littered with #ifdefs for various platforms. Thoughts?
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
huajsj commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r854672421
##########
src/runtime/threading_backend.cc:
##########
@@ -276,7 +334,17 @@ int ThreadGroup::Configure(AffinityMode mode, int nthreads, bool exclude_worker0
return impl_->Configure(mode, nthreads, exclude_worker0, cpus);
}
-void Yield() { std::this_thread::yield(); }
+void Yield() {
+#ifdef __hexagon__
+ // QuRT doesn't have a yield API, so instead we sleep for the minimum amount
+ // of time to let the OS schedule another thread. std::this_thread::yield()
+ // compiles down to an empty function.
+ qurt_sleep(1);
Review Comment:
if the expectation is to let OS to do the schedule for CPU resource, why can not use std::this_thread::yield()? and could you help to put the information about the time unit for '1'(1 second or 1 millisecond)? if a qurt_sleep is necessary, does a 'qurt_sleep(0)' can trigger a yield?
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f_(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ int ret = posix_memalign(&stack_, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ CHECK_EQ(ret, 0);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack_);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ ret = qurt_thread_create(&thread_, &attr, (void (*)(void*))RunFunction, this);
+ CHECK_EQ(ret, QURT_EOK);
+ }
+ QuRTThread(QuRTThread&& other) : thread_(other.thread_), f_(other.f_), stack_(other.stack_) {
+ other.thread_ = 0;
Review Comment:
although line 73 already have the logic to avoid double free, after the construction from 'Rvalue' logically still need to clear member value(stack_, f_) of 'other', because these value already not owned by "other". doing this can help the future maintains.
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f_(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ int ret = posix_memalign(&stack_, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ CHECK_EQ(ret, 0);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack_);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ ret = qurt_thread_create(&thread_, &attr, (void (*)(void*))RunFunction, this);
+ CHECK_EQ(ret, QURT_EOK);
+ }
+ QuRTThread(QuRTThread&& other) : thread_(other.thread_), f_(other.f_), stack_(other.stack_) {
+ other.thread_ = 0;
+ }
+ ~QuRTThread() {
+ if (thread_) {
+ join();
+ free(stack_);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread_; }
+ void join() {
+ int status;
+ qurt_thread_join(thread_, &status);
+ }
+
+ private:
+ static void RunFunction(QuRTThread* t) {
+ t->f_();
+ qurt_thread_exit(QURT_EOK);
+ }
+ qurt_thread_t thread_;
+ Callback f_;
Review Comment:
use a more meaningful name to replace f_, for example 'worker_callback_'?
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f_(worker_callback) {
Review Comment:
check whether 'worker_callback' is valid
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f_(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ int ret = posix_memalign(&stack_, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ CHECK_EQ(ret, 0);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack_);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ ret = qurt_thread_create(&thread_, &attr, (void (*)(void*))RunFunction, this);
+ CHECK_EQ(ret, QURT_EOK);
+ }
+ QuRTThread(QuRTThread&& other) : thread_(other.thread_), f_(other.f_), stack_(other.stack_) {
+ other.thread_ = 0;
+ }
+ ~QuRTThread() {
+ if (thread_) {
+ join();
+ free(stack_);
Review Comment:
there is a hide logic that stack_ bind with thread_, but such hide logic is not guaranteed not change in future, better to independent check and free them.
if (stack_)free(stack_);
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f_(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ int ret = posix_memalign(&stack_, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ CHECK_EQ(ret, 0);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack_);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ ret = qurt_thread_create(&thread_, &attr, (void (*)(void*))RunFunction, this);
+ CHECK_EQ(ret, QURT_EOK);
+ }
+ QuRTThread(QuRTThread&& other) : thread_(other.thread_), f_(other.f_), stack_(other.stack_) {
+ other.thread_ = 0;
+ }
+ ~QuRTThread() {
+ if (thread_) {
+ join();
+ free(stack_);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread_; }
+ void join() {
+ int status;
+ qurt_thread_join(thread_, &status);
+ }
+
+ private:
+ static void RunFunction(QuRTThread* t) {
Review Comment:
use a more meaningful name for 't', like qrt_thread ?
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] supersat commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
supersat commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r853386964
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
Review Comment:
QuRTThread implements a subset of the std::thread API. I could change it to use Google's C++ style, but then I would need to add more #ifdefs around where it's used (e.g., line 107). What's preferable here?
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] supersat commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
supersat commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r855678214
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
Review Comment:
To a certain extent, although it's not clear it's worth the effort. As far as I can tell are a couple options:
- Have multiple classes that implement the ThreadGroup::Impl interface -- one for pthreads, and one for QuRT threads. @csullivan was concerned that this might lead to duplicated code, making it more fragile to maintain.
- Parameterizing ThreadPool::Impl on the underlying thread type. However, we'd still need #ifdefs to avoid calling SetAffinity, which isn't available on Hexagon.
Yield() also doesn't work as-is on Hexagon, so we'd need a fix for that as well.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r856397727
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
Review Comment:
One approach would be to introduce
```
template <typename ThreadType>
class ThreadGroup::Impl;
```
and then wrap both std::thread and QuRT thread into types with a common interface that `ThreadGroup::Impl` calls into.
The thing I got hung up on there is the runtime dispatch. Right now it should be doable from the device type -- when it is `kDLHexagon` dispatch to `ThreadGroup::Impl<QuRTThreadInterface>` when it's `kDLCPU` dispatch to `TheadGroup::Impl<StdThreadInterface>`. However there is impetus to move Hexagon fully over to kDLCPU -- wherein we could no longer do runtime dispatch based on the device type.
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,63 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
Review Comment:
Then the only other options for dispatch I see are then
(1) TVM compile-time dispatch -- during codegen, do something specific based on the target
(2) Build time dispatch via either preprocessor or through changes to the build system to conditionally include one translation unit over another.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a diff in pull request #11018: [Hexagon][Runtime] Add QuRT thread pool backend
Posted by GitBox <gi...@apache.org>.
huajsj commented on code in PR #11018:
URL: https://github.com/apache/tvm/pull/11018#discussion_r852556302
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
+ void join() {
+ int status;
+ qurt_thread_join(thread, &status);
+ }
+
+ private:
+ static void run_func(QuRTThread* t) {
+ t->f();
+ qurt_thread_exit(QURT_EOK);
+ }
+ qurt_thread_t thread;
+ Callback f;
+ void* stack;
Review Comment:
qurt_thread_t thread_;
Callback f_;
void* stack_;
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
Review Comment:
function name convention should use google c/c++ style.
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
+ void join() {
+ int status;
+ qurt_thread_join(thread, &status);
+ }
+
+ private:
+ static void run_func(QuRTThread* t) {
Review Comment:
RunFunction()
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
Review Comment:
check value of 'stack' before doing free
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
+ void join() {
+ int status;
+ qurt_thread_join(thread, &status);
Review Comment:
check 'qurt_thread_join' return value?
##########
src/runtime/threading_backend.cc:
##########
@@ -276,7 +332,14 @@ int ThreadGroup::Configure(AffinityMode mode, int nthreads, bool exclude_worker0
return impl_->Configure(mode, nthreads, exclude_worker0, cpus);
}
-void Yield() { std::this_thread::yield(); }
+void Yield() {
+#ifdef __hexagon__
+ qurt_sleep(1);
Review Comment:
How to determine the 1 as the sleep number? any todo or comments for explain?
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
+ void join() {
+ int status;
+ qurt_thread_join(thread, &status);
+ }
+
+ private:
+ static void run_func(QuRTThread* t) {
+ t->f();
+ qurt_thread_exit(QURT_EOK);
+ }
+ qurt_thread_t thread;
+ Callback f;
+ void* stack;
Review Comment:
stack_ = nullptr
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
Review Comment:
check return value.
##########
src/runtime/threading_backend.cc:
##########
@@ -34,13 +34,61 @@
#endif
#if defined(__hexagon__)
#include <dlfcn.h>
+#include <qurt.h>
+#include <stdlib.h>
+#define HEXAGON_STACK_SIZE 65536
+#define HEXAGON_STACK_ALIGNMENT 32
#endif
#include <algorithm>
#include <thread>
#define CURRENT_THREAD_HANDLE (static_cast<std::thread::native_handle_type>(0))
namespace tvm {
namespace runtime {
namespace threading {
+#ifdef __hexagon__
+// pthreads are broken on older versions of qurt, so
+// we need to use native APIs instead of std::threads
+class QuRTThread {
+ typedef std::function<void()> Callback;
+
+ public:
+ explicit QuRTThread(Callback worker_callback) : f(worker_callback) {
+ static int id = 1;
+ qurt_thread_attr_t attr;
+ char name[32];
+ posix_memalign(&stack, HEXAGON_STACK_ALIGNMENT, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_init(&attr);
+ qurt_thread_attr_set_stack_size(&attr, HEXAGON_STACK_SIZE);
+ qurt_thread_attr_set_stack_addr(&attr, stack);
+ snprintf(name, sizeof(name), "worker %d", id++);
+ qurt_thread_attr_set_name(&attr, name);
+ qurt_thread_create(&thread, &attr, (void (*)(void*))run_func, this);
+ }
+ QuRTThread(QuRTThread&& other) : thread(other.thread), f(other.f), stack(other.stack) {
+ other.thread = 0;
+ }
+ ~QuRTThread() {
+ if (thread) {
+ join();
+ free(stack);
+ }
+ }
+ bool joinable() const { return qurt_thread_get_id() != thread; }
+ void join() {
Review Comment:
Join()
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org