[clang-repl] [ORC] Add support for out-of-process execution on ELF #79936
Conversation
| Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot added clang 
| @llvm/pr-subscribers-clang Author: None (jameshu15869) ChangesThis PR adds initial support for out-of-process execution through llvm-jitlink-executor to clang-repl on ELF. The out-of-process executor can be invoked with "-oop-executor" or "-oop-executor-connect=<host>" on ELF platforms. Now, clang-repl depends upon the ORC runtime to support static initializers and deinitializers. This PR modifies the ORC ELFNix platform to allow JITDylibs to be reinitialized through the "__orc_rt_jit_dlupdate" function, since clang-repl incrementally adds static initializers to a single JITDylib. On x86_64 linux, the following tests pass with the out-of-process executor flag. However, a new new testing file (out-of-process.cpp) was added with select tests to restrict the test to ELF only.
Patch is 28.44 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/79936.diff 13 Files Affected:
diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h index 43573fb1a4b89..314beb7b72dac 100644 --- a/clang/include/clang/Interpreter/Interpreter.h +++ b/clang/include/clang/Interpreter/Interpreter.h @@ -21,6 +21,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ExecutionEngine/JITSymbol.h" +#include "llvm/ExecutionEngine/Orc/ExecutorProcessControl.h" #include "llvm/ExecutionEngine/Orc/Shared/ExecutorAddress.h" #include "llvm/Support/Error.h" #include <memory> @@ -81,6 +82,11 @@ class Interpreter { // An optional parser for CUDA offloading std::unique_ptr<IncrementalParser> DeviceParser; + // An optional parameter for an out-of-process executor + std::unique_ptr<llvm::orc::ExecutorProcessControl> EPC; + + llvm::StringRef OrcRuntimePath; + Interpreter(std::unique_ptr<CompilerInstance> CI, llvm::Error &Err); llvm::Error CreateExecutor(); @@ -98,6 +104,10 @@ class Interpreter { static llvm::Expected<std::unique_ptr<Interpreter>> createWithCUDA(std::unique_ptr<CompilerInstance> CI, std::unique_ptr<CompilerInstance> DCI); + static llvm::Expected<std::unique_ptr<Interpreter>> + createWithOutOfProcessExecutor( + std::unique_ptr<CompilerInstance> CI, + std::unique_ptr<llvm::orc::ExecutorProcessControl> EI, llvm::StringRef OrcRuntimePath); const ASTContext &getASTContext() const; ASTContext &getASTContext(); const CompilerInstance *getCompilerInstance() const; diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp b/clang/lib/Interpreter/IncrementalExecutor.cpp index 40bcef94797d4..30b24caa4a594 100644 --- a/clang/lib/Interpreter/IncrementalExecutor.cpp +++ b/clang/lib/Interpreter/IncrementalExecutor.cpp @@ -63,6 +63,35 @@ IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, } } +IncrementalExecutor::IncrementalExecutor( + llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err, + const clang::TargetInfo &TI, + std::unique_ptr<llvm::orc::ExecutorProcessControl> EPC, llvm::StringRef OrcRuntimePath) + : TSCtx(TSC) { + using namespace llvm::orc; + llvm::ErrorAsOutParameter EAO(&Err); + + auto JTMB = JITTargetMachineBuilder(TI.getTriple()); + JTMB.addFeatures(TI.getTargetOpts().Features); + LLJITBuilder Builder; + Builder.setJITTargetMachineBuilder(JTMB); + Builder.setPrePlatformSetup([](LLJIT &J) { + // Try to enable debugging of JIT'd code (only works with JITLink for + // ELF and MachO). + consumeError(enableDebuggerSupport(J)); + return llvm::Error::success(); + }); + Builder.setExecutorProcessControl(std::move(EPC)); + Builder.setPlatformSetUp(llvm::orc::ExecutorNativePlatform(OrcRuntimePath.str())); + + if (auto JitOrErr = Builder.create()) { + Jit = std::move(*JitOrErr); + } else { + Err = JitOrErr.takeError(); + return; + } +} + IncrementalExecutor::~IncrementalExecutor() {} llvm::Error IncrementalExecutor::addModule(PartialTranslationUnit &PTU) { diff --git a/clang/lib/Interpreter/IncrementalExecutor.h b/clang/lib/Interpreter/IncrementalExecutor.h index dd0a210a06141..a73ba9035182c 100644 --- a/clang/lib/Interpreter/IncrementalExecutor.h +++ b/clang/lib/Interpreter/IncrementalExecutor.h @@ -46,6 +46,9 @@ class IncrementalExecutor { IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err, const clang::TargetInfo &TI); + IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err, + const clang::TargetInfo &TI, + std::unique_ptr<llvm::orc::ExecutorProcessControl> EPC, llvm::StringRef OrcRuntimePath); ~IncrementalExecutor(); llvm::Error addModule(PartialTranslationUnit &PTU); diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp index 7968c62cbd3e7..13e6be3b54abc 100644 --- a/clang/lib/Interpreter/Interpreter.cpp +++ b/clang/lib/Interpreter/Interpreter.cpp @@ -36,6 +36,7 @@ #include "clang/Lex/PreprocessorOptions.h" #include "clang/Sema/Lookup.h" #include "llvm/ExecutionEngine/JITSymbol.h" +#include "llvm/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.h" #include "llvm/ExecutionEngine/Orc/LLJIT.h" #include "llvm/IR/Module.h" #include "llvm/Support/Errc.h" @@ -242,6 +243,15 @@ Interpreter::~Interpreter() { llvm::Twine("Failed to clean up IncrementalExecutor: ") + toString(std::move(Err))); } + + if (EPC) { + if (auto Err = EPC->disconnect()) { + llvm::report_fatal_error( + llvm::Twine("Failed to clean up EPC (IncrementalExecutor has not yet " + "been created): ") + + toString(std::move(Err))); + } + } } // These better to put in a runtime header but we can't. This is because we @@ -315,6 +325,20 @@ Interpreter::createWithCUDA(std::unique_ptr<CompilerInstance> CI, return Interp; } + +llvm::Expected<std::unique_ptr<Interpreter>> +Interpreter::createWithOutOfProcessExecutor( + std::unique_ptr<CompilerInstance> CI, + std::unique_ptr<llvm::orc::ExecutorProcessControl> EI, llvm::StringRef OrcRuntimePath) { + auto Interp = create(std::move(CI)); + if (auto E = Interp.takeError()) { + return std::move(E); + } + (*Interp)->EPC = std::move(EI); + (*Interp)->OrcRuntimePath = OrcRuntimePath; + return Interp; +} + const CompilerInstance *Interpreter::getCompilerInstance() const { return IncrParser->getCI(); } @@ -363,7 +387,13 @@ llvm::Error Interpreter::CreateExecutor() { const clang::TargetInfo &TI = getCompilerInstance()->getASTContext().getTargetInfo(); llvm::Error Err = llvm::Error::success(); - auto Executor = std::make_unique<IncrementalExecutor>(*TSCtx, Err, TI); + std::unique_ptr<IncrementalExecutor> Executor; + if (EPC) { + Executor = + std::make_unique<IncrementalExecutor>(*TSCtx, Err, TI, std::move(EPC), OrcRuntimePath); + } else { + Executor = std::make_unique<IncrementalExecutor>(*TSCtx, Err, TI); + } if (!Err) IncrExecutor = std::move(Executor); @@ -460,10 +490,8 @@ llvm::Error Interpreter::LoadDynamicLibrary(const char *name) { if (!EE) return EE.takeError(); - auto &DL = EE->getDataLayout(); - - if (auto DLSG = llvm::orc::DynamicLibrarySearchGenerator::Load( - name, DL.getGlobalPrefix())) + if (auto DLSG = llvm::orc::EPCDynamicLibrarySearchGenerator::Load( + EE->getExecutionSession(), name)) EE->getMainJITDylib().addGenerator(std::move(*DLSG)); else return DLSG.takeError(); diff --git a/clang/test/Interpreter/dynamic-library.cpp b/clang/test/Interpreter/dynamic-library.cpp index 6c4621f729c1c..c2d186f71ebfe 100644 --- a/clang/test/Interpreter/dynamic-library.cpp +++ b/clang/test/Interpreter/dynamic-library.cpp @@ -15,6 +15,7 @@ // } // RUN: cat %s | env LD_LIBRARY_PATH=%S/Inputs:$LD_LIBRARY_PATH clang-repl | FileCheck %s +// RUN: cat %s | env LD_LIBRARY_PATH=%S/Inputs:$LD_LIBRARY_PATH clang-repl -oop-executor | FileCheck %s extern "C" int printf(const char* format, ...); diff --git a/clang/test/Interpreter/out-of-process.cpp b/clang/test/Interpreter/out-of-process.cpp new file mode 100644 index 0000000000000..ddbf86ec65881 --- /dev/null +++ b/clang/test/Interpreter/out-of-process.cpp @@ -0,0 +1,61 @@ +// REQUIRES: host-supports-jit, x86_64-linux +// RUN: cat %s | clang-repl | FileCheck %s +// RUN: cat %s | clang-repl -oop-executor | FileCheck %s + +extern "C" int printf(const char *, ...); + +// Test code undo. +int x1 = 0; +int x2 = 42; +%undo +int x2 = 24; +auto r1 = printf("x1 = %d\n", x1); +// CHECK: x1 = 0 +auto r2 = printf("x2 = %d\n", x2); +// CHECK-NEXT: x2 = 24 +int foo() { return 1; } +%undo +int foo() { return 2; } +auto r3 = printf("foo() = %d\n", foo()); +// CHECK-NEXT: foo() = 2 +inline int bar() { return 42;} +auto r4 = bar(); +%undo +auto r5 = bar(); + +// Test weak execution. +int __attribute__((weak)) weak_bar() { return 42; } +auto r6 = printf("bar() = %d\n", weak_bar()); +// CHECK: bar() = 42 +int a = 12; +static __typeof(a) b __attribute__((__weakref__("a"))); +int c = b; + +// Test lambdas. +auto l1 = []() { printf("ONE\n"); return 42; }; +auto l2 = []() { printf("TWO\n"); return 17; }; +auto lambda_r1 = l1(); +// CHECK: ONE +auto lambda_r2 = l2(); +// CHECK: TWO +auto lambda_r3 = l2(); +// CHECK: TWO + +// Test multiline. +int i = \ + 12; +printf("i=%d\n", i); +// CHECK: i=12 +void f(int x) \ +{ \ + printf("x=\ + %d", x); \ +} +f(i); +// CHECK: x=12 + +// Test global destructor. +struct D { float f = 1.0; D *m = nullptr; D(){} ~D() { printf("D[f=%f, m=0x%llx]\n", f, reinterpret_cast<unsigned long long>(m)); }} d; +// CHECK: D[f=1.000000, m=0x0] + +%quit diff --git a/clang/tools/clang-repl/CMakeLists.txt b/clang/tools/clang-repl/CMakeLists.txt index 2ccbe292fd49e..adf4ea4180cba 100644 --- a/clang/tools/clang-repl/CMakeLists.txt +++ b/clang/tools/clang-repl/CMakeLists.txt @@ -4,7 +4,9 @@ set( LLVM_LINK_COMPONENTS LineEditor Option OrcJIT + OrcShared Support + TargetParser ) add_clang_tool(clang-repl diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp index 5663c2c5a6c92..1c43e7b503646 100644 --- a/clang/tools/clang-repl/ClangRepl.cpp +++ b/clang/tools/clang-repl/ClangRepl.cpp @@ -16,13 +16,21 @@ #include "clang/Interpreter/CodeCompletion.h" #include "clang/Interpreter/Interpreter.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/ExecutionEngine/Orc/ExecutorProcessControl.h" #include "llvm/ExecutionEngine/Orc/LLJIT.h" +#include "llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h" #include "llvm/LineEditor/LineEditor.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/ManagedStatic.h" // llvm_shutdown #include "llvm/Support/Signals.h" #include "llvm/Support/TargetSelect.h" +#include "llvm/TargetParser/Host.h" +#include <netdb.h> #include <optional> +#include <sys/socket.h> +#include <sys/types.h> +#include <unistd.h> // Disable LSan for this test. // FIXME: Re-enable once we can assume GCC 13.2 or higher. @@ -45,6 +53,21 @@ static llvm::cl::opt<bool> OptHostSupportsJit("host-supports-jit", static llvm::cl::list<std::string> OptInputs(llvm::cl::Positional, llvm::cl::desc("[code to run]")); +static llvm::cl::OptionCategory OOPCategory( + "Out-of-process Execution Options (Only available on ELF/Linux)"); +static llvm::cl::opt<std::string> OutOfProcessExecutor( + "oop-executor", + llvm::cl::desc("Launch an out-of-process executor to run code"), + llvm::cl::ValueOptional, llvm::cl::cat(OOPCategory)); +static llvm::cl::opt<std::string> OutOfProcessExecutorConnect( + "oop-executor-connect", + llvm::cl::desc("Connect to an out-of-process executor via TCP"), + llvm::cl::cat(OOPCategory)); +static llvm::cl::opt<std::string> OrcRuntimePath( + "orc-runtime", + llvm::cl::desc("Path to the ORC runtime"), + llvm::cl::cat(OOPCategory)); + static void LLVMErrorHandler(void *UserData, const char *Message, bool GenCrashDiag) { auto &Diags = *static_cast<clang::DiagnosticsEngine *>(UserData); @@ -143,6 +166,201 @@ ReplListCompleter::operator()(llvm::StringRef Buffer, size_t Pos, return Comps; } +static llvm::Error sanitizeOopArguments(const char *ArgV0) { + llvm::Triple SystemTriple(llvm::sys::getProcessTriple()); + if ((OutOfProcessExecutor.getNumOccurrences() || + OutOfProcessExecutorConnect.getNumOccurrences()) && + (!SystemTriple.isOSBinFormatELF())) + return llvm::make_error<llvm::StringError>( + "Out-process-executors are currently only supported on ELF", + llvm::inconvertibleErrorCode()); + + // Only one of -oop-executor and -oop-executor-connect can be used. + if (!!OutOfProcessExecutor.getNumOccurrences() && + !!OutOfProcessExecutorConnect.getNumOccurrences()) + return llvm::make_error<llvm::StringError>( + "Only one of -" + OutOfProcessExecutor.ArgStr + " and -" + + OutOfProcessExecutorConnect.ArgStr + " can be specified", + llvm::inconvertibleErrorCode()); + + // If -oop-executor was used but no value was specified then use a sensible + // default. + if (!!OutOfProcessExecutor.getNumOccurrences() && + OutOfProcessExecutor.empty()) { + llvm::SmallString<256> OOPExecutorPath(llvm::sys::fs::getMainExecutable( + ArgV0, reinterpret_cast<void *>(&sanitizeOopArguments))); + llvm::sys::path::remove_filename(OOPExecutorPath); + llvm::sys::path::append(OOPExecutorPath, "llvm-jitlink-executor"); + OutOfProcessExecutor = OOPExecutorPath.str().str(); + } + + // Out-of-process executors must run with the ORC runtime for destructor support. + if (OrcRuntimePath.empty() && (OutOfProcessExecutor.getNumOccurrences() || OutOfProcessExecutorConnect.getNumOccurrences())) { + llvm::SmallString<256> OrcPath(llvm::sys::fs::getMainExecutable( + ArgV0, reinterpret_cast<void *>(&sanitizeOopArguments))); + llvm::sys::path::remove_filename(OrcPath); // Remove clang-repl filename. + llvm::sys::path::remove_filename(OrcPath); // Remove ./bin directory. + llvm::sys::path::append(OrcPath, "lib/clang/18/lib/x86_64-unknown-linux-gnu/liborc_rt.a"); + OrcRuntimePath = OrcPath.str().str(); + } + + return llvm::Error::success(); +} + +static llvm::Expected<std::unique_ptr<llvm::orc::ExecutorProcessControl>> +launchExecutor() { + constexpr int ReadEnd = 0; + constexpr int WriteEnd = 1; + + // Pipe FDs. + int ToExecutor[2]; + int FromExecutor[2]; + + pid_t ChildPID; + + // Create pipes to/from the executor.. + if (pipe(ToExecutor) != 0 || pipe(FromExecutor) != 0) + return llvm::make_error<llvm::StringError>( + "Unable to create pipe for executor", llvm::inconvertibleErrorCode()); + + ChildPID = fork(); + + if (ChildPID == 0) { + // In the child... + + // Close the parent ends of the pipes + close(ToExecutor[WriteEnd]); + close(FromExecutor[ReadEnd]); + + // Execute the child process. + std::unique_ptr<char[]> ExecutorPath, FDSpecifier; + { + ExecutorPath = std::make_unique<char[]>(OutOfProcessExecutor.size() + 1); + strcpy(ExecutorPath.get(), OutOfProcessExecutor.data()); + + std::string FDSpecifierStr("filedescs="); + FDSpecifierStr += llvm::utostr(ToExecutor[ReadEnd]); + FDSpecifierStr += ','; + FDSpecifierStr += llvm::utostr(FromExecutor[WriteEnd]); + FDSpecifier = std::make_unique<char[]>(FDSpecifierStr.size() + 1); + strcpy(FDSpecifier.get(), FDSpecifierStr.c_str()); + } + + char *const Args[] = {ExecutorPath.get(), FDSpecifier.get(), nullptr}; + int RC = execvp(ExecutorPath.get(), Args); + if (RC != 0) { + llvm::errs() << "unable to launch out-of-process executor \"" + << ExecutorPath.get() << "\"\n"; + exit(1); + } + } + // else we're the parent... + + // Close the child ends of the pipes + close(ToExecutor[ReadEnd]); + close(FromExecutor[WriteEnd]); + + auto S = llvm::orc::SimpleRemoteEPC::Setup(); + + return llvm::orc::SimpleRemoteEPC::Create< + llvm::orc::FDSimpleRemoteEPCTransport>( + std::make_unique<llvm::orc::DynamicThreadPoolTaskDispatcher>(), + std::move(S), FromExecutor[ReadEnd], ToExecutor[WriteEnd]); +} + +#if LLVM_ON_UNIX && LLVM_ENABLE_THREADS +static llvm::Error createTCPSocketError(llvm::Twine Details) { + return llvm::make_error<llvm::StringError>( + formatv("Failed to connect TCP socket '{0}': {1}", + OutOfProcessExecutorConnect, Details), + llvm::inconvertibleErrorCode()); +} + +static llvm::Expected<int> connectTCPSocket(std::string Host, + std::string PortStr) { + addrinfo *AI; + addrinfo Hints{}; + Hints.ai_family = AF_INET; + Hints.ai_socktype = SOCK_STREAM; + Hints.ai_flags = AI_NUMERICSERV; + + if (int EC = getaddrinfo(Host.c_str(), PortStr.c_str(), &Hints, &AI)) + return createTCPSocketError("Address resolution failed (" + + llvm::StringRef(gai_strerror(EC)) + ")"); + + // Cycle through the returned addrinfo structures and connect to the first + // reachable endpoint. + int SockFD; + addrinfo *Server; + for (Server = AI; Server != nullptr; Server = Server->ai_next) { + // socket might fail, e.g. if the address family is not supported. Skip to + // the next addrinfo structure in such a case. + if ((SockFD = socket(AI->ai_family, AI->ai_socktype, AI->ai_protocol)) < 0) + continue; + + // If connect returns null, we exit the loop with a working socket. + if (connect(SockFD, Server->ai_addr, Server->ai_addrlen) == 0) + break; + + close(SockFD); + } + freeaddrinfo(AI); + + // If we reached the end of the loop without connecting to a valid endpoint, + // dump the last error that was logged in socket() or connect(). + if (Server == nullptr) + return createTCPSocketError(std::strerror(errno)); + + return SockFD; +} +#endif + +static llvm::Expected<std::unique_ptr<llvm::orc::ExecutorProcessControl>> +connectToExecutor() { +#ifndef LLVM_ON_UNIX + // FIXME: Add TCP support for Windows. + return llvm::make_error<StringError>( + "-" + OutOfProcessExecutorConnect.ArgStr + + " not supported on non-unix platforms", + inconvertibleErrorCode()); +#elif !LLVM_ENABLE_THREADS + // Out of process mode using SimpleRemoteEPC depends on threads. + return llvm::make_error<StringError>( + "-" + OutOfProcessExecutorConnect.ArgStr + + " requires threads, but LLVM was built with " + "LLVM_ENABLE_THREADS=Off", + inconvertibleErrorCode()); +#else + + llvm::StringRef Host, PortStr; + std::tie(Host, PortStr) = + llvm::StringRef(OutOfProcessExecutorConnect).split(':'); + if (Host.empty()) + return createTCPSocketError("Host name for -" + + OutOfProcessExecutorConnect.ArgStr + + " can not be empty"); + if (PortStr.empty()) + return createTCPSocketError("Port number in -" + + OutOfProcessExecutorConnect.ArgStr + + " can not be empty"); + int Port = 0; + if (PortStr.getAsInteger(10, Port)) + return createTCPSocketError("Port number '" + PortStr + + "' is not a valid integer"); + + llvm::Expected<int> SockFD = connectTCPSocket(Host.str(), PortStr.str()); + if (!SockFD) + return SockFD.takeError(); + + auto S = llvm::orc::SimpleRemoteEPC::Setup(); + + return llvm::orc::SimpleRemoteEPC::Create< + llvm::orc::FDSimpleRemoteEPCTransport>( + std::make_unique<llvm::orc::DynamicThreadPoolTaskDispatcher>(), + std::move(S), *SockFD, *SockFD); +#endif +} + llvm::ExitOnError ExitOnErr; int main(int argc, const char **argv) { ExitOnErr.setBanner("clang-repl: "); @@ -205,6 +423,8 @@ int main(int argc, const char **argv) { if (CudaEnabled) DeviceCI->LoadRequestedPlugins(); + ExitOnErr(sanitizeOopArguments(argv[0])); + std::unique_ptr<clang::Interpreter> Interp; if (CudaEnabled) { @@ -217,6 +437,16 @@ int main(int argc, const char **argv) { auto CudaRuntimeLibPath = CudaPath + "/lib/libcudart.so"; ExitOnErr(Interp->LoadDynamicLibrary(CudaRuntimeLibPath.c_str())); } + } else if (OutOfProcessExecutor.getNumOccurrences()) { + // Create an instance of llvm-jitlink-executor in a separate process. + auto oopExecutor = ExitOnErr(launchExecutor()); + Interp = ExitOnErr(clang::Interpreter::createWithOutOfProcessExecutor( + std::move(CI), std::move(oopExecutor), OrcRuntimePath)); + } else if (OutOfProcessExecutorConnect.getNumOccurrences()) { + /// If -oop-executor-connect is passed then connect to the executor. + auto REPC = ExitOnErr(connectToExecutor()); + Interp = ExitOnErr(clang::Interpreter::createWithOutOfProcessExecutor( + std::move(CI), std::move(REPC), OrcRuntimePath)); } else Interp = ExitOnErr(clang::Interpreter::create(std::move(CI))); diff --git a/compiler-rt/lib/orc/dlfcn_wrapper.cpp b/compiler-rt/lib/orc/dlfcn_wrapper.cpp index ece63da2cb48e..a8b207d27157e 100644 --- a/compiler-rt/lib/orc/dlfcn_wrapper.cpp +++ b/compiler-rt/lib/orc/dlfcn_wrapper.cpp @@ -20,6 +20,7 @@ using namespace __orc_rt; extern "C" const char *__orc_rt_jit_dlerror(); extern "C" void *__orc_rt_jit_dlopen(const char *pat... [truncated] |
| ✅ With the latest revision this PR passed the C/C++ code formatter. |
vgvassilev left a comment
vgvassilev left a comment There was a problem hiding this comment.
Thank you for working on this. This looks very good. I have left comments from my first review pass. We probably want to wait for @lhames and @weliveindetail to take a look.
| @@ -0,0 +1,61 @@ | |||
| // REQUIRES: host-supports-jit, x86_64-linux | |||
There was a problem hiding this comment.
I believe this test copies some content from other tests. Would it make sense to add an extra RUN line to the tests themselves?
There was a problem hiding this comment.
Since only ELF and Linux are supported right now, I thought that adding extra RUN lines to the others tests would cause unnecessary failures in COFF (Windows) and MachO (Mac). Would there be a way to conditionally execute RUN lines based on platform?
| #include <netdb.h> | ||
| #include <sys/socket.h> | ||
| #include <unistd.h> | ||
| #endif // LLVM_ON_UNIX |
There was a problem hiding this comment.
Have you had a chance to try it out on Windows? If that's beyond our scope we might want to add the platform to the unsupported platforms on the tests.
There was a problem hiding this comment.
I was seeing build failures in CI due to some headers. llvm-jitlink (Where I got most of the code for creating/managing the executor process) does not support connecting to an already running executor over TCP.
| std::make_unique<llvm::orc::DynamicThreadPoolTaskDispatcher>(), | ||
| std::move(S), *SockFD, *SockFD); | ||
| #endif | ||
| } |
There was a problem hiding this comment.
Would these not be useful for the case where a library (libInterpreter) is requested to do oop by user code?
There was a problem hiding this comment.
Could you elaborate more on what you mean by use case? I've tested running oop with a running instance of llvm-jitlink-executor on Linux, but the original llvm-jitlink tool that I got this section of code from didn't actually support TCP connections on Windows.
| Thanks for sharing your patch @jameshu15869! For the moment it seems like a mix of many moving pieces. I think it would be good to review them isolation, at least ORC runtime support and ELFNix platform changes. All the RPC utilities could go into a separate cpp right? We may even think about lifting them to llvm::support at some point, because we already have copies of it here and here. I didn't review in detail, but I think we need to initiate the connection to the executor much earlier. In particular, we must setup the frontend compiler instance with the target triple we get from the executor here: Apart from that, how scalable is the |
| Hi @weliveindetail, thank you for the comments sorry about the extremely delayed response - school has been busy and I just started a new job recently, so I haven't had much time to focus on other things. I was originally thinking that a lot of the ORC runtime changes might not make sense on their own, but now I agree that it makes sense to review them in isolation and agree on lifting RPC utilities. I also agree with your points creating the connection earlier. For the design decisions, I was mostly following the model for Unfortunately, I've had a lot more on my plate this semester and might not have the free time to continue working on this. Should I leave the PR as a draft? Or would it make more sense to close it completely and leave it as a reference for potential GSOC students? |
| Thanks for your follow-up. Yes, I agree it's best to leave it here as a draft. We can reference it from future PRs to provide context, if we decide to implement one part or the other in isolation. Thanks! |
4 participants
This PR adds initial support for out-of-process execution through llvm-jitlink-executor to clang-repl on ELF. The out-of-process executor can be invoked with "-oop-executor" or "-oop-executor-connect=" on ELF platforms. Now, clang-repl depends upon the ORC runtime to support static initializers and deinitializers.
This PR modifies the ORC ELFNix platform to allow JITDylibs to be reinitialized through the "__orc_rt_jit_dlupdate" function, since clang-repl incrementally adds static initializers to a single JITDylib.
On x86_64 linux, the following tests pass with the out-of-process executor flag. However, a new new testing file (out-of-process.cpp) was added with select tests to restrict the test to ELF only.