[PATCH] D61115: Parallel: only allow the first TaskGroup to run tasks parallelly

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: ruiu, sbc100, andrewrk, zturner, dblaikie.
Herald added subscribers: llvm-commits, kristina, jfb.
Herald added a project: LLVM.

Concurrent (e.g. nested) llvm::parallel::for_each() may lead to dead
locks. See PR35788 (fixed by rLLD322041 <https://reviews.llvm.org/rLLD322041>) and PR41508 (fixed by D60757 <https://reviews.llvm.org/D60757>).

When parallel_for_each is about to return, in ~Latch() called by
~TaskGroup(), a thread (in the default executor) may block in
Latch::sync() waiting for Count to become zero. If all threads in the
default executor block, it is a dead lock.

To fix this, force serial execution if the current TaskGroup is not the
first one. For a nested llvm::parallel::for_each(), only the outmost one
runs parallelly.


Repository:
  rL LLVM

https://reviews.llvm.org/D61115

Files:
  include/llvm/Support/Parallel.h
  lib/Support/Parallel.cpp


Index: lib/Support/Parallel.cpp
===================================================================
--- lib/Support/Parallel.cpp
+++ lib/Support/Parallel.cpp
@@ -17,7 +17,9 @@
 #include <stack>
 #include <thread>
 
-using namespace llvm;
+namespace llvm {
+namespace parallel {
+namespace detail {
 
 namespace {
 
@@ -118,11 +120,28 @@
 #endif
 }
 
-void parallel::detail::TaskGroup::spawn(std::function<void()> F) {
-  L.inc();
-  Executor::getDefaultExecutor()->add([&, F] {
+static std::atomic<int> TaskGroupInstances;
+
+// Latch::sync() called by the dtor may cause one thread to block. If is a dead
+// lock if all threads are blocked. To prevent the dead lock, only allow the
+// first TaskGroup to run tasks parallelly. In the scenario of nested
+// parallel_for_each, only the outmost one runs parallelly.
+TaskGroup::TaskGroup() : Parallel(TaskGroupInstances++ == 0) {}
+TaskGroup::~TaskGroup() { --TaskGroupInstances; }
+
+void TaskGroup::spawn(std::function<void()> F) {
+  if (Parallel) {
+    L.inc();
+    Executor::getDefaultExecutor()->add([&, F] {
+      F();
+      L.dec();
+    });
+  } else {
     F();
-    L.dec();
-  });
+  }
 }
+
+} // namespace detail
+} // namespace parallel
+} // namespace llvm
 #endif // LLVM_ENABLE_THREADS
Index: include/llvm/Support/Parallel.h
===================================================================
--- include/llvm/Support/Parallel.h
+++ include/llvm/Support/Parallel.h
@@ -73,8 +73,12 @@
 
 class TaskGroup {
   Latch L;
+  bool Parallel;
 
 public:
+  TaskGroup();
+  ~TaskGroup();
+
   void spawn(std::function<void()> f);
 
   void sync() const { L.sync(); }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61115.196580.patch
Type: text/x-patch
Size: 1629 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190425/ffa92642/attachment.bin>