[PATCH] D109914: [Support] Attempt to fix deadlock in ThreadGroup

Alexandre Ganea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 17 08:46:39 PDT 2021


aganea updated this revision to Diff 373237.
aganea edited the summary of this revision.
aganea added a comment.

Removed the sync from `~Latch()`.
Removed the `std::condition_variable::notify_all()` improvement since it was occasionally breaking on Windows (and Linux too it seems).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109914/new/

https://reviews.llvm.org/D109914

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


Index: llvm/lib/Support/Parallel.cpp
===================================================================
--- llvm/lib/Support/Parallel.cpp
+++ llvm/lib/Support/Parallel.cpp
@@ -151,7 +151,12 @@
 // lock, only allow the first TaskGroup to run tasks parallelly. In the scenario
 // of nested parallel_for_each(), only the outermost one runs parallelly.
 TaskGroup::TaskGroup() : Parallel(TaskGroupInstances++ == 0) {}
-TaskGroup::~TaskGroup() { --TaskGroupInstances; }
+TaskGroup::~TaskGroup() {
+  // We must ensure that all the workloads have finished before decrementing the
+  // instances count.
+  L.sync();
+  --TaskGroupInstances;
+}
 
 void TaskGroup::spawn(std::function<void()> F) {
   if (Parallel) {
Index: llvm/include/llvm/Support/Parallel.h
===================================================================
--- llvm/include/llvm/Support/Parallel.h
+++ llvm/include/llvm/Support/Parallel.h
@@ -40,7 +40,10 @@
 
 public:
   explicit Latch(uint32_t Count = 0) : Count(Count) {}
-  ~Latch() { sync(); }
+  ~Latch() {
+    // Ensure at least that sync() was called.
+    assert(Count == 0);
+  }
 
   void inc() {
     std::lock_guard<std::mutex> lock(Mutex);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D109914.373237.patch
Type: text/x-patch
Size: 1171 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210917/a54f9f44/attachment.bin>


More information about the llvm-commits mailing list