Skip to content

Commit 93d4b1f

Browse files
authored
Reapply "[llvm-jitlink] Use concurrent linking by default." with fixes. (#120958)
Reapplies commit edca1d9 which was reverted in 34531cf while I investigated bot failures, (e.g. https://lab.llvm.org/buildbot/#/builders/137/builds/10791). Commit 158a600 should address the -check failures on the bots, which were caused by checks running earlier under the concurrent linking scheme before all files referenced by the checks had been fully linked. This patch also fixes the -threads option failure by renaming the option to -num-threads to avoid clashing with the ThreadCount cl::opt variable defined in ThinLTOCodeGenerator.cpp.
1 parent 275a277 commit 93d4b1f

File tree

4 files changed

+54
-8
lines changed

4 files changed

+54
-8
lines changed

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)