[PATCH] [Core] Add parallel infrastructure to lld.
Michael Spencer
bigcheesegs at gmail.com
Tue Apr 9 16:35:47 PDT 2013
================
Comment at: include/lld/Core/Parallel.h:103-104
@@ +102,4 @@
+ });
+ if (_stop)
+ break;
+ auto task = _workQueue.front();
----------------
Shankar Kalpathi Easwaran wrote:
> You would need to unlock the mutex here.
unique_lock unlocks the mutex in the destructor.
================
Comment at: include/lld/Core/Parallel.h:47-50
@@ +46,6 @@
+
+ void dec() {
+ if (--_count == 0)
+ _cond.notify_all();
+ }
+
----------------
Shankar Kalpathi Easwaran wrote:
> you might want to add a else case to assert probably.
There's nothing to assert here.
================
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) {
----------------
Shankar Kalpathi Easwaran wrote:
> I think size_t is pretty big for a threadcount.
I'll change it to the return type of hardware_concurrency (unsigned).
================
Comment at: include/lld/Core/Parallel.h:79
@@ +78,3 @@
+ }
+ work();
+ }).detach();
----------------
Shankar Kalpathi Easwaran wrote:
> It might be good to add a name to the task, for tracing purposes.
Which task?
================
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:
> Shankar Kalpathi Easwaran wrote:
> > Nice!
> Doesnot handle the case when some of the threads cant be created.
We don't use exception handling. There's no way to detect this case.
================
Comment at: include/lld/Core/Parallel.h:97
@@ +96,3 @@
+private:
+ void work() {
+ while (true) {
----------------
Shankar Kalpathi Easwaran wrote:
> This could return an ErrorOr to indicate success/failure of this thread.
It can't fail. And what would look at the return value?
================
Comment at: include/lld/Core/Parallel.h:108
@@ +107,3 @@
+ lock.unlock();
+ task();
+ }
----------------
Shankar Kalpathi Easwaran wrote:
> it would be nice to get the task return status and check for success/failure and set the stop field.
Task returns void.
http://llvm-reviews.chandlerc.com/D649
More information about the llvm-commits
mailing list