Skip to content

Commit 0965515

Browse files
committed
"Reapply "[llvm-jitlink] Use concurrent linking by default." with more fixes.
This reapplies edca1d9 which was reverted in 7ec139a due to bot failures. LocalDependencyPropagation.s is updated to use -num-threads=0 in order to avoid interleaving debugging output. ELFNixPlatform.h is updated to protect the deferred runtime function calls map during bootstrap.
1 parent 23ec9ee commit 0965515

File tree

6 files changed

+58
-11
lines changed

6 files changed

+58
-11
lines changed

llvm/include/llvm/ExecutionEngine/Orc/ELFNixPlatform.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ class ELFNixPlatform : public Platform {
156156
RuntimeFunction *func1, RuntimeFunction *func2,
157157
const shared::WrapperFunctionCall::ArgDataBufferType &arg1,
158158
const shared::WrapperFunctionCall::ArgDataBufferType &arg2) {
159+
std::lock_guard<std::mutex> Lock(Mutex);
159160
auto &argList = DeferredRTFnMap[std::make_pair(func1, func2)];
160161
argList.emplace_back(arg1, arg2);
161162
}

llvm/test/ExecutionEngine/JITLink/x86-64/LocalDependencyPropagation.s

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
# REQUIRES: asserts
22
# RUN: llvm-mc -triple=x86_64-apple-macosx10.9 -filetype=obj -o %t %s
3-
# RUN: llvm-jitlink -debug-only=orc -noexec -abs _external_func=0x1 \
4-
# RUN: -entry=_foo %t 2>&1 | FileCheck %s
3+
# RUN: llvm-jitlink -debug-only=orc -num-threads=0 -noexec \
4+
# RUN: -abs _external_func=0x1 -entry=_foo %t 2>&1 | FileCheck %s
55
#
66
# Check that simplification eliminates dependencies on symbols in this unit,
77
# and correctly propagates dependencies on symbols outside the unit (including
88
# via locally scoped symbols). In this test _baz depends on _foo indirectly via
99
# the local symbol _bar. Initially we expect _baz to depend on _foo, and _foo
1010
# on _external_func, after simplification we expect both to depend on
11-
# _external_func only.
11+
# _external_func only.
1212

1313
# CHECK: In main emitting {{.*}}_foo{{.*}}
1414
# CHECK-NEXT: Initial dependencies:

