Skip to content

Commit 54521a2

Browse files
committed
Merge commit b84d773fd004 from llvm git (by Fangrui Song):
[Parallel] Revert sequential task changes 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 (see https://reviews.llvm.org/D133003). In addition, the extra member variables have overhead. This sequential task has only been used for lld parallel relocation scanning. This patch restores the behavior before https://reviews.llvm.org/D148728 . Fix #105958 Pull Request: llvm/llvm-project#109084 This fixes the non-reproducibility we had noticed when linking our EFI loaders, and for which we committed a workaround in f5ce3f4. MFC after: 3 days
1 parent f5a8f6f commit 54521a2

File tree

3 files changed

+48
-61
lines changed

3 files changed

+48
-61
lines changed

contrib/llvm-project/lld/ELF/Relocations.cpp

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1584,30 +1584,44 @@ template <class ELFT> void elf::scanRelocations() {
15841584
bool serial = !config->zCombreloc || config->emachine == EM_MIPS ||
15851585
config->emachine == EM_PPC64;
15861586
parallel::TaskGroup tg;
1587-
for (ELFFileBase *f : ctx.objectFiles) {
1588-
auto fn = [f]() {
1587+
auto outerFn = [&]() {
1588+
for (ELFFileBase *f : ctx.objectFiles) {
1589+
auto fn = [f]() {
1590+
RelocationScanner scanner;
1591+
for (InputSectionBase *s : f->getSections()) {
1592+
if (s && s->kind() == SectionBase::Regular && s->isLive() &&
1593+
(s->flags & SHF_ALLOC) &&
1594+
!(s->type == SHT_ARM_EXIDX && config->emachine == EM_ARM))
1595+
scanner.template scanSection<ELFT>(*s);
1596+
}
1597+
};
1598+
if (serial)
1599+
fn();
1600+
else
1601+
tg.spawn(fn);
1602+
}
1603+
auto scanEH = [] {
15891604
RelocationScanner scanner;
1590-
for (InputSectionBase *s : f->getSections()) {
1591-
if (s && s->kind() == SectionBase::Regular && s->isLive() &&
1592-
(s->flags & SHF_ALLOC) &&
1593-
!(s->type == SHT_ARM_EXIDX && config->emachine == EM_ARM))
1594-
scanner.template scanSection<ELFT>(*s);
1605+
for (Partition &part : partitions) {
1606+
for (EhInputSection *sec : part.ehFrame->sections)
1607+
scanner.template scanSection<ELFT>(*sec);
1608+
if (part.armExidx && part.armExidx->isLive())
1609+
for (InputSection *sec : part.armExidx->exidxSections)
1610+
if (sec->isLive())
1611+
scanner.template scanSection<ELFT>(*sec);
15951612
}
15961613
};
1597-
tg.spawn(fn, serial);
1598-
}
1599-
1600-
tg.spawn([] {
1601-
RelocationScanner scanner;
1602-
for (Partition &part : partitions) {
1603-
for (EhInputSection *sec : part.ehFrame->sections)
1604-
scanner.template scanSection<ELFT>(*sec);
1605-
if (part.armExidx && part.armExidx->isLive())
1606-
for (InputSection *sec : part.armExidx->exidxSections)
1607-
if (sec->isLive())
1608-
scanner.template scanSection<ELFT>(*sec);
1609-
}
1610-
});
1614+
if (serial)
1615+
scanEH();
1616+
else
1617+
tg.spawn(scanEH);
1618+
};
1619+
// If `serial` is true, call `spawn` to ensure that `scanner` runs in a thread
1620+
// with valid getThreadIndex().
1621+
if (serial)
1622+
tg.spawn(outerFn);
1623+
else
1624+
outerFn();
16111625
}
16121626

16131627
static bool handleNonPreemptibleIfunc(Symbol &sym, uint16_t flags) {

contrib/llvm-project/llvm/include/llvm/Support/Parallel.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,7 @@ class TaskGroup {
9797
// Spawn a task, but does not wait for it to finish.
9898
// Tasks marked with \p Sequential will be executed
9999
// exactly in the order which they were spawned.
100-
// Note: Sequential tasks may be executed on different
101-
// threads, but strictly in sequential order.
102-
void spawn(std::function<void()> f, bool Sequential = false);
100+
void spawn(std::function<void()> f);
103101

104102
void sync() const { L.sync(); }
105103

contrib/llvm-project/llvm/lib/Support/Parallel.cpp

Lines changed: 12 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#include "llvm/Support/Threading.h"
1313

1414
#include <atomic>
15-
#include <deque>
1615
#include <future>
1716
#include <thread>
1817
#include <vector>
@@ -39,7 +38,7 @@ namespace {
3938
class Executor {
4039
public:
4140
virtual ~Executor() = default;
42-
virtual void add(std::function<void()> func, bool Sequential = false) = 0;
41+
virtual void add(std::function<void()> func) = 0;
4342
virtual size_t getThreadCount() const = 0;
4443

4544
static Executor *getDefaultExecutor();
@@ -98,56 +97,34 @@ class ThreadPoolExecutor : public Executor {
9897
static void call(void *Ptr) { ((ThreadPoolExecutor *)Ptr)->stop(); }
9998
};
10099

101-
void add(std::function<void()> F, bool Sequential = false) override {
100+
void add(std::function<void()> F) override {
102101
{
103102
std::lock_guard<std::mutex> Lock(Mutex);
104-
if (Sequential)
105-
WorkQueueSequential.emplace_front(std::move(F));
106-
else
107-
WorkQueue.emplace_back(std::move(F));
103+
WorkStack.push_back(std::move(F));
108104
}
109105
Cond.notify_one();
110106
}
111107

112108
size_t getThreadCount() const override { return ThreadCount; }
113109

114110
private:
115-
bool hasSequentialTasks() const {
116-
return !WorkQueueSequential.empty() && !SequentialQueueIsLocked;
117-
}
118-
119-
bool hasGeneralTasks() const { return !WorkQueue.empty(); }
120-
121111
void work(ThreadPoolStrategy S, unsigned ThreadID) {
122112
threadIndex = ThreadID;
123113
S.apply_thread_strategy(ThreadID);
124114
while (true) {
125115
std::unique_lock<std::mutex> Lock(Mutex);
126-
Cond.wait(Lock, [&] {
127-
return Stop || hasGeneralTasks() || hasSequentialTasks();
128-
});
116+
Cond.wait(Lock, [&] { return Stop || !WorkStack.empty(); });
129117
if (Stop)
130118
break;
131-
bool Sequential = hasSequentialTasks();
132-
if (Sequential)
133-
SequentialQueueIsLocked = true;
134-
else
135-
assert(hasGeneralTasks());
136-
137-
auto &Queue = Sequential ? WorkQueueSequential : WorkQueue;
138-
auto Task = std::move(Queue.back());
139-
Queue.pop_back();
119+
auto Task = std::move(WorkStack.back());
120+
WorkStack.pop_back();
140121
Lock.unlock();
141122
Task();
142-
if (Sequential)
143-
SequentialQueueIsLocked = false;
144123
}
145124
}
146125

147126
std::atomic<bool> Stop{false};
148-
std::atomic<bool> SequentialQueueIsLocked{false};
149-
std::deque<std::function<void()>> WorkQueue;
150-
std::deque<std::function<void()>> WorkQueueSequential;
127+
std::vector<std::function<void()>> WorkStack;
151128
std::mutex Mutex;
152129
std::condition_variable Cond;
153130
std::promise<void> ThreadsCreated;
@@ -205,16 +182,14 @@ TaskGroup::~TaskGroup() {
205182
L.sync();
206183
}
207184

208-
void TaskGroup::spawn(std::function<void()> F, bool Sequential) {
185+
void TaskGroup::spawn(std::function<void()> F) {
209186
#if LLVM_ENABLE_THREADS
210187
if (Parallel) {
211188
L.inc();
212-
detail::Executor::getDefaultExecutor()->add(
213-
[&, F = std::move(F)] {
214-
F();
215-
L.dec();
216-
},
217-
Sequential);
189+
detail::Executor::getDefaultExecutor()->add([&, F = std::move(F)] {
190+
F();
191+
L.dec();
192+
});
218193
return;
219194
}
220195
#endif

0 commit comments

Comments
 (0)