[PATCH] [Core] Add parallel infrastructure to lld.

Michael Spencer bigcheesegs at gmail.com
Tue Apr 9 17:17:05 PDT 2013



================
Comment at: include/lld/Core/Parallel.h:47-50
@@ +46,6 @@
+
+  void dec() {
+    if (--_count == 0)
+      _cond.notify_all();
+  }
+
----------------
Shankar Kalpathi Easwaran wrote:
> Michael Spencer wrote:
> > Shankar Kalpathi Easwaran wrote:
> > > you might want to add a else case to assert probably.
> > There's nothing to assert here.
> what I meant was if the latch was decremented twice. you could catch that error.
You mean below 0? decrementing twice is perfectly valid.

================
Comment at: include/lld/Core/Parallel.h:79
@@ +78,3 @@
+      }
+      work();
+    }).detach();
----------------
Shankar Kalpathi Easwaran wrote:
> Michael Spencer wrote:
> > Shankar Kalpathi Easwaran wrote:
> > > It might be good to add a name to the task, for tracing purposes.
> > Which task?
> any task that the thread pool would manage, for example : when you try to add tasks that would read input files parallelly in lld, you could add the name of the file to it, so that you know when the thread starts processing the work.
That's what the ScopedTask stuff is for. That overhead shouldn't be added by default.

================
Comment at: include/lld/Core/Parallel.h:97
@@ +96,3 @@
+private:
+  void work() {
+    while (true) {
----------------
Shankar Kalpathi Easwaran wrote:
> Michael Spencer wrote:
> > 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?
> I thought the task that was scheduled to execute in this thread, could fail and the whole process has to be stopped.
> 
> For example : you tried to parse all the files in lld parallelly and one of the files was a bad file, you might want to return that failure immediately from the thread and stop everything else.
That should be handled at a higher level. ThreadPoolExecutor threads live until the process exits.

================
Comment at: include/lld/Core/Parallel.h:108
@@ +107,3 @@
+      lock.unlock();
+      task();
+    }
----------------
Shankar Kalpathi Easwaran wrote:
> Michael Spencer wrote:
> > 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.
> Ok. I was thinking if Tasks are made to return an ErrorOr/bool, you could get the return value of task and stop other threads on failure.
An Executor is a low level interface for tasks. See http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3562.pdf for what it's based on.

Also, we don't want other threads to stop when a task reports an error. We may be able to recover from it. All threads will stop before the process exits anyway.

If a task needs to return a value, it should use std::packaged_task. If you find the need to use this often, it's trivial to make a wrapper for it.


http://llvm-reviews.chandlerc.com/D649



More information about the llvm-commits mailing list