[llvm] [Parallel] Ensure getThreadIndex()==0 for sequential tasks (PR #109084)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 17 21:45:06 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-support
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
https://reviews.llvm.org/D148728 introduced `bool Sequential` to unify
`execute` and the old `spawn` without argument. However, sequential
tasks might be executed by any worker thread (non-deterministic),
leading to non-determinism output for ld.lld -z nocombreloc.
In addition, the extra member variables have overhead.
This patch restores the behavior before https://reviews.llvm.org/D148728
, but temporarily sets `threadIndex` to 0 when the main thread is
executing the sequential task. In addition, use `vector` instead of
`stack` for `WorkStack`.
Fix #<!-- -->105958
---
Full diff: https://github.com/llvm/llvm-project/pull/109084.diff
1 Files Affected:
- (modified) llvm/lib/Support/Parallel.cpp (+18-36)
``````````diff
diff --git a/llvm/lib/Support/Parallel.cpp b/llvm/lib/Support/Parallel.cpp
index a3ef3d9c621b98..4aec5b14ebb146 100644
--- a/llvm/lib/Support/Parallel.cpp
+++ b/llvm/lib/Support/Parallel.cpp
@@ -12,7 +12,6 @@
#include "llvm/Support/Threading.h"
#include <atomic>
-#include <deque>
#include <future>
#include <thread>
#include <vector>
@@ -39,7 +38,7 @@ namespace {
class Executor {
public:
virtual ~Executor() = default;
- virtual void add(std::function<void()> func, bool Sequential = false) = 0;
+ virtual void add(std::function<void()> func) = 0;
virtual size_t getThreadCount() const = 0;
static Executor *getDefaultExecutor();
@@ -98,13 +97,10 @@ class ThreadPoolExecutor : public Executor {
static void call(void *Ptr) { ((ThreadPoolExecutor *)Ptr)->stop(); }
};
- void add(std::function<void()> F, bool Sequential = false) override {
+ void add(std::function<void()> F) override {
{
std::lock_guard<std::mutex> Lock(Mutex);
- if (Sequential)
- WorkQueueSequential.emplace_front(std::move(F));
- else
- WorkQueue.emplace_back(std::move(F));
+ WorkStack.push_back(std::move(F));
}
Cond.notify_one();
}
@@ -112,42 +108,23 @@ class ThreadPoolExecutor : public Executor {
size_t getThreadCount() const override { return ThreadCount; }
private:
- bool hasSequentialTasks() const {
- return !WorkQueueSequential.empty() && !SequentialQueueIsLocked;
- }
-
- bool hasGeneralTasks() const { return !WorkQueue.empty(); }
-
void work(ThreadPoolStrategy S, unsigned ThreadID) {
threadIndex = ThreadID;
S.apply_thread_strategy(ThreadID);
while (true) {
std::unique_lock<std::mutex> Lock(Mutex);
- Cond.wait(Lock, [&] {
- return Stop || hasGeneralTasks() || hasSequentialTasks();
- });
+ Cond.wait(Lock, [&] { return Stop || !WorkStack.empty(); });
if (Stop)
break;
- bool Sequential = hasSequentialTasks();
- if (Sequential)
- SequentialQueueIsLocked = true;
- else
- assert(hasGeneralTasks());
-
- auto &Queue = Sequential ? WorkQueueSequential : WorkQueue;
- auto Task = std::move(Queue.back());
- Queue.pop_back();
+ auto Task = std::move(WorkStack.back());
+ WorkStack.pop_back();
Lock.unlock();
Task();
- if (Sequential)
- SequentialQueueIsLocked = false;
}
}
std::atomic<bool> Stop{false};
- std::atomic<bool> SequentialQueueIsLocked{false};
- std::deque<std::function<void()>> WorkQueue;
- std::deque<std::function<void()>> WorkQueueSequential;
+ std::vector<std::function<void()>> WorkStack;
std::mutex Mutex;
std::condition_variable Cond;
std::promise<void> ThreadsCreated;
@@ -217,13 +194,18 @@ TaskGroup::~TaskGroup() {
void TaskGroup::spawn(std::function<void()> F, bool Sequential) {
#if LLVM_ENABLE_THREADS
if (Parallel) {
+ if (Sequential) {
+ // Act as worker thread 0.
+ threadIndex = 0;
+ F();
+ threadIndex = UINT_MAX;
+ return;
+ }
L.inc();
- detail::Executor::getDefaultExecutor()->add(
- [&, F = std::move(F)] {
- F();
- L.dec();
- },
- Sequential);
+ detail::Executor::getDefaultExecutor()->add([&, F = std::move(F)] {
+ F();
+ L.dec();
+ });
return;
}
#endif
``````````
</details>
https://github.com/llvm/llvm-project/pull/109084
More information about the llvm-commits
mailing list