llvm/tools/llvm-jitlink/llvm-jitlink-coff.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ static Expected<Symbol &> getCOFFStubTarget(LinkGraph &G, Block &B) {
6666

6767
namespace llvm {
6868
Error registerCOFFGraphInfo(Session &S, LinkGraph &G) {
69+
std::lock_guard<std::mutex> Lock(S.M);
70+
6971
auto FileName = sys::path::filename(G.getName());
7072
if (S.FileInfos.count(FileName)) {
7173
return make_error<StringError>("When -check is passed, file names must be "

llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ static Error registerSymbol(LinkGraph &G, Symbol &Sym, Session::FileInfo &FI,
101101
namespace llvm {
102102

103103
Error registerELFGraphInfo(Session &S, LinkGraph &G) {
104+
std::lock_guard<std::mutex> Lock(S.M);
105+
104106
auto FileName = sys::path::filename(G.getName());
105107
if (S.FileInfos.count(FileName)) {
106108
return make_error<StringError>("When -check is passed, file names must be "

llvm/tools/llvm-jitlink/llvm-jitlink-macho.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ static Expected<Symbol &> getMachOStubTarget(LinkGraph &G, Block &B) {
6969
namespace llvm {
7070

7171
Error registerMachOGraphInfo(Session &S, LinkGraph &G) {
72+
std::lock_guard<std::mutex> Lock(S.M);
73+
7274
auto FileName = sys::path::filename(G.getName());
7375
if (S.FileInfos.count(FileName)) {
7476
return make_error<StringError>("When -check is passed, file names must be "

llvm/tools/llvm-jitlink/llvm-jitlink.cpp

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ static cl::list<std::string> InputFiles(cl::Positional, cl::OneOrMore,
9191
cl::desc("input files"),
9292
cl::cat(JITLinkCategory));
9393

94+
static cl::opt<size_t> MaterializationThreads(
95+
"num-threads", cl::desc("Number of materialization threads to use"),
96+
cl::init(std::numeric_limits<size_t>::max()), cl::cat(JITLinkCategory));
97+
9498
static cl::list<std::string>
9599
LibrarySearchPaths("L",
96100
cl::desc("Add dir to the list of library search paths"),
@@ -400,6 +404,7 @@ bool lazyLinkingRequested() {
400404
}
401405

402406
static Error applyHarnessPromotions(Session &S, LinkGraph &G) {
407+
std::lock_guard<std::mutex> Lock(S.M);
403408

404409
// If this graph is part of the test harness there's nothing to do.
405410
if (S.HarnessFiles.empty() || S.HarnessFiles.count(G.getName()))
@@ -450,7 +455,11 @@ static Error applyHarnessPromotions(Session &S, LinkGraph &G) {
450455
return Error::success();
451456
}
452457

453-
static void dumpSectionContents(raw_ostream &OS, LinkGraph &G) {
458+
static void dumpSectionContents(raw_ostream &OS, Session &S, LinkGraph &G) {
459+
std::lock_guard<std::mutex> Lock(S.M);
460+
461+
outs() << "Relocated section contents for " << G.getName() << ":\n";
462+
454463
constexpr orc::ExecutorAddrDiff DumpWidth = 16;
455464
static_assert(isPowerOf2_64(DumpWidth), "DumpWidth must be a power of two");
456465

@@ -842,7 +851,7 @@ static Expected<std::unique_ptr<ExecutorProcessControl>> launchExecutor() {
842851
S.CreateMemoryManager = createSharedMemoryManager;
843852

844853
return SimpleRemoteEPC::Create<FDSimpleRemoteEPCTransport>(
845-
std::make_unique<DynamicThreadPoolTaskDispatcher>(std::nullopt),
854+
std::make_unique<DynamicThreadPoolTaskDispatcher>(MaterializationThreads),
846855
std::move(S), FromExecutor[ReadEnd], ToExecutor[WriteEnd]);
847856
#endif
848857
}
@@ -984,10 +993,16 @@ Expected<std::unique_ptr<Session>> Session::Create(Triple TT,
984993
auto PageSize = sys::Process::getPageSize();
985994
if (!PageSize)
986995
return PageSize.takeError();
996+
std::unique_ptr<TaskDispatcher> Dispatcher;
997+
if (MaterializationThreads == 0)
998+
Dispatcher = std::make_unique<InPlaceTaskDispatcher>();
999+
else
1000+
Dispatcher = std::make_unique<DynamicThreadPoolTaskDispatcher>(
1001+
MaterializationThreads);
1002+
9871003
EPC = std::make_unique<SelfExecutorProcessControl>(
988-
std::make_shared<SymbolStringPool>(),
989-
std::make_unique<InPlaceTaskDispatcher>(), std::move(TT), *PageSize,
990-
createInProcessMemoryManager());
1004+
std::make_shared<SymbolStringPool>(), std::move(Dispatcher),
1005+
std::move(TT), *PageSize, createInProcessMemoryManager());
9911006
}
9921007

9931008
Error Err = Error::success();
@@ -1221,6 +1236,7 @@ void Session::modifyPassConfig(LinkGraph &G, PassConfiguration &PassConfig) {
12211236

12221237
if (ShowGraphsRegex)
12231238
PassConfig.PostFixupPasses.push_back([this](LinkGraph &G) -> Error {
1239+
std::lock_guard<std::mutex> Lock(M);
12241240
// Print graph if ShowLinkGraphs is specified-but-empty, or if
12251241
// it contains the given graph.
12261242
if (ShowGraphsRegex->match(G.getName())) {
@@ -1239,9 +1255,8 @@ void Session::modifyPassConfig(LinkGraph &G, PassConfiguration &PassConfig) {
12391255
[this](LinkGraph &G) { return applyHarnessPromotions(*this, G); });
12401256

12411257
if (ShowRelocatedSectionContents)
1242-
PassConfig.PostFixupPasses.push_back([](LinkGraph &G) -> Error {
1243-
outs() << "Relocated section contents for " << G.getName() << ":\n";
1244-
dumpSectionContents(outs(), G);
1258+
PassConfig.PostFixupPasses.push_back([this](LinkGraph &G) -> Error {
1259+
dumpSectionContents(outs(), *this, G);
12451260
return Error::success();
12461261
});
12471262

@@ -1613,6 +1628,31 @@ static Error sanitizeArguments(const Triple &TT, const char *ArgV0) {
16131628
}
16141629
}
16151630

1631+
if (MaterializationThreads == std::numeric_limits<size_t>::max()) {
1632+
if (auto HC = std::thread::hardware_concurrency())
1633+
MaterializationThreads = HC;
1634+
else {
1635+
errs() << "Warning: std::thread::hardware_concurrency() returned 0, "
1636+
"defaulting to -threads=1.\n";
1637+
MaterializationThreads = 1;
1638+
}
1639+
}
1640+
1641+
if (!!OutOfProcessExecutor.getNumOccurrences() ||
1642+
!!OutOfProcessExecutorConnect.getNumOccurrences()) {
1643+
if (NoExec)
1644+
return make_error<StringError>("-noexec cannot be used with " +
1645+
OutOfProcessExecutor.ArgStr + " or " +
1646+
OutOfProcessExecutorConnect.ArgStr,
1647+
inconvertibleErrorCode());
1648+
1649+
if (MaterializationThreads == 0)
1650+
return make_error<StringError>("-threads=0 cannot be used with " +
1651+
OutOfProcessExecutor.ArgStr + " or " +
1652+
OutOfProcessExecutorConnect.ArgStr,
1653+
inconvertibleErrorCode());
1654+
}
1655+
16161656
// Only one of -oop-executor and -oop-executor-connect can be used.
16171657
if (!!OutOfProcessExecutor.getNumOccurrences() &&
16181658
!!OutOfProcessExecutorConnect.getNumOccurrences())

0 commit comments

Comments
 (0)