[PATCH] [Core] Add parallel infrastructure to lld.
Shankar Kalpathi Easwaran
shankarke at gmail.com
Tue Apr 9 16:06:14 PDT 2013
================
Comment at: include/lld/Core/Parallel.h:103-104
@@ +102,4 @@
+ });
+ if (_stop)
+ break;
+ auto task = _workQueue.front();
----------------
You would need to unlock the mutex here.
================
Comment at: include/lld/Core/Parallel.h:73-81
@@ +72,11 @@
+ // take a while.
+ std::thread([&, threadCount] {
+ for (std::size_t i = 1; i < threadCount; ++i) {
+ std::thread([=] {
+ work();
+ }).detach();
+ }
+ work();
+ }).detach();
+ }
+
----------------
Nice!
================
Comment at: include/lld/Core/Parallel.h:68-69
@@ +67,4 @@
+public:
+ ThreadPoolExecutor(std::size_t threadCount =
+ std::thread::hardware_concurrency())
+ : _stop(false), _done(threadCount) {
----------------
I think size_t is pretty big for a threadcount.
================
Comment at: include/lld/Core/Parallel.h:108
@@ +107,3 @@
+ lock.unlock();
+ task();
+ }
----------------
it would be nice to get the task return status and check for success/failure and set the stop field.
================
Comment at: include/lld/Core/Parallel.h:97
@@ +96,3 @@
+private:
+ void work() {
+ while (true) {
----------------
This could return an ErrorOr to indicate success/failure of this thread.
================
Comment at: include/lld/Core/Parallel.h:73-80
@@ +72,10 @@
+ // take a while.
+ std::thread([&, threadCount] {
+ for (std::size_t i = 1; i < threadCount; ++i) {
+ std::thread([=] {
+ work();
+ }).detach();
+ }
+ work();
+ }).detach();
+ }
----------------
Shankar Kalpathi Easwaran wrote:
> Nice!
Doesnot handle the case when some of the threads cant be created.
================
Comment at: include/lld/Core/Parallel.h:47-50
@@ +46,6 @@
+
+ void dec() {
+ if (--_count == 0)
+ _cond.notify_all();
+ }
+
----------------
you might want to add a else case to assert probably.
================
Comment at: include/lld/Core/Parallel.h:79
@@ +78,3 @@
+ }
+ work();
+ }).detach();
----------------
It might be good to add a name to the task, for tracing purposes.
http://llvm-reviews.chandlerc.com/D649
More information about the llvm-commits
mailing list